Commit Graph

71322 Commits

Author SHA1 Message Date
Taylor Blau
639c4a3992 builtin/repack.c: extract redundant pack cleanup for --geometric
To reduce the complexity of the already quite-long `cmd_repack()`
implementation, extract out the parts responsible for deleting redundant
packs from a geometric repack out into its own sub-routine.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-13 12:32:47 -07:00
Taylor Blau
054b5e4873 builtin/repack.c: extract marking packs for deletion
At the end of a repack (when given `-d`), Git attempts to remove any
packs which have been made "redundant" as a result of the repacking
operation. For example, an all-into-one (`-A` or `-a`) repack makes
every pre-existing pack which is not marked as kept redundant. Geometric
repacks (with `--geometric=<n>`) make any packs which were rolled up
redundant, and so on.

But before deleting the set of packs we think are redundant, we first
check to see whether or not we just wrote a pack which is identical to
any one of the packs we were going to delete. When this is the case, Git
must avoid deleting that pack, since it matches a pack we just wrote
(so deleting it may cause the repository to become corrupt).

Right now we only process the list of non-kept packs in a single pass.
But a future change will split the existing non-kept packs further into
two lists: one for cruft packs, and another for non-cruft packs.

Factor out this routine to prepare for calling it twice on two separate
lists in a future patch.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-13 12:32:47 -07:00
Taylor Blau
e2b43831a5 builtin/repack.c: extract structure to store existing packs
The repack machinery needs to keep track of which packfiles were present
in the repository at the beginning of a repack, segmented by whether or
not each pack is marked as kept.

The names of these packs are stored in two `string_list`s, corresponding
to kept- and non-kept packs, respectively. As a consequence, many
functions within the repack code need to take both `string_list`s as
arguments, leading to code like this:

    ret = write_cruft_pack(&cruft_po_args, packtmp, pack_prefix,
                           cruft_expiration, &names,
                           &existing_nonkept_packs, /* <- */
                           &existing_kept_packs);   /* <- */

Wrap up this pair of `string_list`s into a single structure that stores
both. This saves us from having to pass both string lists separately,
and prepares for adding additional fields to this structure.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-13 12:32:47 -07:00
Junio C Hamano
d6c51973e4 The eighth batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-13 10:07:57 -07:00
Junio C Hamano
d070b77d25 Merge branch 'ob/sequencer-reword-error-message'
Update an error message (which would probably never been seen).

* ob/sequencer-reword-error-message:
  sequencer: fix error message on failure to copy SQUASH_MSG
2023-09-13 10:07:57 -07:00
Junio C Hamano
877c9919d6 Merge branch 'bc/more-git-var'
Fix-up for a topic that already has graduated.

* bc/more-git-var:
  var: avoid a segmentation fault when `HOME` is unset
2023-09-13 10:07:57 -07:00
Junio C Hamano
331f20d52d Merge branch 'ew/hash-with-openssl-evp'
Fix-up new-ish code to support OpenSSL EVP API.

* ew/hash-with-openssl-evp:
  treewide: fix various bugs w/ OpenSSL 3+ EVP API
2023-09-13 10:07:57 -07:00
Junio C Hamano
c52a02a0f0 Merge branch 'jk/unused-post-2.42-part2'
Unused parameters to functions are marked as such, and/or removed,
in order to bring us closer to -Wunused-parameter clean.

* jk/unused-post-2.42-part2:
  parse-options: mark unused parameters in noop callback
  interpret-trailers: mark unused "unset" parameters in option callbacks
  parse-options: add more BUG_ON() annotations
  merge: do not pass unused opt->value parameter
  parse-options: mark unused "opt" parameter in callbacks
  parse-options: prefer opt->value to globals in callbacks
  checkout-index: delay automatic setting of to_tempfile
  format-patch: use OPT_STRING_LIST for to/cc options
  merge: simplify parsing of "-n" option
  merge: make xopts a strvec
