Bugfix for the recently graduated "git rebase --rebase-merges".
* js/rebase-r-and-merge-head:
status: rebase and merge can be in progress at the same time
built-in rebase --skip/--abort: clean up stale .git/<name> files
rebase -i: include MERGE_HEAD into files to clean up
rebase -r: do not write MERGE_HEAD unless needed
rebase -r: demonstrate bug with conflicting merges
When editing a patch in a "git add -i" session, a hunk could be
made to no-op. The "git apply" program used to reject a patch with
such a no-op hunk to catch user mistakes, but it is now updated to
explicitly allow a no-op hunk in an edited patch.
"git rebase --autostash" did not correctly re-attach the HEAD at times.
* js/rebase-autostash-detach-fix:
built-in rebase --autostash: leave the current branch alone if possible
built-in rebase: demonstrate regression with --autostash
The "--no-patch" option, which can be used to get a high-level
overview without the actual line-by-line patch difference shown, of
the "range-diff" command was earlier broken, which has been
corrected.
* ab/range-diff-no-patch:
range-diff: make diff option behavior (e.g. --stat) consistent
range-diff: fix regression in passing along diff options
range-diff doc: add a section about output stability
Various functions have been audited for "-Wunused-parameter" warnings
and bugs in them got fixed.
* jk/unused-parameter-fixes:
midx: double-check large object write loop
assert NOARG/NONEG behavior of parse-options callbacks
parse-options: drop OPT_DATE()
apply: return -1 from option callback instead of calling exit(1)
cat-file: report an error on multiple --batch options
tag: mark "--message" option with NONEG
show-branch: mark --reflog option as NONEG
format-patch: mark "--no-numbered" option with NONEG
status: mark --find-renames option with NONEG
cat-file: mark batch options with NONEG
pack-objects: mark index-version option as NONEG
ls-files: mark exclude options as NONEG
am: handle --no-patch-format option
apply: mark include/exclude options as NONEG
range-diff: make diff option behavior (e.g. --stat) consistent
Make the behavior when diff options (e.g. "--stat") are passed
consistent with how "diff" behaves.
Before 73a834e9e2 ("range-diff: relieve callers of low-level
configuration burden", 2018-07-22) running range-diff with "--stat"
would produce stat output and the diff output, as opposed to how
"diff" behaves where once "--stat" is specified "--patch" also needs
to be provided to emit the patch output.
As noted in a previous change ("range-diff doc: add a section about
output stability", 2018-11-07) the "--stat" output with "range-diff"
is useless at the moment.
But we should behave consistently with "diff" in anticipation of such
output being useful in the future, because it would make for confusing
UI if "diff" and "range-diff" behaved differently when it came to how
they interpret diff options.
The new behavior is also consistent with the existing documentation
added in ba931edd28 ("range-diff: populate the man page",
2018-08-13). See "[...]also accepts the regular diff options[...]" in
git-range-diff(1).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The procedure to install dependencies before testing at Travis CI
is getting revamped for both simplicity and flexibility, taking
advantage of the recent move to the vm-based environment.
* sg/travis-install-dependencies:
travis-ci: install packages in 'ci/install-dependencies.sh'
"git add" needs to internally run "diff-files" equivalent, and the
codepath learned the same optimization as "diff-files" has to run
lstat(2) in parallel to find which paths have been updated in the
working tree.
* bp/add-diff-files-optim:
add: speed up cmd_add() by utilizing read_cache_preload()
The interface into "xdiff" library used to discover the offset and
size of a generated patch hunk by first formatting it into the
textual hunk header "@@ -n,m +k,l @@" and then parsing the numbers
out. A new interface has been introduced to allow callers a more
direct access to them.
* jk/xdiff-interface:
xdiff-interface: drop parse_hunk_header()
range-diff: use a hunk callback
diff: convert --check to use a hunk callback
combine-diff: use an xdiff hunk callback
diff: use hunk callback for word-diff
diff: discard hunk headers for patch-ids earlier
diff: avoid generating unused hunk header lines
xdiff-interface: provide a separate consume callback for hunks
xdiff: provide a separate emit callback for hunks
Assorted fixes for bugs found while auditing -Wunused-parameter
warnings.
* jk/misc-unused-fixes:
approxidate: fix NULL dereference in date_time()
pathspec: handle non-terminated strings with :(attr)
approxidate: handle pending number for "specials"
rev-list: handle flags for --indexed-objects
The code to traverse objects for reachability, used to decide what
objects are unreferenced and expendable, have been taught to also
consider per-worktree refs of other worktrees as starting points to
prevent data loss.
* nd/per-worktree-ref-iteration:
git-worktree.txt: correct linkgit command name
reflog expire: cover reflog from all worktrees
fsck: check HEAD and reflog from other worktrees
fsck: move fsck_head_link() to get_default_heads() to avoid some globals
revision.c: better error reporting on ref from different worktrees
revision.c: correct a parameter name
refs: new ref types to make per-worktree refs visible to all worktrees
Add a place for (not) sharing stuff between worktrees
refs.c: indent with tabs, not spaces
The helper function to refresh the cached stat information in the
in-core index has learned to perform the lstat() part of the
operation in parallel on multi-core platforms.
* bp/refresh-index-using-preload:
refresh_index: remove unnecessary calls to preload_index()
speed up refresh_index() by utilizing preload_index()
"git send-email --transfer-encoding=..." in recent versions of Git
sometimes produced an empty "Content-Transfer-Encoding:" header,
which has been corrected.
* al/send-email-auto-cte-fixup:
send-email: avoid empty transfer encoding header
In preparation to the day when we can deprecate and remove the
"rebase -p", make sure we can skip and later remove tests for
it.
* js/rebase-p-tests:
tests: optionally skip `git rebase -p` tests
t3418: decouple test cases from a previous `rebase -p` test case
t3404: decouple some test cases from outcomes of previous test cases
"git rev-parse --exclude=* --branches --branches" (i.e. first
saying "add only things that do not match '*' out of all branches"
and then adding all branches, without any exclusion this time")
worked as expected, but "--exclude=* --all --all" did not work the
same way, which has been fixed.
* ag/rev-parse-all-exclude-fix:
rev-parse: clear --exclude list after 'git rev-parse --all'
The submodule support has been updated to read from the blob at
HEAD:.gitmodules when the .gitmodules file is missing from the
working tree.
* ao/submodule-wo-gitmodules-checked-out:
t/helper: add test-submodule-nested-repo-config
submodule: support reading .gitmodules when it's not in the working tree
submodule: add a helper to check if it is safe to write to .gitmodules
t7506: clean up .gitmodules properly before setting up new scenario
submodule: use the 'submodule--helper config' command
submodule--helper: add a new 'config' subcommand
t7411: be nicer to future tests and really clean things up
t7411: merge tests 5 and 6
submodule: factor out a config_set_in_gitmodules_file_gently function
submodule: add a print_config_from_gitmodules() helper
* nb/worktree-api-doc:
worktree: rename is_worktree_locked to worktree_lock_reason
worktree: update documentation for lock_reason and lock_reason_valid
A couple of tests used to leave the repository in a state that is
deliberately corrupt, which have been corrected.
* ab/pack-tests-cleanup:
index-pack tests: don't leave test repo dirty at end
pack-objects tests: don't leave test .git corrupt at end
pack-objects test: modernize style
Tests for the recently introduced multi-pack index machinery.
* ds/test-multi-pack-index:
packfile: close multi-pack-index in close_all_packs
multi-pack-index: define GIT_TEST_MULTI_PACK_INDEX
midx: close multi-pack-index on repack
midx: fix broken free() in close_midx()
A pattern with '**' that does not have a slash on either side used
to be an invalid one, but the code now treats such double-asterisks
the same way as two normal asterisks that happen to be adjacent to
each other.
* nd/wildmatch-double-asterisk:
wildmatch: change behavior of "foo**bar" in WM_PATHNAME mode
A fourth class of configuration files (in addition to the
traditional "system wide", "per user in the $HOME directory" and
"per repository in the $GIT_DIR/config") has been introduced so
that different worktrees that share the same repository (hence the
same $GIT_DIR/config file) can use different customization.
* nd/per-worktree-config:
worktree: add per-worktree config files
t1300: extract and use test_cmp_config()
"git ls-remote $there foo" was broken by recent update for the
protocol v2 and stopped showing refs that match 'foo' that are not
refs/{heads,tags}/foo, which has been fixed.
* jk/proto-v2-ref-prefix-fix:
ls-remote: pass heads/tags prefixes to transport
ls-remote: do not send ref prefixes for patterns
Split the overly large Documentation/config.txt file into million
little pieces. This potentially allows each individual piece
included into the manual page of the command it affects more easily.
* nd/config-split: (81 commits)
config.txt: remove config/dummy.txt
config.txt: move worktree.* to a separate file
config.txt: move web.* to a separate file
config.txt: move versionsort.* to a separate file
config.txt: move user.* to a separate file
config.txt: move url.* to a separate file
config.txt: move uploadpack.* to a separate file
config.txt: move uploadarchive.* to a separate file
config.txt: move transfer.* to a separate file
config.txt: move tag.* to a separate file
config.txt: move submodule.* to a separate file
config.txt: move stash.* to a separate file
config.txt: move status.* to a separate file
config.txt: move splitIndex.* to a separate file
config.txt: move showBranch.* to a separate file
config.txt: move sequencer.* to a separate file
config.txt: move sendemail-config.txt to config/
config.txt: move reset.* to a separate file
config.txt: move rerere.* to a separate file
config.txt: move repack.* to a separate file
...
built-in rebase: reinstate `checkout -q` behavior where appropriate
When we converted a `git checkout -q $onto^0` call to use
`reset_head()`, we inadvertently incurred a change from a twoway_merge
to a oneway_merge, as if we wanted a `git reset --hard` instead.
This has performance ramifications under certain, though, as the
oneway_merge needs to lstat() every single index entry whereas
twoway_merge does not.
So let's go back to the old behavior.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently, we only accept the flag indicating whether the HEAD should be
detached not. In the next commit, we want to introduce another flag: to
toggle between emulating `reset --hard` vs `checkout -q`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
`--exclude` from rev-list and rev-parse fails to exclude references if
the next `--branches`, `--tags` or `--remotes` use the optional
inclusive glob because those options are implemented as particular cases
of `--glob=`, which itself requires that exclude patterns begin with
'refs/'.
But it makes sense for `--branches=glob` and friends to be aware that
exclusions patterns for them shouldn't be 'refs/<type>/' prefixed, the
same way exclude patterns for `--branches` and friends (without the
optional glob) already are.
Let's record in 'refs.c:struct ref_filter' which context the exclude
pattern is tied to, so refs.c:filter_refs() can decide if it should
ignore the prefix when trying to match.
Signed-off-by: Rafael Ascensão <rafa.almas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs: show --exclude failure with --branches/tags/remotes=glob
The documentation of `--exclude=` option from rev-list and rev-parse
explicitly states that exclude patterns *should not* start with 'refs/'
when used with `--branches`, `--tags` or `--remotes`.
However, following this advice results in refereces not being excluded
if the next `--branches`, `--tags`, `--remotes` use the optional
inclusive glob.
Demonstrate this failure.
Signed-off-by: Rafael Ascensão <rafa.almas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When editing patches e.g. in `git add -e`, it is quite common that a
hunk ends up having no -/+ lines, i.e. it is now supposed to do nothing.
This use case was broken by ad6e8ed37bc1 (apply: reject a hunk that does
not do anything, 2015-06-01) with the good intention of catching a very
real, different issue in hand-edited patches.
So let's use the `--recount` option as the tell-tale whether the user
would actually be okay with no-op hunks.
Add a test case to make sure that this use case does not regress again.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
status: rebase and merge can be in progress at the same time
Since `git rebase -r` was introduced, that is possible. But our
machinery did not think that possible, and failed to say anything about
the rebase in progress when in the middle of a merge.
Let's work around that in the minimal fashion.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
built-in rebase --skip/--abort: clean up stale .git/<name> files
The scripted version of the rebase used to execute `git reset --hard`
when skipping or aborting. When we ported this to C, we did update the
worktree and some reflogs, but we failed to imitate `git reset --hard`'s
behavior regarding files in .git/ such as MERGE_HEAD.
Let's address this oversight.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rebase -i: include MERGE_HEAD into files to clean up
Every once in a while, the interactive rebase makes sure that no stale
files are lying around. These days, we need to include MERGE_HEAD into
that set of files, as the `merge` command will generate them.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we detect that a `merge` can be skipped because the merged commit
is already an ancestor of HEAD, we do not need to commit, therefore
writing the MERGE_HEAD file is useless.
It is actually worse than useless: a subsequent `git commit` will pick
it up and think that we want to merge that commit, still.
To avoid that, move the code that writes the MERGE_HEAD file to a
location where we already know that the `merge` cannot be skipped.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rebase -r: demonstrate bug with conflicting merges
When calling `merge` on a branch that has already been merged, that
`merge` is skipped quietly, but currently a MERGE_HEAD file is being
left behind and will then be grabbed by the next `pick` (that did
not want to create a *merge* commit).
Demonstrate this.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
511726e4b1 ("builtin/notes: fix premature failure when trying to add
the empty blob", 2014-11-09) removed the check for !len but left a
call to free the buffer that will be otherwise NULL
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Acked-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch makes the output of `git shortlog -nse v2.10.0..master`
duplicate-free by taking/guessing the current and preferred
addresses for authors that appear with more than one address.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
range-diff: fix regression in passing along diff options
In 73a834e9e2 ("range-diff: relieve callers of low-level configuration
burden", 2018-07-22) we broke passing down options like --no-patch,
--stat etc.
Fix that regression, and add a test asserting the pre-73a834e9e2
behavior for some of these diff options.
As noted in a change leading up to this ("range-diff doc: add a
section about output stability", 2018-11-07) the output is not meant
to be stable. So this regression test will likely need to be tweaked
once we get a "proper" --stat option.
See
https://public-inbox.org/git/nycvar.QRO.7.76.6.1811071202480.39@tvgsbejvaqbjf.bet/
for a further explanation of the regression. The fix here is not the
same as in Johannes's on-list patch, for reasons that'll be explained
in a follow-up commit.
The quoting of "EOF" here mirrors that of an earlier test. Perhaps
that should be fixed, but let's leave that up to a later cleanup
change.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
range-diff doc: add a section about output stability
The range-diff command is already advertised as porcelain, but let's
make it really clear that the output is completely subject to change,
particularly when it comes to diff options such as --stat. Right now
that option doesn't work, but fixing that is the subject of a later
change.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
built-in rebase --autostash: leave the current branch alone if possible
When we converted a `git reset --hard` call in the original Unix shell
script to built-in code, we asked to reset the worktree and the index
and explicitly *not* to detach the HEAD. By mistake, though, we still
did. Let's fix this.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
built-in rebase: demonstrate regression with --autostash
An unnamed colleague of Ævar Arnfjörð Bjarmason reported a breakage
where a `pull --rebase` (which did not really need to do anything but
stash, see that nothing was changed, and apply the stash again) also
detached the HEAD.
This patch adds a minimal reproducer for this regression.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Windows: force-recompile git.res for differing architectures
When git.rc is compiled into git.res, the result is actually dependent
on the architecture. That is, you cannot simply link a 32-bit git.res
into a 64-bit git.exe.
Therefore, to allow 32-bit and 64-bit builds in the same directory, we
let git.res depend on GIT-PREFIX so that it gets recompiled when
compiling for a different architecture (this works because the exec path
changes based on the architecture: /mingw32/libexec/git-core for 32-bit
and /mingw64/libexec/git-core for 64-bit).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we see a time like "noon", we pass "12" to our date_time() helper,
which sets the hour to 12pm. If the current time is before noon, then we
wrap around to yesterday using date_yesterday(). But unlike the normal
calls to date_yesterday() from approxidate_alpha(), we pass a NULL "num"
parameter. Since c27cc94fad (approxidate: handle pending number for
"specials", 2018-11-02), that causes a segfault.
One way to fix this is by checking for NULL. But arguably date_time() is
abusing our helper by passing NULL in the first place (and this is the
only case where one of these "special" parsers is used this way). So
instead, let's have it just do the 1-day subtraction itself. It's still
just a one-liner due to our update_tm() helper.
Note that the test added here is a little funny, as we say "10am noon",
which makes the "10am" seem pointless. But this bug can only be
triggered when it the currently-parsed hour is before the special time.
The latest special time is "tea" at 1700, but t0006 uses a hard-coded
TEST_DATE_NOW of 1900. We could reset TEST_DATE_NOW, but that may lead
to confusion in other tests. Just saying "10am noon" makes this test
self-contained.
Reported-by: Carlo Arenas <carenas@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pull: handle --verify-signatures for unborn branch
We usually just forward the --verify-signatures option along to
git-merge, and trust it to do the right thing. However, when we are on
an unborn branch (i.e., there is no HEAD yet), we handle this case
ourselves without even calling git-merge. And in this code path, we do
not respect the verification option at all.
It may be more maintainable in the long run to call git-merge for the
unborn case. That would fix this bug, as well as prevent similar ones in
the future. But unfortunately it's not easy to do. As t5520.3
demonstrates, there are some special cases that git-merge does not
handle, like "git pull .. master:master" (by the time git-merge is
invoked, we've overwritten the unborn HEAD).
So for now let's just teach git-pull to handle this feature.
Reported-by: Felix Eckhofer <felix@eckhofer.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge: handle --verify-signatures for unborn branch
When git-merge sees that we are on an unborn branch (i.e., there is no
HEAD), it follows a totally separate code path than the usual merge
logic. This code path does not know about verify_signatures, and so we
fail to notice bad or missing signatures.
This has been broken since --verify-signatures was added in efed002249
(merge/pull: verify GPG signatures of commits being merged, 2013-03-31).
In an ideal world, we'd unify the flow for this case with the regular
merge logic, which would fix this bug and avoid introducing similar
ones. But because the unborn case is so different, it would be a burden
on the rest of the function to continually handle the missing HEAD. So
let's just port the verification check to this special case.
Reported-by: Felix Eckhofer <felix@eckhofer.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The logic to implement "merge --verify-signatures" is inline in
cmd_merge(), but this site misses some cases. Let's extract the logic
into a function so we can call it from more places.
We'll move it to commit.[ch], since one of the callers (git-pull) is
outside our source file. This function isn't all that general (after
all, its main function is to exit the program) but it's not worth trying
to fix that. The heavy lifting is done by check_commit_signature(), and
our purpose here is just sharing the die() logic. We'll mark it with a
comment to make that clear.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Windows port learned to use nano-second resolution file timestamps.
* js/mingw-ns-filetime:
mingw: implement nanosecond-precision file times
mingw: replace MSVCRT's fstat() with a Win32-based implementation
mingw: factor out code to set stat() data
Operations on promisor objects make sense in the context of only a
small subset of the commands that internally use the revisions
machinery, but the "--exclude-promisor-objects" option were taken
and led to nonsense results by commands like "log", to which it
didn't make much sense. This has been corrected.
* md/exclude-promisor-objects-fix:
exclude-promisor-objects: declare when option is allowed
Documentation/git-log.txt: do not show --exclude-promisor-objects
"git send-email" learned to disable SMTP authentication via the
"--smtp-auth=none" option, even when the smtp username is given
(which turns the authentication on by default).
The command line completion machinery (in contrib/) has been
updated to allow the completion script to tweak the list of options
that are reported by the parse-options machinery correctly.
* nd/completion-negation:
completion: fix __gitcomp_builtin no longer consider extra options
"git fetch" over protocol v2 into a shallow repository failed to
fetch full history behind a new tip of history that was diverged
before the cut-off point of the history that was previously fetched
shallowly.
* jt/upload-pack-v2-fix-shallow:
upload-pack: clear flags before each v2 request
upload-pack: make want_obj not global
upload-pack: make have_obj not global
Some codepaths failed to form a proper URL when .gitmodules record
the URL to a submodule repository as relative to the repository of
superproject, which has been corrected.
* sb/submodule-url-to-absolute:
submodule helper: convert relative URL to absolute URL if needed
"git repack" in a shallow clone did not correctly update the
shallow points in the repository, leading to a repository that
does not pass fsck.
* js/shallow-and-fetch-prune:
repack -ad: prune the list of shallow commits
shallow: offer to prune only non-existing entries
repack: point out a bug handling stale shallow info
completion: use __gitcomp_builtin for format-patch
This helps format-patch gain completion for a couple new options,
notably --range-diff.
Since send-email completion relies on $__git_format_patch_options
which is now reduced, we need to do something not to regress
send-email completion.
The workaround here is implement --git-completion-helper in
send-email.perl just as a bridge to "format-patch --git-completion-helper".
This is enough to use __gitcomp_builtin on send-email (to take
advantage of caching).
In the end, send-email.perl can probably reuse the same info it passes
to GetOptions() to generate full --git-completion-helper output so
that we don't need to keep track of its options in git-completion.bash
anymore. But that's something for another boring day.
Helped-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The write_midx_large_offsets() function takes an array of object
entries, the number of entries in the array (nr_objects), and the number
of entries with large offsets (nr_large_offset). But we never actually
use nr_objects; instead we keep walking down the array and counting down
nr_large_offset until we've seen all of the large entries.
This is correct, but we can be a bit more defensive. If there were ever
a mismatch between nr_large_offset and the actual set of large-offset
objects, we'd walk off the end of the array.
Since we know the size of the array, we can use nr_objects to make sure
we don't walk too far.
Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
assert NOARG/NONEG behavior of parse-options callbacks
When we define a parse-options callback, the flags we put in the option
struct must match what the callback expects. For example, a callback
which does not handle the "unset" parameter should only be used with
PARSE_OPT_NONEG. But since the callback and the option struct are not
defined next to each other, it's easy to get this wrong (as earlier
patches in this series show).
Fortunately, the compiler can help us here: compiling with
-Wunused-parameters can show us which callbacks ignore their "unset"
parameters (and likewise, ones that ignore "arg" expect to be triggered
with PARSE_OPT_NOARG).
But after we've inspected a callback and determined that all of its
callers use the right flags, what do we do next? We'd like to silence
the compiler warning, but do so in a way that will catch any wrong calls
in the future.
We can do that by actually checking those variables and asserting that
they match our expectations. Because this is such a common pattern,
we'll introduce some helper macros. The resulting messages aren't
as descriptive as we could make them, but the file/line information from
BUG() is enough to identify the problem (and anyway, the point is that
these should never be seen).
Each of the annotated callbacks in this patch triggers
-Wunused-parameters, and was manually inspected to make sure all callers
use the correct options (so none of these BUGs should be triggerable).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are no users of OPT_DATE except for test-parse-options; its
only caller went away in 27ec394a97 (prune: introduce OPT_EXPIRY_DATE()
and use it, 2013-04-25).
It also has a bug: it does not specify PARSE_OPT_NONEG, but its callback
does not respect the "unset" flag, and will feed NULL to approxidate()
and segfault. Probably this should be marked with NONEG, or the callback
should set the timestamp to some sentinel value (e.g,. "0", or
"(time_t)-1").
But since there are no callers, deleting it means we don't even have to
think about what the right behavior should be.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
apply: return -1 from option callback instead of calling exit(1)
The option callback for "apply --whitespace" exits with status "1" on
error. It makes more sense for it to just return an error to
parse-options. That code will exit, too, but it will use status "129"
that is customary for option errors.
The exit() dates back to aaf6c447aa (builtin/apply: make
parse_whitespace_option() return -1 instead of die()ing, 2016-08-08).
That commit gives no reason why we'd prefer the current exit status (it
looks like it was just bumping the "die" up a level in the callstack,
but did not go as far as it could have).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
cat-file: report an error on multiple --batch options
The options callback for --batch and --batch-check detects when the two
mutually incompatible options are used. But it simply returns an error
code to parse-options, meaning the program will quit without any kind of
message to the user.
Instead, let's use error() to print something and return -1. Note that
this flips the error return from 1 to -1, but negative values are more
idiomatic here (and parse-options treats them the same).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We do not allow "--no-message" to work now, as the option callback
returns "-1" when it sees a NULL arg. However, that will cause
parse-options to exit(129) without printing anything further, leaving
the user confused about what happened.
Instead, let's explicitly mark it as PARSE_OPT_NONEG, which will give a
useful error message (and print the usual -h output).
In theory this could be used to override an earlier "-m", but it's not
clear how it would interact with other message options (e.g., would it
also clear data read for "-F"?). Since it's already disabled and nobody
is asking for it, let's punt on that and just improve the error message.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running "git show-branch --no-reflog" will behave as if "--reflog" was
given with no options, which makes no sense.
In theory this option might be used to cancel an earlier "--reflog"
option, but the semantics are not clear. Let's punt on it and just
disallow the broken option.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
format-patch: mark "--no-numbered" option with NONEG
We have separate parse-options entries for "numbered" and "no-numbered",
which means that we accept "--no-no-numbered". It does not behave
sensibly, though (it ignores the "unset" flag and acts like
"--no-numbered").
We could fix that, but obviously this is silly and unintentional. Let's
just disallow it.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If you run "git status --no-find-renames", it will behave the same as
"--find-renames", because we ignore the "unset" parameter (we see a NULL
"arg", but since the score argument is optional, we just think that the
user did not provide a score).
We already have a separate "--no-renames" to disable renames, so there's
not much point in supporting "--no-find-renames". Let's just flag it as
an error.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running "cat-file --no-batch" will behave as if "--batch" was given,
since the option callback does not handle the "unset" flag (likewise for
"--no-batch-check").
In theory this might be used to cancel an earlier --batch, but it's not
immediately obvious how that would interact with --batch-check. Let's
just disallow the negated form of both options.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running "git pack-objects --no-index-version" will segfault, since the
callback is not prepared to handle the "unset" flag.
In theory this might be used to counteract an earlier "--index-version",
or override a pack.indexversion config setting. But the semantics aren't
immediately obvious, and it's unlikely anybody wants this. Let's just
disable the broken option for now.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running "git ls-files --no-exclude" will currently segfault, as its
option callback does not handle the "unset" parameter.
In theory this could be used to clear the exclude list, but it is not
clear how that would interact with the other exclude options, nor is the
current code capable of clearing the list. Let's just disable the broken
option.
Note that --no-exclude-from will similarly segfault, but
--no-exclude-standard will not. It just silently does the wrong thing
(pretending as if --exclude-standard was specified).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running "git am --no-patch-format" will currently segfault, since it
tries to parse a NULL argument. Instead, let's have it cancel any
previous --patch-format option.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>