The auto-stashed local changes created by "git merge --autostash"
was mixed into a conflicted state left in the working tree, which
has been corrected.
* en/merge-unstash-only-on-clean-merge:
merge: only apply autostash when appropriate
Junio C Hamano [Thu, 1 Sep 2022 20:40:18 +0000 (13:40 -0700)]
Merge branch 'sg/parse-options-subcommand'
Introduce the "subcommand" mode to parse-options API and update the
command line parser of Git commands with subcommands.
* sg/parse-options-subcommand: (23 commits)
remote: run "remote rm" argv through parse_options()
maintenance: add parse-options boilerplate for subcommands
pass subcommand "prefix" arguments to parse_options()
builtin/worktree.c: let parse-options parse subcommands
builtin/stash.c: let parse-options parse subcommands
builtin/sparse-checkout.c: let parse-options parse subcommands
builtin/remote.c: let parse-options parse subcommands
builtin/reflog.c: let parse-options parse subcommands
builtin/notes.c: let parse-options parse subcommands
builtin/multi-pack-index.c: let parse-options parse subcommands
builtin/hook.c: let parse-options parse subcommands
builtin/gc.c: let parse-options parse 'git maintenance's subcommands
builtin/commit-graph.c: let parse-options parse subcommands
builtin/bundle.c: let parse-options parse subcommands
parse-options: add support for parsing subcommands
parse-options: drop leading space from '--git-completion-helper' output
parse-options: clarify the limitations of PARSE_OPT_NODASH
parse-options: PARSE_OPT_KEEP_UNKNOWN only applies to --options
api-parse-options.txt: fix description of OPT_CMDMODE
t0040-parse-options: test parse_options() with various 'parse_opt_flags'
...
Junio C Hamano [Thu, 1 Sep 2022 20:40:17 +0000 (13:40 -0700)]
Merge branch 'ds/bundle-uri-clone'
Implement "git clone --bundle-uri".
* ds/bundle-uri-clone:
clone: warn on failure to repo_init()
clone: --bundle-uri cannot be combined with --depth
bundle-uri: add support for http(s):// and file://
clone: add --bundle-uri option
bundle-uri: create basic file-copy logic
remote-curl: add 'get' capability
Junio C Hamano [Mon, 29 Aug 2022 21:55:15 +0000 (14:55 -0700)]
Merge branch 'es/fix-chained-tests'
Fix broken "&&-" chains and failures in early iterations of a loop.
* es/fix-chained-tests:
t5329: notice a failure within a loop
t: detect and signal failure within loop
t1092: fix buggy sparse "blame" test
t2407: fix broken &&-chains in compound statement
Junio C Hamano [Mon, 29 Aug 2022 21:55:14 +0000 (14:55 -0700)]
Merge branch 'sg/xcalloc-cocci-fix'
xcalloc(), imitating calloc(), takes "number of elements of the
array", and "size of a single element", in this order. A call that
does not follow this ordering has been corrected.
* sg/xcalloc-cocci-fix:
promisor-remote: fix xcalloc() argument order
Junio C Hamano [Mon, 29 Aug 2022 21:55:13 +0000 (14:55 -0700)]
Merge branch 'mg/sequencer-untranslate-reflog'
The sequencer machinery translated messages left in the reflog by
mistake, which has been corrected.
* mg/sequencer-untranslate-reflog:
sequencer: do not translate command names
sequencer: do not translate parameters to error_resolve_conflict()
sequencer: do not translate reflog messages
Junio C Hamano [Mon, 29 Aug 2022 21:55:12 +0000 (14:55 -0700)]
Merge branch 'jk/unused-fixes'
Code clean-up to remove unused function parameters.
* jk/unused-fixes:
xdiff: drop unused mmfile parameters from xdl_do_patience_diff()
reflog: assert PARSE_OPT_NONEG in parse-options callbacks
reftable: drop unused parameter from reader_seek_linear()
verify_one_sparse(): drop unused parameters
match_pathname(): drop unused "flags" parameter
log-tree: drop unused commit param in remerge_diff()
xdiff: drop unused mmfile parameters from xdl_do_histogram_diff()
Junio C Hamano [Mon, 29 Aug 2022 21:55:12 +0000 (14:55 -0700)]
Merge branch 'vd/scalar-enables-fsmonitor'
"scalar" now enables built-in fsmonitor on enlisted repositories,
when able.
* vd/scalar-enables-fsmonitor:
scalar: update technical doc roadmap with FSMonitor support
scalar unregister: stop FSMonitor daemon
scalar: enable built-in FSMonitor on `register`
scalar: move config setting logic into its own function
scalar-delete: do not 'die()' in 'delete_enlistment()'
scalar-[un]register: clearly indicate source of error
scalar-unregister: handle error codes greater than 0
scalar: constrain enlistment search
Junio C Hamano [Mon, 29 Aug 2022 21:55:11 +0000 (14:55 -0700)]
Merge branch 'en/ancestry-path-in-a-range'
"git rev-list --ancestry-path=C A..B" is a natural extension of
"git rev-list A..B"; instead of choosing a subset of A..B to those
that have ancestry relationship with A, it lets a subset with
ancestry relationship with C.
* en/ancestry-path-in-a-range:
revision: allow --ancestry-path to take an argument
t6019: modernize tests with helper
rev-list-options.txt: fix simple typo
Junio C Hamano [Mon, 29 Aug 2022 21:55:11 +0000 (14:55 -0700)]
Merge branch 'mt/rot13-in-c'
Test portability improvements.
* mt/rot13-in-c:
tests: use the new C rot13-filter helper to avoid PERL prereq
t0021: implementation the rot13-filter.pl script in C
t0021: avoid grepping for a Perl-specific string at filter output
Junio C Hamano [Mon, 29 Aug 2022 21:55:10 +0000 (14:55 -0700)]
Merge branch 'ds/decorate-filter-tweak'
The namespaces used by "log --decorate" from "refs/" hierarchy by
default has been tightened.
* ds/decorate-filter-tweak:
fetch: use ref_namespaces during prefetch
maintenance: stop writing log.excludeDecoration
log: create log.initialDecorationSet=all
log: add --clear-decorations option
log: add default decoration filter
log-tree: use ref_namespaces instead of if/else-if
refs: use ref_namespaces for replace refs base
refs: add array of ref namespaces
t4207: test coloring of grafted decorations
t4207: modernize test
refs: allow "HEAD" as decoration filter
Jeff King [Sun, 28 Aug 2022 12:50:50 +0000 (08:50 -0400)]
pack-bitmap-write: drop unused pack_idx_entry parameters
Our write_selected_commits_v1() function takes an array of
pack_idx_entry structs. We used to need them for computing commit
positions, but since aa30162559 (bitmap: move `get commit positions`
code to `bitmap_writer_finish`, 2022-08-14), the caller passes in a
separate array of positions for us. We can drop the unused array (and
its matching length parameter).
Likewise, when we added write_lookup_table() in 93eb41e240
(pack-bitmap-write.c: write lookup table extension, 2022-08-14), it
receives the same array of positions. So its "index" parameter was never
used at all.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 26 Aug 2022 18:13:12 +0000 (11:13 -0700)]
Merge branch 'jk/fsck-tree-mode-bits-fix' into maint
"git fsck" reads mode from tree objects but canonicalizes the mode
before passing it to the logic to check object sanity, which has
hid broken tree objects from the checking logic. This has been
corrected, but to help exiting projects with broken tree objects
that they cannot fix retroactively, the severity of anomalies this
code detects has been demoted to "info" for now.
source: <YvQcNpizy9uOZiAz@coredump.intra.peff.net>
* jk/fsck-tree-mode-bits-fix:
fsck: downgrade tree badFilemode to "info"
fsck: actually detect bad file modes in trees
tree-walk: add a mechanism for getting non-canonicalized modes
Junio C Hamano [Fri, 26 Aug 2022 18:13:12 +0000 (11:13 -0700)]
Merge branch 'js/safe-directory-plus' into maint
Platform-specific code that determines if a directory is OK to use
as a repository has been taught to report more details, especially
on Windows.
source: <pull.1286.v2.git.1659965270.gitgitgadget@gmail.com>
* js/safe-directory-plus:
mingw: handle a file owned by the Administrators group correctly
mingw: be more informative when ownership check fails on FAT32
mingw: provide details about unsafe directories' ownership
setup: prepare for more detailed "dubious ownership" messages
setup: fix some formatting
Junio C Hamano [Fri, 26 Aug 2022 18:13:12 +0000 (11:13 -0700)]
Merge branch 'pw/use-glibc-tunable-for-malloc-optim' into maint
Avoid repeatedly running getconf to ask libc version in the test
suite, and instead just as it once per script.
source: <pull.1311.git.1659620305757.gitgitgadget@gmail.com>
* pw/use-glibc-tunable-for-malloc-optim:
tests: cache glibc version check
Junio C Hamano [Fri, 26 Aug 2022 18:13:10 +0000 (11:13 -0700)]
Merge branch 'jk/struct-zero-init-with-older-gcc' into maint
Older gcc with -Wall complains about the universal zero initializer
"struct s = { 0 };" idiom, which makes developers' lives
inconvenient (as -Werror is enabled by DEVELOPER=YesPlease). The
build procedure has been tweaked to help these compilers.
source: <YuQ60ZUPBHAVETD7@coredump.intra.peff.net>
* jk/struct-zero-init-with-older-gcc:
config.mak.dev: squelch -Wno-missing-braces for older gcc
Junio C Hamano [Fri, 26 Aug 2022 18:13:10 +0000 (11:13 -0700)]
Merge branch 'js/mingw-with-python' into maint
Conditionally allow building Python interpreter on Windows
source: <pull.1306.v2.git.1659109272.gitgitgadget@gmail.com>
* js/mingw-with-python:
mingw: remove unneeded `NO_CURL` directive
mingw: remove unneeded `NO_GETTEXT` directive
windows: include the Python bits when building Git for Windows
Junio C Hamano [Fri, 26 Aug 2022 18:13:10 +0000 (11:13 -0700)]
Merge branch 'ca/unignore-local-installation-on-windows' into maint
Fix build procedure for Windows that uses CMake so that it can pick
up the shell interpreter from local installation location.
source: <pull.1304.git.1658912756815.gitgitgadget@gmail.com>
* ca/unignore-local-installation-on-windows:
cmake: support local installations of git
bitmap-lookup-table: add performance tests for lookup table
Add performance tests to verify the performance of lookup table.
`p5310-pack-bitmaps.sh` contain tests with and without lookup table.
`p5312-pack-bitmaps-revs.sh` contain same tests with and without
lookup table but with `pack.writeReverseIndex` enabled.
Lookup table makes Git run faster in most of the cases. Below is the
result of `t/perf/p5310-pack-bitmaps.sh`.`perf/p5326-multi-pack-bitmaps.sh`
gives similar result. The repository used in the test is linux kernel.
Test this tree
-----------------------------------------------------------------------
5310.4: enable lookup table: false 0.01(0.00+0.00)
5310.5: repack to disk 320.89(230.20+23.45)
5310.6: simulated clone 14.04(5.78+1.79)
5310.7: simulated fetch 1.95(3.05+0.20)
5310.8: pack to file (bitmap) 44.73(20.55+7.45)
5310.9: rev-list (commits) 0.78(0.46+0.10)
5310.10: rev-list (objects) 4.07(3.97+0.08)
5310.11: rev-list with tag negated via --not 0.06(0.02+0.03)
--all (objects)
5310.12: rev-list with negative tag (objects) 0.21(0.15+0.05)
5310.13: rev-list count with blob:none 0.24(0.17+0.06)
5310.14: rev-list count with blob:limit=1k 7.07(5.92+0.48)
5310.15: rev-list count with tree:0 0.25(0.17+0.07)
5310.16: simulated partial clone 5.67(3.28+0.64)
5310.18: clone (partial bitmap) 16.05(8.34+1.86)
5310.19: pack to file (partial bitmap) 59.76(27.22+7.43)
5310.20: rev-list with tree filter (partial bitmap) 0.90(0.18+0.16)
5310.24: enable lookup table: true 0.01(0.00+0.00)
5310.25: repack to disk 319.73(229.30+23.01)
5310.26: simulated clone 13.69(5.72+1.78)
5310.27: simulated fetch 1.84(3.02+0.16)
5310.28: pack to file (bitmap) 45.63(20.67+7.50)
5310.29: rev-list (commits) 0.56(0.39+0.8)
5310.30: rev-list (objects) 3.77(3.74+0.08)
5310.31: rev-list with tag negated via --not 0.05(0.02+0.03)
--all (objects)
5310.32: rev-list with negative tag (objects) 0.21(0.15+0.05)
5310.33: rev-list count with blob:none 0.23(0.17+0.05)
5310.34: rev-list count with blob:limit=1k 6.65(5.72+0.40)
5310.35: rev-list count with tree:0 0.23(0.16+0.06)
5310.36: simulated partial clone 5.57(3.26+0.59)
5310.38: clone (partial bitmap) 15.89(8.39+1.84)
5310.39: pack to file (partial bitmap) 58.32(27.55+7.47)
5310.40: rev-list with tree filter (partial bitmap) 0.73(0.18+0.15)
Test 4-15 are tested without using lookup table. Same tests are
repeated in 16-30 (using lookup table).
Mentored-by: Taylor Blau <me@ttaylorr.com> Co-Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-bitmap-write: learn pack.writeBitmapLookupTable and add tests
Teach Git to provide a way for users to enable/disable bitmap lookup
table extension by providing a config option named 'writeBitmapLookupTable'.
Default is false.
Also add test to verify writting of lookup table.
Mentored-by: Taylor Blau <me@ttaylorr.com> Co-Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Co-Authored-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The bitmap lookup table extension was documented by an earlier
change, but Git does not yet know how to write that extension.
Teach Git to write bitmap lookup table extension. The table contains
the list of `N` <commit_pos, offset, xor_row>` triplets. These
triplets are sorted according to their commit pos (ascending order).
The meaning of each data in the i'th triplet is given below:
- commit_pos stores commit position (in the pack-index or midx).
It is a 4 byte network byte order unsigned integer.
- offset is the position (in the bitmap file) from which that
commit's bitmap can be read.
- xor_row is the position of the triplet in the lookup table
whose bitmap is used to compress this bitmap, or `0xffffffff`
if no such bitmap exists.
Mentored-by: Taylor Blau <me@ttaylorr.com> Co-mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Co-authored-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
bitmap: move `get commit positions` code to `bitmap_writer_finish`
The `write_selected_commits_v1` function takes care of writing commit
positions along with their corresponding bitmaps in the disk. It is
OK because this `search commit position of a given commit` algorithm
is needed only once here. But in later changes of the `lookup table
extension series`, we need same commit positions which means we have
to run the above mentioned algorithm one more time.
Move the `search commit position of a given commit` algorithm to
`bitmap_writer_finish()` and use the `commit_positions` array
to get commit positions of their corresponding bitmaps.
Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When reading bitmap file, Git loads each and every bitmap one by one
even if all the bitmaps are not required. A "bitmap lookup table"
extension to the bitmap format can reduce the overhead of loading
bitmaps which stores a list of bitmapped commit id pos (in the midx
or pack, along with their offset and xor offset. This way Git can
load only the necessary bitmaps without loading the previous bitmaps.
Older versions of Git ignore the lookup table extension and don't
throw any kind of warning or error while parsing the bitmap file.
Add some information for the new "bitmap lookup table" extension in the
bitmap-format documentation.
Mentored-by: Taylor Blau <me@ttaylorr.com> Co-Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Co-Authored-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 25 Aug 2022 21:42:31 +0000 (14:42 -0700)]
Merge branch 'jk/pipe-command-nonblock'
Fix deadlocks between main Git process and subprocess spawned via
the pipe_command() API, that can kill "git add -p" that was
reimplemented in C recently.
* jk/pipe-command-nonblock:
pipe_command(): mark stdin descriptor as non-blocking
pipe_command(): handle ENOSPC when writing to a pipe
pipe_command(): avoid xwrite() for writing to pipe
git-compat-util: make MAX_IO_SIZE define globally available
nonblock: support Windows
compat: add function to enable nonblocking pipes
Junio C Hamano [Thu, 25 Aug 2022 21:42:29 +0000 (14:42 -0700)]
Merge branch 'en/submodule-merge-messages-fixes'
Further update the help messages given while merging submodules.
* en/submodule-merge-messages-fixes:
merge-ort: provide helpful submodule update message when possible
merge-ort: avoid surprise with new sub_flag variable
merge-ort: remove translator lego in new "submodule conflict suggestion"
submodule merge: update conflict error message
Jeff King [Thu, 25 Aug 2022 10:51:40 +0000 (06:51 -0400)]
remote: run "remote rm" argv through parse_options()
The "git remote rm" command's option parsing is fairly primitive: it
insists on a single argument, which it treats as the remote name, and
displays a usage message otherwise.
This is OK, and maybe even convenient, as you could run:
git remote rm --foo
to drop a remote named "--foo". But it's also weirdly unlike most of the
rest of Git, which would complain that there is no option "--foo". The
right way to spell it by our conventions is:
git remote rm -- --foo
but this doesn't currently work.
So let's bring the command in line with the rest of Git (including its
sibling subcommands!) by feeding argv to parse_options(). We already
have an empty options array for the usage helper.
Note that we have to adjust the argc index down by one, as
parse_options() eats the program name from the start of the array.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Aug 2022 10:51:06 +0000 (06:51 -0400)]
maintenance: add parse-options boilerplate for subcommands
Several of the git-maintenance subcommands don't take any options, so
they don't bother looking at argv at all. This means they'll silently
accept garbage, like:
$ git maintenance register --foo
[no output]
$ git maintenance stop bar
[no output]
Let's give them the basic boilerplate to detect and handle these cases:
$ git maintenance stop bar
usage: git maintenance stop
We could reduce the number of lines of code here a bit with a shared
helper function. But it's worth building out the boilerplate, as it may
serve as the base for adding options later.
Note one complication: maintenance_start() calls directly into
maintenance_register(), so it now needs to pass a plausible argv (we
don't care, but parse_options() is expecting there to at least be an
argv[0] program name). This is an extra line of code, but it eliminates
the need for an explanatory comment.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Aug 2022 10:47:00 +0000 (06:47 -0400)]
pass subcommand "prefix" arguments to parse_options()
Recent commits such as bf0a6b65fc (builtin/multi-pack-index.c: let
parse-options parse subcommands, 2022-08-19) converted a few functions
to match our usual argc/argv/prefix conventions, but the prefix argument
remains unused.
However, there is a good use for it: they should pass it to their own
parse_options() functions, where it may be used to adjust the value of
any filename options. In all but one of these functions, there's no
behavior change, since they don't use OPT_FILENAME. But this is an
actual fix for one option, which you can see by modifying the test suite
like so:
diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh
index 4fe57414c1..d0974d4371 100755
--- a/t/t5326-multi-pack-bitmaps.sh
+++ b/t/t5326-multi-pack-bitmaps.sh
@@ -186,7 +186,11 @@ test_expect_success 'writing a bitmap with --refs-snapshot' '
# Then again, but with a refs snapshot which only sees
# refs/tags/one.
- git multi-pack-index write --bitmap --refs-snapshot=snapshot &&
+ (
+ mkdir subdir &&
+ cd subdir &&
+ git multi-pack-index write --bitmap --refs-snapshot=../snapshot
+ ) &&
I'd emphasize that this wasn't broken by bf0a6b65fc; it has been broken
all along, because the sub-function never got to see the prefix. It is
that commit which is actually enabling us to fix it (and which also
brought attention to the problem because it triggers -Wunused-parameter!)
The other functions changed here don't use OPT_FILENAME at all. In their
cases this isn't fixing anything visible, but it's following the usual
pattern and future-proofing them against somebody adding new options and
being surprised.
I didn't include a test for the one visible case above. We don't
generally test routine parse-options behavior for individual options.
The challenge here was finding the problem, and now that this has been
done, it's not likely to regress. Likewise, we could apply the patch
above to cover it "for free" but it makes reading the rest of the test
unnecessarily complicated.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 22 Aug 2022 20:59:25 +0000 (13:59 -0700)]
t5329: notice a failure within a loop
We try to write "|| return 1" (or "|| exit 1" in a subshell) at the
end of a sequence of &&-chained command in a loop of our tests, so
that a failure of any step during the earlier iteration of the loop
can properly be caught.
There is one loop in this test script that is used to compute the
expected result, that will be later compared with an actual output
produced by the "test-tool pack-mtimes" command. This particular
loop, however, is placed on the upstream side of a pipe, whose
non-zero exit code does not get noticed.
Emit a line that will never be produced by the "test-tool pack-mtimes"
to cause the later comparison to fail. As we use test_cmp to compare
this "expected output" file with the "actual output", the "error
message" we are emitting into the expected output stream will stand
out and shown to the tester.
Derrick Stolee [Tue, 23 Aug 2022 17:28:11 +0000 (17:28 +0000)]
ci: update 'static-analysis' to Ubuntu 22.04
GitHub Actions scheduled a brownout of Ubuntu 18.04, which canceled all
runs of the 'static-analysis' job in our CI runs. Update to 22.04 to
avoid this as the brownout later turns into a complete deprecation.
The use of 18.04 was set in d051ed77ee6 (.github/workflows/main.yml: run
static-analysis on bionic, 2021-02-08) due to the lack of Coccinelle
being available on 20.04 (which continues today).
Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 23 Aug 2022 06:48:36 +0000 (06:48 +0000)]
t4301: add more interesting merge-tree testcases
This adds several tests of `merge-tree -z` extended conflict output
behavior to the testsuite, including some tests adapted from t6422.
These tests mark current behavior, not necessarily optimal behavior. In
particular, some path_msg() calls might want to include additional
paths.
These testcases also make something clear about the <Conflicted file>
info section of the output. That section consists of a sequence of
lines of the form
<mode> <object> <stage> <filename>
where <stage> is always greater than 0 (since each line comes from a
conflicted file). The lines correspond to conflicts that would be
placed in the index if we were doing a merge in a working tree. It is
perhaps natural to assume that for any given line, the <object> and
<filename> correspond to a single <revision>:<filename> pair from one of
the commits being merged (or from the merge base). This is true for
simple conflicts. However, these testcases make it clear that this is
not the case in general. For example, <object> may be the hash of a
three-way content merge of three different files (and with different
filenames).
The tests no longer pass under TEST_PASSES_SANITIZE_LEAK; it appears
that doing a directory rename with "git mv", among other possible
problems, triggers issues.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 23 Aug 2022 02:42:21 +0000 (02:42 +0000)]
merge: small code readability improvement
After our loop through the selected strategies, we compare best_strategy
to wt_strategy. This is fine, but the fact that the code setting
best_strategy sets it to use_strategies[i]->name requires a little bit
of extra checking to determine that at the time of setting, that's the
same as wt_strategy. Just setting best_strategy to wt_strategy makes it
a little easier to verify what the loop is doing, at least for this
reader.
Further, use_strategies[i]->name is used in a number of places, where we
could just use wt_strategy. The latter takes less time for this reader
to parse (one variable name instead of three), so just use wt_strategy
to make the code slightly faster for human readers to parse.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 23 Aug 2022 02:42:20 +0000 (02:42 +0000)]
merge: cleanup confusing logic for handling successful merges
builtin/merge.c has a loop over the specified strategies, where if they
all fail with conflicts, it picks the one with the least number of
conflicts.
In the codepath that finds a successful merge, if an automatic commit
was wanted, the code breaks out of the above loop, which makes sense.
However, if the user requested there be no automatic commit, the loop
would continue. That seems weird; --no-commit should not affect the
choice of merge strategy, but the code as written makes one think it
does. However, since the loop itself embeds "!merge_was_ok" as a
condition on continuing to loop, it actually would also exit early if
--no-commit was specified, it just exited from a different location.
Restructure the code slightly to make it clear that the loop will
immediately exit whenever we find a merge strategy that is successful.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 23 Aug 2022 02:42:19 +0000 (02:42 +0000)]
merge: only apply autostash when appropriate
If a merge failed and we are leaving conflicts in the working directory
for the user to resolve, we should not attempt to apply any autostash.
Further, if we fail to apply the autostash (because either the merge
failed, or the user requested --no-commit), then we should instruct the
user how to apply it later.
Add a testcase verifying we have corrected this behavior.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Tue, 23 Aug 2022 09:57:33 +0000 (11:57 +0200)]
promisor-remote: fix xcalloc() argument order
Pass the number of elements first and their size second, as expected
by xcalloc().
Patch generated with:
make SPATCH_FLAGS=--recursive-includes contrib/coccinelle/xcalloc.cocci.patch
Our default SPATCH_FLAGS ('--all-includes') doesn't catch this
transformation by default, unless used in combination with a large-ish
SPATCH_BATCH_SIZE which happens to put 'promisor-remote.c' with a file
that includes 'repository.h' directly in the same batch.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Derrick Stolee [Tue, 23 Aug 2022 14:05:13 +0000 (10:05 -0400)]
clone: warn on failure to repo_init()
The --bundle-uri option was added in 55568919616 (clone: add
--bundle-uri option, 2022-08-09), but this also introduced a call to
repo_init() whose return value was ignored. Fix that ignored value by
warning that the bundle URI process could not continue if it failed.
Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Anthony Delannoy [Mon, 22 Aug 2022 21:15:07 +0000 (23:15 +0200)]
preload-index: fix memleak
Fix a memory leak occuring in case of pathspec copy in preload_index.
Direct leak of 8 byte(s) in 8 object(s) allocated from:
#0 0x7f0a353ead47 in __interceptor_malloc (/usr/lib/gcc/x86_64-pc-linux-gnu/11.3.0/libasan.so.6+0xb5d47)
#1 0x55750995e840 in do_xmalloc /home/anthony/src/c/git/wrapper.c:51
#2 0x55750995e840 in xmalloc /home/anthony/src/c/git/wrapper.c:72
#3 0x55750970f824 in copy_pathspec /home/anthony/src/c/git/pathspec.c:684
#4 0x557509717278 in preload_index /home/anthony/src/c/git/preload-index.c:135
#5 0x55750975f21e in refresh_index /home/anthony/src/c/git/read-cache.c:1633
#6 0x55750915b926 in cmd_status builtin/commit.c:1547
#7 0x5575090e1680 in run_builtin /home/anthony/src/c/git/git.c:466
#8 0x5575090e1680 in handle_builtin /home/anthony/src/c/git/git.c:720
#9 0x5575090e284a in run_argv /home/anthony/src/c/git/git.c:787
#10 0x5575090e284a in cmd_main /home/anthony/src/c/git/git.c:920
#11 0x5575090dbf82 in main /home/anthony/src/c/git/common-main.c:56
#12 0x7f0a348230ab (/lib64/libc.so.6+0x290ab)
Signed-off-by: Anthony Delannoy <anthony.2lannoy@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 22 Aug 2022 19:50:49 +0000 (15:50 -0400)]
midx.c: avoid adding preferred objects twice
The last commit changes the behavior of midx.c's `get_sorted_objects()`
function to handle the case of writing a MIDX bitmap while reusing an
existing MIDX and changing the identity of the preferred pack
separately.
As part of this change, all objects from the (new) preferred pack are
added to the fanout table in a separate pass. Since these copies of the
objects all have their preferred bits set, any duplicates will be
resolved in their favor.
Importantly, this includes any copies of those same objects that come
from the existing MIDX. We know at the time of adding them that they'll
be redundant if their source pack is the (new) preferred one, so we can
avoid adding them to the list in this case.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 22 Aug 2022 19:50:46 +0000 (15:50 -0400)]
midx.c: include preferred pack correctly with existing MIDX
This patch resolves an issue where the object order used to generate a
MIDX bitmap would violate an invariant that all of the preferred pack's
objects are represented by that pack in the MIDX.
The problem arises when reusing an existing MIDX while generating a new
one, and occurs specifically when the identity of the preferred pack
changes from one MIDX to another, along with a few other conditions:
- the new preferred pack must also be present in the existing MIDX
- the new preferred pack must *not* have been the preferred pack in
the existing MIDX
- most importantly, there must be at least one object present in the
physical preferred pack (ie., it shows up in that pack's index)
but was selected from a *different* pack when the previous MIDX
was generated
When the above conditions are all met, we end up (incorrectly)
discarding copies of some objects in the pack selected as the preferred
pack. This is because `get_sorted_entries()` adds objects to its list
by doing the following at each fanout level:
- first, adding all objects from that fanout level from an existing
MIDX
- then, adding all objects from that fanout level in each pack *not*
included in the existing MIDX
So if some object was not selected from the to-be-preferred pack when
writing the previous MIDX, then we will never consider it as a candidate
when generating the new MIDX. This means that it's possible for the
preferred pack to not include all of its objects in the MIDX's
pseudo-pack object order, which is an invariant violation of that order.
Resolve this by adding all objects from the preferred pack separately
when it appears in the existing MIDX (if one was present). This will
duplicate objects from that pack that *did* appear in the MIDX, but this
is fine, since get_sorted_entries() already handles duplicates. (A
future optimization in this area could avoid adding copies of objects
that we know already existing in the MIDX.)
Note that we no longer need to compute the preferred-ness of objects
added from the MIDX, since we only want to select the preferred objects
from a single source. (We could still mark these preferred bits, but
doing so is redundant and unnecessary).
This resolves the bug demonstrated by t5326.174 ("preferred pack change
with existing MIDX bitmap").
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 22 Aug 2022 19:50:43 +0000 (15:50 -0400)]
midx.c: extract `midx_fanout_add_pack_fanout()`
Extract a routine to add all objects whose object ID's first byte is
`cur_fanout` from a given pack (identified by its index into the `struct
pack_info` array maintained by the MIDX writing routine).
Unlike the previous extraction (for `midx_fanout_add_midx_fanout()`),
this function will be called twice, once for all new packs, and again
for the preferred pack (if it appears in an existing MIDX). The latter
change is to resolve the bug described a few patches ago, and will be
made in the subsequent commit.
Similar to the previous refactoring, this function also enhances the
readability of its caller in `get_sorted_entries()`.
Its functionality is unchanged in this commit.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 22 Aug 2022 19:50:41 +0000 (15:50 -0400)]
midx.c: extract `midx_fanout_add_midx_fanout()`
Extract a routine to add all objects whose object ID's first byte is
`cur_fanout` from an existing MIDX.
This function will only be called once, so extracting it is purely
cosmetic to improve the readability of `get_sorted_entries()` (its sole
caller) below.
The functionality is unchanged in this commit.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 22 Aug 2022 19:50:38 +0000 (15:50 -0400)]
midx.c: extract `struct midx_fanout`
To build up a list of objects (along with their packs, and the offsets
within those packs that each object appears at), the MIDX code
implements `get_sorted_entries()` which builds up a list of candidates,
sorts them, and then removes duplicate entries.
To do this, it keeps an array of `pack_midx_entry` structures that it
builds up once for each fanout level (ie., for all possible values of
the first byte of each object's ID).
This array is a function-local variable of `get_sorted_entries()`. Since
it uses the ALLOC_GROW() macro, having the `alloc_fanout` variable also
be local to that function, and only modified within that function is
convenient.
However, subsequent changes will extract the two ways this array is
filled (from a pack at some fanout value, and from an existing MIDX at
some fanout value) into separate functions. Instead of passing around
pointers to the entries array, along with `nr_fanout` and
`alloc_fanout`, encapsulate these three into a structure instead. Then
pass around a pointer to this structure instead.
This patch does not yet extract the above two functions, but sets us up
to begin doing so in the following commit. For now, the implementation
of get_sorted_entries() is only modified to replace `entries_by_fanout`
with `fanout.entries`, `nr_fanout` with `fanout.nr`, and so on.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 22 Aug 2022 19:50:35 +0000 (15:50 -0400)]
t/lib-bitmap.sh: avoid silencing stderr
The midx_bitmap_partial_tests() function is responsible for setting up a
state where some (but not all) packs in the repository are covered by a
MIDX (and bitmap).
This function has redirected the `git multi-pack-index write --bitmap`'s
stderr to a file "err" since its introduction back in c51f5a6437 (t5326:
test multi-pack bitmap behavior, 2021-08-31).
This was likely a stray change left over from a slightly different
version of this test, since the file "err" is never read after being
written. This leads to confusingly-missing output, especially when the
contents of stderr are important.
Resolve this confusion by avoiding silencing stderr in this case.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 22 Aug 2022 19:50:32 +0000 (15:50 -0400)]
t5326: demonstrate potential bitmap corruption
It is possible to generate a corrupt MIDX bitmap when certain conditions
are met. This happens when the preferred pack "P" changes to one (say,
"Q") that:
- "Q" has objects included in an existing MIDX,
- but "Q" is different than "P",
- and "Q" and "P" have some objects in common
When this is the case, not all objects from "Q" will be selected from
"Q" (ie., the generated MIDX will represent them as coming from a
different pack), despite "Q" being preferred.
This is an invariant violation, since all objects contained in the
MIDX's preferred pack are supposed to originate from the preferred pack.
In other words, all duplicate objects are resolved in favor of the copy
that comes from the MIDX's preferred pack, if any.
This violation results in a corrupt object order, which cannot be
interpreted by the pack-bitmap code, leading to broken clones and other
defects.
This test demonstrates the above problem by constructing a minimal
reproduction, and showing that the final `git clone` invocation fails.
The reproduction is mostly straightforward, except that the new pack
generated between MIDX writes (which is necessary in order to prevent
that operation from being a noop) must sort ahead of all existing packs
in order to prevent a different pack (neither "P" nor "Q") from
appearing as preferred (meaning all its objects appear in order at the
beginning of the pseudo-pack order).
Subsequent commits will first refactor the midx.c::get_sorted_entries()
function, and then fix this bug.
Reported-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine [Mon, 22 Aug 2022 18:26:42 +0000 (18:26 +0000)]
t: detect and signal failure within loop
Failures within `for` and `while` loops can go unnoticed if not detected
and signaled manually since the loop itself does not abort when a
contained command fails, nor will a failure necessarily be detected when
the loop finishes since the loop returns the exit code of the last
command it ran on the final iteration, which may not be the command
which failed. Therefore, detect and signal failures manually within
loops using the idiom `|| return 1` (or `|| exit 1` within subshells).
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine [Mon, 22 Aug 2022 18:26:41 +0000 (18:26 +0000)]
t1092: fix buggy sparse "blame" test
This test wants to verify that `git blame` errors out when asked to
blame a file _not_ in the sparse checkout. However, the very first file
it asks to blame _is_ present in the checkout, thus `test_must_fail git
blame $file` gives an unexpected result (the "blame" succeeds). This
problem went unnoticed because the test invokes `test_must_fail git
blame $file` in loop but forgets to break out of the loop early upon
failure, thus the failure gets swallowed.
Fix the test by having it not ask to blame a file present in the sparse
checkout, and instead only blame files not present, as intended. While
at it, also add the missing `|| return 1` which allowed this bug to go
unnoticed.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Eric Sunshine [Mon, 22 Aug 2022 18:26:40 +0000 (18:26 +0000)]
t2407: fix broken &&-chains in compound statement
The breaks in the &&-chain in this test went unnoticed because the
"magic exit code 117" &&-chain checker built into test-lib.sh only
recognizes broken &&-chains at the top-level; it does not work within
`{...}` groups, `(...)` subshells, `$(...)` substitutions, or within
bodies of compound statements, such as `if`, `for`, `while`, `case`,
etc. Furthermore, `chainlint.sed` detects broken &&-chains only in
`(...)` subshells. Thus, the &&-chain breaks in this test fall into the
blind spots of the &&-chain linters.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sat, 20 Aug 2022 07:36:25 +0000 (03:36 -0400)]
xdiff: drop unused mmfile parameters from xdl_do_patience_diff()
The entry point to the patience-diff algorithm takes two mmfile_t
structs with the original file contents, but it doesn't actually do
anything useful with them. This is similar to the case recently cleaned
up in the histogram code via f1d019071e (xdiff: drop unused mmfile
parameters from xdl_do_histogram_diff(), 2022-08-19), but there's a bit
more subtlety going on.
We pass them into the recursive patience_diff(), which in turn passes
them into fill_hashmap(), which stuffs the pointers into a struct. But
the only thing which reads the struct fields is our recursion into
patience_diff()!
So it's unlikely that something like -Wunused-parameter could find this
case: it would have to detect the circular dependency caused by the
recursion (not to mention tracing across struct field assignments).
But once found, it's easy to have the compiler confirm what's going on:
1. Drop the "file1" and "file2" fields from the hashmap struct
definition. Remove the assignments in fill_hashmap(), and
temporarily substitute NULL in the recursive call to
patience_diff(). Compiling shows that no other code touched those
fields.
2. Now fill_hashmap() will trigger -Wunused-parameter. Drop "file1"
and "file2" from its definition and callsite.
3. Now patience_diff() will trigger -Wunused-parameter. Drop them
there, too. One of the callsites is the recursion with our
NULL values, so those temporary values go away.
4. Now xdl_do_patience_diff() will trigger -Wunused-parameter. Drop
them there. And we're done.
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 19 Aug 2022 08:55:02 +0000 (04:55 -0400)]
reflog: assert PARSE_OPT_NONEG in parse-options callbacks
In the spirit of 517fe807d6 (assert NOARG/NONEG behavior of
parse-options callbacks, 2018-11-05), this asserts that our callbacks
were invoked using the right flags (since otherwise they'd segfault on
the NULL arg). Both cases are already correct here, so this is mostly
about annotating the functions, and appeasing -Wunused-parameters.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 19 Aug 2022 08:54:56 +0000 (04:54 -0400)]
reftable: drop unused parameter from reader_seek_linear()
The reader code passes around a "struct reftable_reader" context
variable. But the seek function doesn't need it; the table iterator we
already get is sufficient.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sat, 20 Aug 2022 09:02:48 +0000 (05:02 -0400)]
verify_one_sparse(): drop unused parameters
This function has never used its repository or cache_tree parameters
since it was introduced in 9ad2d5ea71 (sparse-index: loose integration
with cache_tree_verify(), 2021-03-30).
As that commit notes, it may eventually be extended further, and that
might require looking at more data. But we can easily add them back if
necessary (and the repository is even included in the index_state these
days already). In the mean time, dropping them makes the code shorter
and appeases -Wunused-parameter.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye [Fri, 19 Aug 2022 20:49:08 +0000 (20:49 +0000)]
p0004: fix prereq declaration
Fix multi-threaded 'p0004' test's use of the 'REPO_BIG_ENOUGH_FOR_MULTI'
prerequisite. Unlike normal 't/' tests, 't/perf/' tests need to have their
prerequisites declared with the '--prereq' flag.
Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Michael J Gruber [Thu, 18 Aug 2022 13:13:28 +0000 (15:13 +0200)]
sequencer: do not translate command names
When action_name is used to denote a command `git %s` do not translate
since command names are never translated.
Suggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Michael J Gruber <git@grubix.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Michael J Gruber [Thu, 18 Aug 2022 13:13:27 +0000 (15:13 +0200)]
sequencer: do not translate parameters to error_resolve_conflict()
`error_resolve_conflict()` checks the untranslated action_name
parameter, so pass it as is.
Suggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Michael J Gruber <git@grubix.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Michael J Gruber [Thu, 18 Aug 2022 13:13:26 +0000 (15:13 +0200)]
sequencer: do not translate reflog messages
Traditionally, reflog messages were never translated, in particular not
on storage.
Due to the switch of more parts of git to the sequencer, old changes in
the sequencer code may lead to recent changes in git's behaviour. E.g.: c28cbc5ea6 ("sequencer: mark action_name() for translation", 2016-10-21)
marked several uses of `action_name()` for translation. Recently, this
lead to a partially translated reflog:
`rebase: fast-forward` is translated (e.g. in de to `Rebase: Vorspulen`)
whereas other reflog entries such as `rebase (pick):` remain
untranslated as they should be.
Change the relevant line in the sequencer so that this reflog entry
remains untranslated, as well.
Signed-off-by: Michael J Gruber <git@grubix.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 19 Aug 2022 08:50:54 +0000 (04:50 -0400)]
match_pathname(): drop unused "flags" parameter
This field has not been used since the function was introduced in b559263216 (exclude: split pathname matching code into a separate
function, 2012-10-15), though there was a brief period where it was
erroneously used and then reverted in ed4958477b (dir: fix pattern
matching on dirs, 2021-09-24) and 5ceb663e92 (dir: fix
directory-matching bug, 2021-11-02).
It's possible we'd eventually add a flag that makes it useful here, but
there are only a handful of callers. It would be easy to add back if
necessary, and in the meantime this makes the function interface less
misleading.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 19 Aug 2022 08:50:33 +0000 (04:50 -0400)]
log-tree: drop unused commit param in remerge_diff()
This function has never used its "commit" parameter since it was added
in db757e8b8d (show, log: provide a --remerge-diff capability,
2022-02-02).
This makes sense; we already have separate parameters for the parents
(which lets us redo the merge) and the oid of the result tree (which we
can then diff against the remerge result).
Let's drop the unused parameter in the name of clarity.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 19 Aug 2022 08:49:41 +0000 (04:49 -0400)]
xdiff: drop unused mmfile parameters from xdl_do_histogram_diff()
These are no longer used since 9df0fc3d57 (xdiff: fix a memory leak,
2022-02-16), as the caller is expected to call xdl_prepare_env() itself.
After that change the histogram code only examines the prepared
xdfenv_t, not the original buffers.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:11 +0000 (18:04 +0200)]
builtin/worktree.c: let parse-options parse subcommands
'git worktree' parses its subcommands with a long list of if
statements. parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
handling missing or unknown subcommands, and listing subcommands for
Bash completion.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:10 +0000 (18:04 +0200)]
builtin/stash.c: let parse-options parse subcommands
'git stash' parses its subcommands with a long list of if-else if
statements. parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
and listing subcommands for Bash completion.
Note that the push_stash() function implementing the 'push' subcommand
accepts an extra flag parameter to indicate whether push was assumed,
so add a wrapper function with the standard subcommand function
signature.
Note also that this change "hides" the '-h' option in 'git stash push
-h' from the parse_option() call in cmd_stash(), as it comes after the
subcommand. Consequently, from now on it will emit the usage of the
'push' subcommand instead of the usage of 'git stash'. We had a
failing test for this case, which can now be flipped to expect
success.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:09 +0000 (18:04 +0200)]
builtin/sparse-checkout.c: let parse-options parse subcommands
'git sparse-checkout' parses its subcommands with a couple of if
statements. parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
handling missing or unknown subcommands, and listing subcommands for
Bash completion.
Note that some of the functions implementing each subcommand only
accept the 'argc' and '**argv' parameters, so add a (unused) '*prefix'
parameter to make them match the type expected by parse-options, and
thus avoid casting function pointers.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:08 +0000 (18:04 +0200)]
builtin/remote.c: let parse-options parse subcommands
'git remote' parses its subcommands with a long list of if-else if
statements. parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
handling unknown subcommands, and listing subcommands for Bash
completion. Make sure that the default operation mode doesn't accept
any arguments; and while at it remove the capitalization of the error
message and adjust the test checking it accordingly.
Note that 'git remote' has both 'remove' and 'rm' subcommands, and the
former is preferred [1], so hide the latter for completion.
Note also that the functions implementing each subcommand only accept
the 'argc' and '**argv' parameters, so add a (unused) '*prefix'
parameter to make them match the type expected by parse-options, and
thus avoid casting a bunch of function pointers.
[1] e17dba8fe1 (remote: prefer subcommand name 'remove' to 'rm',
2012-09-06)
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:07 +0000 (18:04 +0200)]
builtin/reflog.c: let parse-options parse subcommands
'git reflog' parses its subcommands with a couple of if-else if
statements. parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
and listing subcommands for Bash completion.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:06 +0000 (18:04 +0200)]
builtin/notes.c: let parse-options parse subcommands
'git notes' parses its subcommands with a long list of if-else if
statements. parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
handling unknown subcommands, and listing subcommands for Bash
completion. Make sure that the default operation mode doesn't accept
any arguments.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:05 +0000 (18:04 +0200)]
builtin/multi-pack-index.c: let parse-options parse subcommands
'git multi-pack-index' parses its subcommands with a couple of if-else
if statements. parse-options has just learned to parse subcommands,
so let's use that facility instead, with the benefits of shorter code,
handling missing or unknown subcommands, and listing subcommands for
Bash completion.
Note that the functions implementing each subcommand only accept the
'argc' and '**argv' parameters, so add a (unused) '*prefix' parameter
to make them match the type expected by parse-options, and thus avoid
casting function pointers.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:04 +0000 (18:04 +0200)]
builtin/hook.c: let parse-options parse subcommands
'git hook' parses its currently only subcommand with an if statement.
parse-options has just learned to parse subcommands, so let's use that
facility instead, with the benefits of shorter code, handling missing
or unknown subcommands, and listing subcommands for Bash completion.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:03 +0000 (18:04 +0200)]
builtin/gc.c: let parse-options parse 'git maintenance's subcommands
'git maintenanze' parses its subcommands with a couple of if
statements. parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
handling missing or unknown subcommands, and listing subcommands for
Bash completion.
This change makes 'git maintenance' consistent with other commands in
that the help text shown for '-h' goes to standard output, not error,
in the exit code and error message on unknown subcommand, and the
error message on missing subcommand. There is a test checking these,
which is now updated accordingly.
Note that some of the functions implementing each subcommand don't
accept any parameters, so add the (unused) 'argc', '**argv' and
'*prefix' parameters to make them match the type expected by
parse-options, and thus avoid casting function pointers.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:02 +0000 (18:04 +0200)]
builtin/commit-graph.c: let parse-options parse subcommands
'git commit-graph' parses its subcommands with an if-else if
statement. parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
handling missing or unknown subcommands, and listing subcommands for
Bash completion.
Note that the functions implementing each subcommand only accept the
'argc' and '**argv' parameters, so add a (unused) '*prefix' parameter
to make them match the type expected by parse-options, and thus avoid
casting function pointers.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:01 +0000 (18:04 +0200)]
builtin/bundle.c: let parse-options parse subcommands
'git bundle' parses its subcommands with a couple of if-else if
statements. parse-options has just learned to parse subcommands, so
let's use that facility instead, with the benefits of shorter code,
handling missing or unknown subcommands, and listing subcommands for
Bash completion.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor [Fri, 19 Aug 2022 16:04:00 +0000 (18:04 +0200)]
parse-options: add support for parsing subcommands
Several Git commands have subcommands to implement mutually exclusive
"operation modes", and they usually parse their subcommand argument
with a bunch of if-else if statements.
Teach parse-options to handle subcommands as well, which will result
in shorter and simpler code with consistent error handling and error
messages on unknown or missing subcommand, and it will also make
possible for our Bash completion script to handle subcommands
programmatically.
The approach is guided by the following observations:
- Most subcommands [1] are implemented in dedicated functions, and
most of those functions [2] either have a signature matching the
'int cmd_foo(int argc, const char **argc, const char *prefix)'
signature of builtin commands or can be trivially converted to
that signature, because they miss only that last prefix parameter
or have no parameters at all.
- Subcommand arguments only have long form, and they have no double
dash prefix, no negated form, and no description, and they don't
take any arguments, and can't be abbreviated.
- There must be exactly one subcommand among the arguments, or zero
if the command has a default operation mode.
- All arguments following the subcommand are considered to be
arguments of the subcommand, and, conversely, arguments meant for
the subcommand may not preceed the subcommand.
So in the end subcommand declaration and parsing would look something
like this:
Here each OPT_SUBCOMMAND specifies the name of the subcommand and the
function implementing it, and the address of the same 'fn' subcommand
function pointer. parse_options() then processes the arguments until
it finds the first argument matching one of the subcommands, sets 'fn'
to the function associated with that subcommand, and returns, leaving
the rest of the arguments unprocessed. If none of the listed
subcommands is found among the arguments, parse_options() will show
usage and abort.
If a command has a default operation mode, 'fn' should be initialized
to the function implementing that mode, and parse_options() should be
invoked with the PARSE_OPT_SUBCOMMAND_OPTIONAL flag. In this case
parse_options() won't error out when not finding any subcommands, but
will return leaving 'fn' unchanged. Note that if that default
operation mode has any --options, then the PARSE_OPT_KEEP_UNKNOWN_OPT
flag is necessary as well (otherwise parse_options() would error out
upon seeing the unknown option meant to the default operation mode).
Some thoughts about the implementation:
- The same pointer to 'fn' must be specified as 'value' for each
OPT_SUBCOMMAND, because there can be only one set of mutually
exclusive subcommands; parse_options() will BUG() otherwise.
There are other ways to tell parse_options() where to put the
function associated with the subcommand given on the command line,
but I didn't like them:
- Change parse_options()'s signature by adding a pointer to
subcommand function to be set to the function associated with
the given subcommand, affecting all callsites, even those that
don't have subcommands.
- Introduce a specific parse_options_and_subcommand() variant
with that extra funcion parameter.
- I decided against automatically calling the subcommand function
from within parse_options(), because:
- There are commands that have to perform additional actions
after option parsing but before calling the function
implementing the specified subcommand.
- The return code of the subcommand is usually the return code
of the git command, but preserving the return code of the
automatically called subcommand function would have made the
API awkward.
- Also add a OPT_SUBCOMMAND_F() variant to allow specifying an
option flag: we have two subcommands that are purposefully
excluded from completion ('git remote rm' and 'git stash save'),
so they'll have to be specified with the PARSE_OPT_NOCOMPLETE
flag.
- Some of the 'parse_opt_flags' don't make sense with subcommands,
and using them is probably just an oversight or misunderstanding.
Therefore parse_options() will BUG() when invoked with any of the
following flags while the options array contains at least one
OPT_SUBCOMMAND:
- PARSE_OPT_KEEP_DASHDASH: parse_options() stops parsing
arguments when encountering a "--" argument, so it doesn't
make sense to expect and keep one before a subcommand, because
it would prevent the parsing of the subcommand.
However, this flag is allowed in combination with the
PARSE_OPT_SUBCOMMAND_OPTIONAL flag, because the double dash
might be meaningful for the command's default operation mode,
e.g. to disambiguate refs and pathspecs.
- PARSE_OPT_STOP_AT_NON_OPTION: As its name suggests, this flag
tells parse_options() to stop as soon as it encouners a
non-option argument, but subcommands are by definition not
options... so how could they be parsed, then?!
- PARSE_OPT_KEEP_UNKNOWN: This flag can be used to collect any
unknown --options and then pass them to a different command or
subsystem. Surely if a command has subcommands, then this
functionality should rather be delegated to one of those
subcommands, and not performed by the command itself.
However, this flag is allowed in combination with the
PARSE_OPT_SUBCOMMAND_OPTIONAL flag, making possible to pass
--options to the default operation mode.
- If the command with subcommands has a default operation mode, then
all arguments to the command must preceed the arguments of the
subcommand.
AFAICT we don't have any commands where this makes a difference,
because in those commands either only the command accepts any
arguments ('notes' and 'remote'), or only the default subcommand
('reflog' and 'stash'), but never both.
- The 'argv' array passed to subcommand functions currently starts
with the name of the subcommand. Keep this behavior. AFAICT no
subcommand functions depend on the actual content of 'argv[0]',
but the parse_options() call handling their options expects that
the options start at argv[1].
- To support handling subcommands programmatically in our Bash
completion script, 'git cmd --git-completion-helper' will now list
both subcommands and regular --options, if any. This means that
the completion script will have to separate subcommands (i.e.
words without a double dash prefix) from --options on its own, but
that's rather easy to do, and it's not much work either, because
the number of subcommands a command might have is rather low, and
those commands accept only a single --option or none at all. An
alternative would be to introduce a separate option that lists
only subcommands, but then the completion script would need not
one but two git invocations and command substitutions for commands
with subcommands.
Note that this change doesn't affect the behavior of our Bash
completion script, because when completing the --option of a
command with subcommands, e.g. for 'git notes --<TAB>', then all
subcommands will be filtered out anyway, as none of them will
match the word to be completed starting with that double dash
prefix.
[1] Except 'git rerere', because many of its subcommands are
implemented in the bodies of the if-else if statements parsing the
command's subcommand argument.
[2] Except 'credential', 'credential-store' and 'fsmonitor--daemon',
because some of the functions implementing their subcommands take
special parameters.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This doesn't matter for the completion script, because field splitting
discards that space anyway.
However, later patches in this series will teach parse-options to
handle subcommands, and subcommands will be included in the completion
helper output as well. This will make the loop printing options (and
subcommands) a tad more complex, so I wanted to test the result. The
test would have to account for the presence of that leading space,
which bugged my OCD, so let's get rid of it.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