2023-09-13 10:07:56 -07:00
Philippe Blain
4333267995 completion: improve doc for complex aliases
The completion code can be told to use a particular completion for
aliases that shell out by using ': git <cmd> ;' as the first command of
the alias. This only works if <cmd> and the semicolon are separated by a
space, since if the space is missing __git_aliased_command returns (for
example) 'checkout;' instead of just 'checkout', and then
__git_complete_command fails to find a completion for 'checkout;'.

The examples have that space but it's not clear if it's just for
style or if it's mandatory. Explicitly mention it.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 17:46:01 -07:00
Philippe Blain
0b658eae75 completion: commit: complete trailers tokens more robustly
In the previous commit, we added support for completing configured
trailer tokens in 'git commit --trailer'.

Make the implementation more robust by:

- using '__git' instead of plain 'git', as the rest of the completion
  script does
- using a stricter pattern for --get-regexp to avoid false hits
- using 'cut' and 'rev' instead of 'awk' to account for tokens including
  dots.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 17:33:59 -07:00
Oswald Buddenhagen
63642d58b4 sequencer: remove unreachable exit condition in pick_commits()
This was introduced by 56dc3ab04 ("sequencer (rebase -i): implement the
'edit' command", 2017-01-02), and was pointless from the get-go: all
early exits from the loop above are returns, so todo_list->current ==
todo_list->nr is an invariant after the loop.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 17:32:09 -07:00
Oswald Buddenhagen
8aae489756 t3404-rebase-interactive.sh: fix typos in title of a rewording test
This test was introduced by commit 0c164ae7a ("rebase -i: add another
reword test", 2021-08-20). I didn't quite get what it was meant to do,
so here's an explanation from Phillip:

The purpose of the test is to ensure that

  (i) There are no uncommitted changes when the editor runs. i.e., we
      commit without running the editor and then reword by amending
      that commit. This ensures that we have the same user experience
      whether or not the commit was fast-forwarded [1].

 (ii) That the todo list is re-read after the commit has been reworded.
      This is to allow the user to update the todo list while the rebase
      is paused for editing the commit message.

[1] https://lore.kernel.org/git/20190812175046.GM20404@szeder.dev/

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 17:24:56 -07:00
Junio C Hamano
83708f80fc test-tool: retire "index-version"
As "git update-index --show-index-version" can do the same thing,
the 'index-version' subcommand in the test-tool lost its reason to
exist.  Remove it and replace its use with the end-user facing
'git update-index --show-index-version'.

Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 16:21:53 -07:00
Junio C Hamano
606e088d5d update-index: add --show-index-version
"git update-index --index-version N" is used to set the index format
version to a specific version, but there was no way to query the
current version used in the on-disk index file.

Teach the command a new "--show-index-version" option, and also
teach the "--index-version N" option to report what the version was
when run with the "--verbose" option.

Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 16:21:53 -07:00
Junio C Hamano
764b2330db update-index doc: v4 is OK with JGit and libgit2
Being invented in late 2012 no longer makes the index v4 format
"relatively young".

The support for the index version 4 was added to libgit2 with their
5625d86b (index: support index v4, 2016-05-17) and to JGit with
their e9cb0a8e (DirCache: support index V4, 2020-08-10).

Let's update the paragraph that discouraged its use for folks overly
cautious about cross-tool compatibility.

Helped-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-12 16:21:53 -07:00
Josip Sokcevic
6a044a2048 diff-lib: fix check_removed when fsmonitor is on
`git diff-index` may return incorrect deleted entries when fsmonitor
is used in a repository with git submodules. This can be observed on
Mac machines, but it can affect all other supported platforms too.

If fsmonitor is used, `stat *st` is not initialized if cache_entry has
CE_FSMONITOR_VALID set. But, there are three call sites that rely on stat
afterwards, which can result in incorrect results.

This change partially reverts commit 4f3d6d02 (fsmonitor: skip lstat
deletion check during git diff-index, 2021-03-17).

Signed-off-by: Josip Sokcevic <sokcevic@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-11 16:45:49 -07:00
Johannes Schindelin
5e8515e8e8 maintenance(systemd): support the Windows Subsystem for Linux
When running in the Windows Subsystem for Linux (WSL), it is usually
necessary to use the Git Credential Manager for authentication when
performing the background fetches.

This requires interoperability between the Windows Subsystem for Linux
and the Windows host to work, which uses so-called vsocks, i.e. sockets
intended for communcations between virtual machines and the host they
are running on.

However, when Git is configured to run background maintenance via
`systemd`, the address families available to those maintenance processes
are restricted, and did not include `AF_VSOCK`. This leads to problems
e.g. when a background fetch tries to access github.com:

	systemd[437]: Starting Optimize Git repositories data...
	git[747387]: WSL (747387) ERROR: UtilBindVsockAnyPort:285: socket failed 97
	git[747381]: fatal: could not read Username for 'https://github.com': No such device or address
	git[747381]: error: failed to prefetch remotes
	git[747381]: error: task 'prefetch' failed
	systemd[437]: git-maintenance@hourly.service: Main process exited, code=exited, status=1/FAILURE
	systemd[437]: git-maintenance@hourly.service: Failed with result 'exit-code'.
	systemd[437]: Failed to start Optimize Git repositories data.

Address this (pun intended) by adding the `AF_VSOCK` address family to
the allow list.

This fixes https://github.com/microsoft/git/issues/604.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-11 12:41:30 -07:00
René Scharfe
48944f214c diff --no-index: fix -R with stdin
When -R is given, queue_diff() swaps the mode and name variables of the
two files to produce a reverse diff.  1e3f26542a (diff --no-index:
support reading from named pipes, 2023-07-05) added variables that
indicate whether files are special, i.e named pipes or - for stdin.
These new variables were not swapped, though, which broke the handling
of stdin with with -R.  Swap them like the other metadata variables.

Reported-by: Martin Storsjö <martin@martin.st>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-11 12:05:37 -07:00
Linus Arver
94430d03df trailer: split process_command_line_args into separate functions
Previously, process_command_line_args did two things:

    (1) parse trailers from the configuration, and
    (2) parse trailers defined on the command line.

Separate (1) outside to a new function, parse_trailers_from_config.
Rename the remaining logic to parse_trailers_from_command_line_args.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-11 10:01:19 -07:00
Linus Arver
c2a8edf997 trailer: split process_input_file into separate pieces
Currently, process_input_file does three things:

    (1) parse the input string for trailers,
    (2) print text before the trailers, and
    (3) calculate the position of the input where the trailers end.

Rename this function to parse_trailers(), and make it only do
(1). The caller of this function, process_trailers, becomes responsible
for (2) and (3). These items belong inside process_trailers because they
are both concerned with printing the surrounding text around
trailers (which is already one of the immediate concerns of
process_trailers).

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-11 10:01:19 -07:00
Linus Arver
13211ae23f trailer: separate public from internal portion of trailer_iterator
The fields here are not meant to be used by downstream callers, so put
them behind an anonymous struct named as "internal" to warn against
their use. This follows the pattern in 576de3d956 (unpack_trees: start
splitting internal fields from public API, 2023-02-27).

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-11 10:01:18 -07:00
Johannes Schindelin
9f892830d6 completion(switch/checkout): treat --track and -t the same
When `git switch --track ` is to be completed, only remote refs are
eligible because that is what the `--track` option targets.

And when the short-hand `-t` is used instead, the same _should_ happen.
Let's make it so.

Note that the bug exists both in the completions of `switch` and
`completion`, even if it manifests in slightly different ways: While
the completion of `git switch -t ` will not even look at remote refs,
the completion of `git checkout -t ` will look at both remote _and_
local refs. Both should look only at remote refs.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-08 09:26:15 -07:00
Linus Arver
6ccbc66794 trailer doc: <token> is a <key> or <keyAlias>, not both
The `--trailer` option takes a "<token>=<value>" argument, for example

    --trailer "Acked-by=Bob"

And in this exampple it is understood that "Acked-by" is the <token>.
However, the user can use a shorter "ack" string by defining
configuration like

    git config trailer.ack.key "Acked-by"

However, in the docs we define the above configuration as

    trailer.<token>.key

so the <token> can mean either the longer "Acked-by" or the shorter
"ack".

Separate the two meanings of <token> into <key> and <keyAlias>, and
update the configuration syntax to say "trailer.<keyAlias>.key".

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
ab76661f22 trailer doc: separator within key suppresses default separator
Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
db97296122 trailer doc: emphasize the effect of configuration variables
The sentence does not mention the effect of configuration variables at
all, when they are actively used by default (unless --parse is
specified) to potentially add new trailers, without the user having to
always supply --trailer manually.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
289a0b2447 trailer --unfold help: prefer "reformat" over "join"
The phrase "join whitespace-continued values" requires some additional
context. For example, "whitespace" means newlines (not just space
characters), and "join" means to join only the multiple lines together
for a single trailer (and not that we are joining multiple trailers
together). That is, "join" means to convert

    token: This is a very long value, with spaces and
      newlines in it.

to

    token: This is a very long value, with spaces and newlines in it.

and does not mean to convert

    token: value1
    token: value2

to

    token: value1 value2.

Update the help text to resolve the above ambiguity. While we're add it,
update the docs to use similar language as the change in the help text.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
cb088cbe0f trailer --parse docs: add explanation for its usefulness
For users who are skimming the docs to go straight to the individual
breakdown of each flag, it may not be clear why --parse is a convenience
alias (without them also looking at the other options that --parse turns
on). To save them the trouble of looking at the other options (and
computing what that would mean), describe a summary of the overall
effect.

Similarly update the area when we first mention --parse near the top of
the doc.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
a6c72e7046 trailer --only-input: prefer "configuration variables" over "rules"
Use the phrase "configuration variables" instead of "rules" because

(1) we already say "configuration variables" in multiple
    places in the docs (where the word "rules" is only used for describing
    "--only-input" behavior and for an unrelated case of mentioning how
    the trailers do not follow "rules for RFC 822 headers"), and

(2) this phrase is more specific than just "rules".

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
8c7d4acb07 trailer --parse help: expose aliased options
The existing description "set parsing options" is vague, because
arguably _all_ of the options for interpret-trailers have to do with
parsing to some degree.

Explain what this flag does to match what is in the docs, namely how
it is an alias for "--only-trailers --only-input --unfold".

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
b674f25b81 trailer --no-divider help: describe usual "---" meaning
It's unclear what treating something "specially" means.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
467bb1b97a trailer: trailer location is a place, not an action
Fix the help text to say "placement" instead of "action" because the
values are placements, not actions.

While we're at it, tweak the documentation to say "placements" instead
of "values", similar to how the existing language for "--if-exists" uses
the word "action" to describe both the syntax (with the phrase
"--if-exists <action>") and the possible values (with the phrase
"possible actions").

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
f659c56a8c trailer doc: narrow down scope of --where and related flags
The wording "all configuration variables" is misleading (the same could
be said to the descriptions of the "--[no-]if-exists" and the
"--[no-]if-missing" options).  Specifying --where=value overrides only
the trailer.where variable and applicable trailer.<token>.where
variables, and --no-where stops the overriding of these variables.
Ditto for the other two with their relevant configuration variables.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
73574f21b4 trailer: add tests to check defaulting behavior with --no-* flags
While the "--no-where" flag is tested, the "--no-if-exists" and
"--no-if-missing" flags are not, so add tests for them. But also add
tests for all "--no-*" flags to check their effects, both when (1) there
are relevant configuration variables set, and (2) they are not set.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:44 -07:00
Linus Arver
e670ba2500 trailer test description: this tests --where=after, not --where=before
Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:04:36 -07:00
Linus Arver
84e53330f0 trailer tests: make test cases self-contained
By using "test_config" instead of "git config", we avoid leaking
configuration state across test cases. This in turn helps to make the
tests more self-contained, by explicitly capturing the configuration
setup. It then makes it easier to add tests anywhere in this 1500+ line
file, without worrying about what implicit state was set in some prior
test case defined earlier up in the script.

This commit was created mechanically as follows: we changed the first
occurrence of a particular "git config trailer.*" option, then ran the
tests repeatedly to see which ones broke, adding in the extra
"test_config" equivalents to make them pass again. In addition, in some
test cases we removed "git config --unset ..." lines because they were
no longer necessary (as the --unset was being used to clean up leaked
configuration state from earlier test cases).

The process described above was done repeatedly until there were no more
unbridled "git config" invocations. Some "git config" invocations still
do exist in the script, but they were already cleaned up properly with

    test_when_finished "git config --remove-section ..."

so they were left alone.

Note that these cleanups result in generally longer test case setups
because the previously hidden state is now being exposed. Although we
could then clean up the test cases' "expected" values to be less
verbose (the verbosity arising from the use of implicit state), we
choose not to do so here, to make sure that this cleanup does not change
any meanings behind the test cases.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 23:03:57 -07:00
Junio C Hamano
94e83dcf5b The seventh batch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 15:06:19 -07:00
Junio C Hamano
09684a12b0 Merge branch 'dd/format-patch-rfc-updates'
"git format-patch --rfc --subject-prefix=<foo>" used to ignore the
"--subject-prefix" option and used "[RFC PATCH]"; now we will add
"RFC" prefix to whatever subject prefix is specified.

This is a backward compatible change that may deserve a note.

* dd/format-patch-rfc-updates:
  format-patch: --rfc honors what --subject-prefix sets
2023-09-07 15:06:08 -07:00
Junio C Hamano
32de857fb2 Merge branch 'jk/ci-retire-allow-ref'
CI update.

* jk/ci-retire-allow-ref:
  ci: deprecate ci/config/allow-ref script
  ci: allow branch selection through "vars"
2023-09-07 15:06:08 -07:00
Junio C Hamano
300b2a1047 Merge branch 'ws/git-svn-retire-faketerm'
Code clean-up.

* ws/git-svn-retire-faketerm:
  git-svn: drop FakeTerm hack
2023-09-07 15:06:08 -07:00
Junio C Hamano
25ff15d108 Merge branch 'jk/unused-post-2.42'
Unused parameters to functions are marked as such, and/or removed,
in order to bring us closer to -Wunused-parameter clean.

* jk/unused-post-2.42: (22 commits)
  update-ref: mark unused parameter in parser callbacks
  gc: mark unused descriptors in scheduler callbacks
  bundle-uri: mark unused parameters in callbacks
  fetch: mark unused parameter in ref_transaction callback
  credential: mark unused parameter in urlmatch callback
  grep: mark unused parmaeters in pcre fallbacks
  imap-send: mark unused parameters with NO_OPENSSL
  worktree: mark unused parameters in noop repair callback
  negotiator/noop: mark unused callback parameters
  add-interactive: mark unused callback parameters
  grep: mark unused parameter in output function
  test-trace2: mark unused argv/argc parameters
  trace2: mark unused config callback parameter
  trace2: mark unused us_elapsed_absolute parameters
  stash: mark unused parameter in diff callback
  ls-tree: mark unused parameter in callback
  commit-graph: mark unused data parameters in generation callbacks
  worktree: mark unused parameters in each_ref_fn callback
  pack-bitmap: mark unused parameters in show_object callback
  ref-filter: mark unused parameters in parser callbacks
  ...
2023-09-07 15:06:07 -07:00
Junio C Hamano
8af5aac986 Merge branch 'tb/multi-cruft-pack'
Use of --max-pack-size to allow multiple packfiles to be created is
now supported even when we are sending unreachable objects to cruft
packs.

* tb/multi-cruft-pack:
  Documentation/gitformat-pack.txt: drop mixed version section
  Documentation/gitformat-pack.txt: remove multi-cruft packs alternative
  builtin/pack-objects.c: support `--max-pack-size` with `--cruft`
  builtin/pack-objects.c: remove unnecessary strbuf_reset()
2023-09-07 15:06:07 -07:00
René Scharfe
aae8558b10 grep: reject --no-or
Since 3e230fa1b2 (grep: use parseopt, 2009-05-07) git grep has been
accepting the option --no-or.  It does the same as --or: nothing.
That's confusing and unintended.  Forbid negating --or.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 13:35:07 -07:00
Philippe Blain
c7153fad2d completion: commit: complete configured trailer tokens
Since 2daae3d1d1 (commit: add --trailer option, 2021-03-23), 'git
commit' can add trailers to commit messages. To make that feature more
pleasant to use at the command line, update the Bash completion code to
offer configured trailer tokens.

Add a __git_trailer_tokens function to list the configured trailers
tokens, and use it in _git_commit to suggest the configured tokens,
suffixing the completion words with ':' so that the user only has to add
the trailer value.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-07 12:37:07 -07:00
Phillip Wood
203573b024 rebase -i: fix adding failed command to the todo list
When rebasing commands are moved from the todo list in "git-rebase-todo"
to the "done" file (which is used by "git status" to show the recently
executed commands) just before they are executed. This means that if a
command fails because it would overwrite an untracked file it has to be
added back into the todo list before the rebase stops for the user to
fix the problem.

Unfortunately when a failed command is added back into the todo list the
command preceding it is erroneously appended to the "done" file.  This
means that when rebase stops after "pick B" fails the "done" file
contains

	pick A
	pick B
	pick A

instead of

	pick A
	pick B

This happens because save_todo() updates the "done" file with the
previous command whenever "git-rebase-todo" is updated. When we add the
failed pick back into "git-rebase-todo" we do not want to update
"done". Fix this by adding a "reschedule" parameter to save_todo() which
prevents the "done" file from being updated when adding a failed command
back into the "git-rebase-todo" file. A couple of the existing tests are
modified to improve their coverage as none of them trigger this bug or
check the "done" file.

Reported-by: Stefan Haller <lists@haller-berlin.de>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:44 -07:00
Phillip Wood
405509cbd6 rebase --continue: refuse to commit after failed command
If a commit cannot be picked because it would overwrite an untracked
file then "git rebase --continue" should refuse to commit any staged
changes as the commit was not picked. This is implemented by refusing to
commit if the message file is missing. The message file is chosen for
this check because it is only written when "git rebase" stops for the
user to resolve merge conflicts.

Existing commands that refuse to commit staged changes when continuing
such as a failed "exec" rely on checking for the absence of the author
script in run_git_commit(). This prevents the staged changes from being
committed but prints

    error: could not open '.git/rebase-merge/author-script' for
    reading

before the message about not being able to commit. This is confusing to
users and so checking for the message file instead improves the user
experience. The existing test for refusing to commit after a failed exec
is updated to check that we do not print the error message about a
missing author script anymore.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:44 -07:00
Phillip Wood
e032abd5a0 rebase: fix rewritten list for failed pick
git rebase keeps a list that maps the OID of each commit before it was
rebased to the OID of the equivalent commit after the rebase.  This list
is used to drive the "post-rewrite" hook that is called at the end of a
successful rebase. When a rebase stops for the user to resolve merge
conflicts the OID of the commit being picked is written to
".git/rebase-merge/stopped-sha". Then when the rebase is continued that
OID is added to the list of rewritten commits. Unfortunately if a commit
cannot be picked because it would overwrite an untracked file we still
write the "stopped-sha1" file. This means that when the rebase is
continued the commit is added into the list of rewritten commits even
though it has not been picked yet.

Fix this by not calling error_with_patch() for failed commands. The pick
has failed so there is nothing to commit and therefore we do not want to
set up the state files for committing staged changes when the rebase
continues. This change means we no-longer write a patch for the failed
command or display the error message printed by error_with_patch(). As
the command has failed the patch isn't really useful and in any case the
user can inspect the commit associated with the failed command by
inspecting REBASE_HEAD. Unless the user has disabled it we already print
an advice message that is more helpful than the message from
error_with_patch() which the user will still see. Even if the advice is
disabled the user will see the messages from the merge machinery
detailing the problem.

The code to add a failed command back into the todo list is duplicated
between pick_one_commit() and the loop in pick_commits(). Both sites
print advice about the command being rescheduled, decrement the current
item and save the todo list. To avoid duplicating this code
pick_one_commit() is modified to set a flag to indicate that the command
should be rescheduled in the main loop. This simplifies things as only
the remaining copy of the code needs to be modified to set REBASE_HEAD
rather than calling error_with_patch().

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:43 -07:00
Phillip Wood
f2b5f41eda sequencer: factor out part of pick_commits()
This simplifies the next commit. If a pick fails we now return the error
at the end of the loop body rather than returning early, a successful
"edit" command continues to return early. There are three things to
check to ensure that removing the early return for an error does not
change the behavior of the code:

(1) We could enter the block guarded by "if (reschedule)". This block
    is not entered because "reschedlue" is always zero when picking a
    commit.

(2) We could enter the block guarded by
    "else if (is_rebase_i(opts) &&  check_todo && !res)". This block is
    not entered when returning an error because "res" is non-zero in
    that case.

(3) todo_list->current could be incremented before returning. That is
    avoided by moving the increment which is of course a potential
    change in behavior itself. The move is safe because none of the
    callers look at todo_list after this function returns. Moving the
    increment makes it clear we only want to advance the current item
    if the command was successful.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:43 -07:00
Phillip Wood
9f67899b41 sequencer: use rebase_path_message()
Rather than constructing the path in a struct strbuf use the ready
made function to get the path name instead. This was the last
remaining use of the strbuf so remove it as well.

As with the previous patch we now use a hard coded string rather than
git_dir() when constructing the path. This is safe for the same
reason (make_patch() is only called when rebasing) and is protected by
the assertion added in the previous patch.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:43 -07:00
Phillip Wood
206a78d710 rebase -i: remove patch file after conflict resolution
When a rebase stops for the user to resolve conflicts it writes a patch
for the conflicting commit to .git/rebase-merge/patch. This file has
been written since the introduction of "git-rebase-interactive.sh" in
1b1dce4bae (Teach rebase an interactive mode, 2007-06-25). I assume the
idea was to enable the user inspect the conflicting commit in the same
way as they could for the patch based rebase. This file should be
deleted when the rebase continues as if the rebase stops for a failed
"exec" command or a "break" command it is confusing to the user if there
is a stale patch lying around from an unrelated command. As the path is
now used in two different places rebase_path_patch() is added and used
to obtain the path for the patch.

To construct the path write_patch() previously used get_dir() which
returns different paths depending on whether we're rebasing or
cherry-picking/reverting. As this function is only called when
rebasing it is safe to use a hard coded string for the directory
instead. An assertion is added to make sure we don't starting calling
this function when cherry-picking in the future.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:43 -07:00
Phillip Wood
36ac861a30 rebase -i: move unlink() calls
At the start of each iteration the loop that picks commits removes the
state files from the previous pick. However some of these files are only
written if there are conflicts in which case we exit the loop before the
end of the loop body. Therefore they only need to be removed when the
rebase continues, not at the start of each iteration.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-09-06 10:29:43 -07:00