The argument parser used by setup_revisions() modifies the argv array
that is passed to it, consolidating non-options and unknown options at
the start of the array. This led to problems with memory leaks when argv
pointed to allocated strings. We addressed that in cd43948798 (revision:
manage memory ownership of argv in setup_revisions(), 2025-09-19). Now
instead of copying strings to the earlier part of argv, we actually move
them, setting the original location to NULL (so that we know we have
exactly one pointer to the string).
This works fine for setup_revisions() itself, but the underlying
handle_revision_opt() has another entry point: parse_revision_opt().
This lets a parse-options user parse a single revision option, but the
movement introduced by cd43948798 confuses its error code path. If we
see an unknown option, then handle_revision_opt() will move it out of
the way (to the "unknown options" section) and return an error. But
parse_revision_opt() then tries to access the original argv location,
which has now been set to NULL, and you get:
$ git shortlog -n --no-such-option
error: unknown option `(null)'
Whereas prior to cd43948798, it would have been a leftover copy of the
pointer (that may or may not eventually get written over, but was valid
for this immediate message). And you get what you'd expect:
$ git shortlog -n --no-such-option
error: unknown option `--no-such-option'
Making things even more confusing, it only happens if there's another
option before the unknown one! That's because with just:
$ git shortlog --no-such-option
we "consolidate" to the exact same spot, and no movement occurs at all.
Note that we use shortlog in these examples because it is one of only
two commands that use the parse_revision_opt() interface (the other is
blame).
There are a few options for fixing this. One is that we can observe that
the "move" semantics introduced by cd43948798 only matter when the argv
strings are allocated on the heap, in which case the caller passes in
the free_removed_argv_elements flag to tell us. But we never use that
flag with parse_revision_opt(). So we could do something like this:
diff --git a/revision.c b/revision.c
index ee1df92d1d..7b858d54c1 100644
--- a/revision.c
+++ b/revision.c
@@ -2340,7 +2340,8 @@ static void overwrite_argv(int *argc, const char **argv,
if (*value != argv[*argc]) {
mark_argv_for_free(opt, revs, argv[*argc]);
argv[*argc] = *value;
- *value = NULL;
+ if (opt && opt->free_removed_argv_elements)
+ *value = NULL;
}
(*argc)++;
}
to restore the pre-cd43948798 semantics when heap-allocated strings are
not in use. We'd just keep the extra pointer in the original location,
but nobody cares because they're not going to free anything anyway.
That's enough to fix this case, and could fix any other theoretical
cases we haven't noticed. The downside is that it's an accident waiting
to happen if we ever do teach parse_revision_opt() to handle allocated
argv strings.
But are there other theoretical cases? I don't think so. The code paths
touched by cd43948798 are either in setup_revisions() itself (which also
learned how to handle this movement) or in handle_revision_opt(), the
low-level static helper. It has only two callers: setup_revisions()
itself, and parse_revision_opt() in which we see the current breakage.
So fixing parse_revision_opt() should cover all of our bases, and keep
the code ready for a potential future change to handle allocated
strings.
The fix is just to tell parse_revision_opt() to look for the unknown
option in the consolidated destination rather than the original
location. We might write to that consolidated location for other
reasons (like moving pseudo-revision options like "--all"), but there is
only one code path that returns the 0 for an unknown option, and it
always moves the option before doing so. So the "end" of that
consolidated area will always have our unknown option.
This patch implements that solution and demonstrates the breakage and
fix using shortlog.
Reported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In parse_revision_opt(), we report "unknown option" when
handle_revision_opt() returns a negative value or zero. But these are
two different conditions: a negative value indicates a malformed option
(like a missing argument) and zero indicates an unknown option.
So malformed options produce nonsense like this:
$ git shortlog --default
error: bad --default argument
error: unknown option `--default'
usage: [...]
We should treat a negative return as a logic error which has already
been reported by handle_revision_opt(), and just show the regular usage
message.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We replaced deprecated macos-13 with macos-14 image in GitHub
Actions CI, but we forgot that the image is for arm64. We have
been seeing a lot of test failures ever since. Switch to arm64
binary for Perforce tests.
* jc/ci-use-arm64-p4-on-macos:
Use Perforce arm64 binary on macOS CI jobs
The previous step replaced deprecated macos-13 image with macos-14
image on GitHub Actions CI. While x86-64 binaries can work there,
because macos-14 images are arm64 based (we could replace it with
macos-14-large that is x86-64), it makes more sense to use arm64
binary there. Without this change, we have been getting unusually
higher rate of failures from random macOS CI jobs railing to run
t98xx series of tests.
Helped-by: Koji Nakamaru <koji.nakamaru@gree.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Introduced via aea86cf00f (The nineteenth batch, 2025-10-14).
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git last-modified" was optimized by narrowing the set of paths to
follow as it dug deeper in the history.
* tc/last-modified-active-paths-optimization:
last-modified: implement faster algorithm
Building "git contacts" script (in contrib/) left the resulting
file unexecutable, which has been corrected.
* dk/make-git-contacts-executable:
perl: also mark git-contacts executable
The build procedure based on meson learned to allow builders to
specify the directory to install HTML documents.
* dk/meson-html-dir:
meson: make GIT_HTML_PATH configurable
Build procedure for Wincred credential helper has been updated.
* tu/credential-wincred-makefile-update:
wincred: align Makefile with other Makefiles in contrib
Refreshes the Irish translation for Git 2.52, including new strings and
consistency improvements. Verified with `git-po-helper check`.
Signed-off-by: Aindriú Mac Giolla Eoin <aindriu80@gmail.com>
A recently added configuration variable and command line option
syntax ":(optional)" for values that are of filename type
inconsistently behaved on an empty file (configuration took it
happily, while the command line option pretended as if it did not
exist), which has been corrected.
* dk/parseopt-optional-filename-fixes:
parseopt: remove unreachable code
parseopt: restore const qualifier to parsed filename
config: use boolean type for a simple flag
parseopt: use boolean type for a simple flag
doc: clarify command equivalence comment
parseopt: fix :(optional) at command line to only ignore missing files
Messages from fast-import/export are now marked for i18n.
* cc/fast-import-export-i18n-cleanup:
gpg-interface: mark a string for translation
fast-import: mark strings for translation
fast-export: mark strings for translation
gpg-interface: use left shift to define GPG_VERIFY_*
gpg-interface: simplify ssh fingerprint parsing
Makefile-based builds can configure Git's internal HTML_PATH by defining
htmldir, which is useful for packagers that put documentation in
different locations. Gentoo, for example, uses version-suffixed
directories like ${prefix}/share/doc/git-2.51 and puts the HTML
documentation in an 'html' subdirectory of the same.
Propagate the same configuration knob to Meson-based builds so that
"git --html-path" on such systems can be configured to output the
correct directory.
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When installing git-contacts with Meson via -Dcontrib=contacts, the default
Perl generation fails to mark it executable. As a result, "git contacts"
reports "'contacts' is not a git command."
Unlike generate-script.sh, we aren't testing the basename here; so, glob
the script name in the case arm to match wherever the input comes from.
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Replace $(LOADLIBES) because it is deprecated since long and it is
used nowhere else in the git project.
* Use $(gitexecdir) instead of $(libexecdir) because config.mak defines
$(libexecdir) as $(prefix)/libexec, not as $(prefix)/libexec/git-core.
* Similar to other Makefiles, let install target rule create
$(gitexecdir) to make sure the directory exists before copying the
executable and also let it respect $(DESTDIR).
* Shuffle the lines for the default settings to align them with the
other Makefiles in contrib/credential.
* Define .PHONY for all special targets (all, install, clean).
Signed-off-by: Thomas Uhle <thomas.uhle@mailbox.tu-dresden.de>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The version of macos image used in GitHub CI has been updated to
macos-14, as the macos-13 that we have been using got deprecated.
* jc/ci-use-macos-14:
GitHub CI: macos-13 images are no more
The help text and manual page of "git bisect" command have been
made consistent with each other.
* rz/t0450-bisect-doc-update:
bisect: update usage and docs to match each other
As this image was deprecated on Sep 22nd, and will be dropped on Dec
4th, replace these jobs to use macos-14 images instead.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
At this point in the code after running skip_prefix() on the
variable and receiving the result in the same variable, the contents
of the variable can never be NULL. The function either (1) updates
the variable to point at a later part of the string it originally
pointed at, or (2) leaves it intact if the string does not have the
prefix. (1) will never make the variable NULL, and (2) cannot be
the source of NULL, because the variable cannot be NULL before
calling skip_prefix(), which would die immediately by dereferencing
the NULL pointer in that case.
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This was unintentionally dropped in ccfcaf399f (parseopt: values of
pathname type can be prefixed with :(optional), 2025-09-28). Notably,
continue dropping the const qualifier when free'ing value; see
4049b9cfc0 (fix const issues with some functions, 2007-10-16) or
83838d5c1b (cast variable in call to free() in builtin/diff.c and
submodule.c, 2011-11-06) for more details on why.
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation of command parsing for :(optional) includes a terse
comment; expand it to be clearer to readers.
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Unlike the configuration option magic, the parseopt code also ignores
empty files: compare implementations from ccfcaf399f (parseopt: values
of pathname type can be prefixed with :(optional), 2025-09-28) and
749d6d166d (config: values of pathname type can be prefixed with
:(optional), 2025-09-28).
Unify the 2 by not ignoring empty files, which is less surprising and
the intended semantics from the first patch for config.
Suggested-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King