Jeff King [Fri, 24 Feb 2023 06:39:31 +0000 (01:39 -0500)]
notes: mark unused callback parameters
for_each_note() requires a callback, but not all callbacks need all of
the parameters. Likewise, init_notes() takes a callback to implement the
"combine" strategy, but the "ignore" variant obviously doesn't look at
its arguments at all. Mark unused parameters as appropriate to silence
compiler warnings.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:39:27 +0000 (01:39 -0500)]
prio-queue: mark unused parameters in comparison functions
The prio_queue_compare_fn interface has a void pointer to allow callers
to pass arbitrary data, but most comparison functions don't need it.
Mark those cases to make -Wunused-parameter happy.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:39:24 +0000 (01:39 -0500)]
for_each_object: mark unused callback parameters
The for_each_{loose,packed}_object interface uses callback functions,
but not every callback needs all of the parameters. Mark the unused ones
to satisfy -Wunused-parameter.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:39:22 +0000 (01:39 -0500)]
list-objects: mark unused callback parameters
Our graph-traversal functions take callbacks for showing commits and
objects, but not all callbacks need each parameter. Likewise for the
similar traverse_bitmap_commit_list(), which has a different interface
but serves the same purpose. And the include_check mechanism, which
passes along a void pointer which is not always used.
Mark the unused ones to to make -Wunused-parameter happy.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:39:20 +0000 (01:39 -0500)]
mark unused parameters in signal handlers
Signal handlers receive their signal number as a parameter, but many
don't care what it is (because they only handle one signal, or because
their action is the same regardless of the signal). Mark such parameters
to silence -Wunused-parameter.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:39:18 +0000 (01:39 -0500)]
run-command: mark error routine parameters as unused
After forking but before exec-ing a command, we install special
error/warn/die handlers in the child. These ignore the error messages
they get, since the idea is that they shouldn't be called in the first
place.
Arguably they could pass along that error message _in addition_ to
saying "error() should not be called in a child", but since the whole
point is to avoid any conflicts on stdio/malloc locks, etc, we're better
to just keep these simple. Seeing them trigger is effectively a bug, and
the developer is probably better off grabbing a stack trace.
But we do want to mark the functions so that -Wunused-parameter doesn't
complain.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:39:15 +0000 (01:39 -0500)]
mark "pointless" data pointers in callbacks
Both the object_array_filter() and trie_find() functions use callback
functions that let the caller specify which elements match. These
callbacks take a void pointer in case the caller wants to pass in extra
data. But in each case, the single user of these functions just passes
NULL, and the callback ignores the extra pointer.
We could just remove these unused parameters from the callback interface
entirely. But it's good practice to provide such a pointer, as it guides
future callers of the function in the right direction (rather than
tempting them to access global data). Plus it's consistent with other
generic callback interfaces.
So let's instead annotate the unused parameters, in order to silence the
compiler's -Wunused-parameter warning.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:39:06 +0000 (01:39 -0500)]
ref-filter: mark unused callback parameters
The ref-filter code uses virtual functions to handle specific atoms, but
many of the functions ignore some parameters:
- most atom parsers do not need the ref_format itself, unless they are
looking at centralized options like use_color, quote_style, etc.
- meta-atom handlers like append_atom(), align_atom_handler(), etc,
can't generate errors, so ignore their "err" parameter
- likewise, the handlers for then/else/end do not even need to look at
their atom_value, as the "if" handler put everything they need into
the ref_formatting_state stack
Since these functions all have to conform to virtual function
interfaces, we can't just drop the unused parameters, but must mark them
as UNUSED (to appease -Wunused-parameter).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:38:43 +0000 (01:38 -0500)]
http-backend: mark unused parameters in virtual functions
The http-backend dispatches requests via a table of virtual functions.
Some of the functions ignore their "arg" parameter, because it's
implicit in the function (e.g., get_info_refs knows that it is
dispatched only for a request to "/info/refs").
Mark these unused parameters to silence -Wunused-parameter.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:38:35 +0000 (01:38 -0500)]
http-backend: mark argc/argv unused
We can't drop them because it's cmd_main(), which has a set prototype,
but the CGI interface does not do anything with such arguments.
Arguably we could detect them and complain. It's possible this could
detect misconfigurations or other mistakes, but:
- as far as I can tell common webservers like apache do not have any
mechanism to pass arguments to a CGI at all, so this isn't a mistake
one could even make
- it's possible that some obscure webserver might pass arguments, and
we'd break that case. I have no idea if such a webserver exists; the
CGI standard says only "The script is invoked in a system-defined
manner".
So probably it would not hurt to detect them, but it also is unlikely to
help anyone. Let's just mark them as unused, which retains the current
behavior but silences -Wunused-parameter.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:38:30 +0000 (01:38 -0500)]
object-name: mark unused parameters in disambiguate callbacks
The object-name disambiguation code triggers a callback for each
possible object id we find. This is really used for two purposes:
- "hint" functions like disambiguate_commit_only report back on
whether the value is usable
- iterator functions like repo_for_each_abbrev() use it to collect
and report matching names.
Compiling with -Wunused-parameter generates several warnings, but
they're distinct for each type. The "hint" functions never look at the
void cb_data pointer; they only care whether the oid matches our hint.
The iterator functions never look at the "struct repository" parameter;
they're just reporting back the oids they see, and always return 0.
So arguably these could be two separate interfaces:
But doing so would complicate the disambiguation code, which now has to
accept and call the two different types. Since we can easily squelch the
compiler warnings by annotating the functions, let's just do that.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:38:23 +0000 (01:38 -0500)]
serve: mark unused parameters in virtual functions
Each v2 "serve" action has a virtual function for advertising and
implementing the command. A few of these are so trivial that they don't
need to look at their parameters, especially the "repository" parameter.
We can mark them so that -Wunused-parameter doesn't complain.
Note that upload_pack_v2() probably _should_ be using its repository
pointer. But teaching the functions it calls to do so is non-trivial.
Even using it for something as simple as reading config is tricky, both
because it shares code with the v1 upload pack, and because the
git_protected_config() mechanism it uses does not have a repo-specific
interface. So we'll just annotate it for now, and cleaning it up can be
part of the larger work to drop references to the_repository.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:38:10 +0000 (01:38 -0500)]
serve: use repository pointer to get config
A few of the v2 "serve" callbacks ignore their repository parameter and
read config using the_repository (either directly or implicitly by
calling wrapper functions). This isn't a bug since the server code only
handles a single main repository anyway (and indeed, if you look at the
callers, these repository parameters will always be the_repository). But
in the long run we want to get rid of the_repository, so let's take a
tiny step in that direction.
As a bonus, this silences some -Wunused-parameter warnings.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:37:22 +0000 (01:37 -0500)]
ls-refs: drop config caching
The code for the v2 ls-refs command has an ensure_config_read() function
that tries to read the lsrefs.unborn config only once and caches it in
some static global variables.
There's no real need for this caching. In any given process we'd only
need the value twice (once to decide whether to advertise, and once if
somebody runs the command). And since the config code already has its
own cache, each access is only incurring a hash lookup and string
comparison anyway.
Since the values we set are going to be specific to the_repository, the
globals we set are a mild anti-pattern. In practice it's not a bug (yet)
since the server-side v2 code only handles a single repository anyway.
But it doesn't hurt to take a small step in the right direction and
model a good approach.
Note that we currently set two booleans: advertise_unborn and
allow_unborn. But we can get away with a single value, since "advertise"
naturally implies "allow". That lets us just convert this to a function
with a return value.
Note that we still always read from the_repository; we'll deal with that
in a follow-on patch.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 06:34:44 +0000 (01:34 -0500)]
ref-filter: drop unused atom parameter from get_worktree_path()
The get_worktree_path() function is used to populate the %(worktreepath)
value, but it has never used its "atom" parameter since it was added in 2582083fa1 (ref-filter: add worktreepath atom, 2019-04-28).
Normally we'd use the atom struct to cache any work we do, but in this
case there's a global hashmap that does that caching already. So we can
just drop the unused parameter.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 22 Feb 2023 22:55:45 +0000 (14:55 -0800)]
Merge branch 'ab/hook-api-with-stdin'
Extend the run-hooks API to allow feeding data from the standard
input when running the hook script(s).
* ab/hook-api-with-stdin:
hook: support a --to-stdin=<path> option
sequencer: use the new hook API for the simpler "post-rewrite" call
hook API: support passing stdin to hooks, convert am's 'post-rewrite'
run-command: allow stdin for run_processes_parallel
run-command.c: remove dead assignment in while-loop
Junio C Hamano [Wed, 22 Feb 2023 22:55:45 +0000 (14:55 -0800)]
Merge branch 'ab/various-leak-fixes'
Leak fixes.
* ab/various-leak-fixes:
push: free_refs() the "local_refs" in set_refspecs()
push: refactor refspec_append_mapped() for subsequent leak-fix
receive-pack: release the linked "struct command *" list
grep API: plug memory leaks by freeing "header_list"
grep.c: refactor free_grep_patterns()
builtin/merge.c: free "&buf" on "Your local changes..." error
builtin/merge.c: use fixed strings, not "strbuf", fix leak
show-branch: free() allocated "head" before return
commit-graph: fix a parse_options_concat() leak
http-backend.c: fix cmd_main() memory leak, refactor reg{exec,free}()
http-backend.c: fix "dir" and "cmd_arg" leaks in cmd_main()
worktree: fix a trivial leak in prune_worktrees()
repack: fix leaks on error with "goto cleanup"
name-rev: don't xstrdup() an already dup'd string
various: add missing clear_pathspec(), fix leaks
clone: use free() instead of UNLEAK()
commit-graph: use free_commit_graph() instead of UNLEAK()
bundle.c: don't leak the "args" in the "struct child_process"
tests: mark tests as passing with SANITIZE=leak
Junio C Hamano [Thu, 16 Feb 2023 01:11:52 +0000 (17:11 -0800)]
Merge branch 'ab/sequencer-unleak'
Plug leaks in sequencer subsystem and its users.
* ab/sequencer-unleak:
commit.c: free() revs.commit in get_fork_point()
builtin/rebase.c: free() "options.strategy_opts"
sequencer.c: always free() the "msgbuf" in do_pick_commit()
builtin/rebase.c: fix "options.onto_name" leak
builtin/revert.c: move free-ing of "revs" to replay_opts_release()
sequencer API users: fix get_replay_opts() leaks
sequencer.c: split up sequencer_remove_state()
rebase: use "cleanup" pattern in do_interactive_rebase()
Junio C Hamano [Thu, 16 Feb 2023 01:11:52 +0000 (17:11 -0800)]
Merge branch 'ds/bundle-uri-5'
The bundle-URI subsystem adds support for creation-token heuristics
to help incremental fetches.
* ds/bundle-uri-5:
bundle-uri: test missing bundles with heuristic
bundle-uri: store fetch.bundleCreationToken
fetch: fetch from an external bundle URI
bundle-uri: drop bundle.flag from design doc
clone: set fetch.bundleURI if appropriate
bundle-uri: download in creationToken order
bundle-uri: parse bundle.<id>.creationToken values
bundle-uri: parse bundle.heuristic=creationToken
t5558: add tests for creationToken heuristic
bundle: verify using check_connected()
bundle: test unbundling with incomplete history
Junio C Hamano [Thu, 16 Feb 2023 01:11:51 +0000 (17:11 -0800)]
Merge branch 'cb/grep-fallback-failing-jit'
In an environment where dynamically generated code is prohibited to
run (e.g. SELinux), failure to JIT pcre patterns is expected. Fall
back to interpreted execution in such a case.
* cb/grep-fallback-failing-jit:
grep: fall back to interpreter if JIT memory allocation fails
Junio C Hamano [Tue, 14 Feb 2023 22:15:55 +0000 (14:15 -0800)]
Merge branch 'jk/unused-post-2.39' into maint-2.39
Code clean-up around unused function parameters.
* jk/unused-post-2.39:
userdiff: mark unused parameter in internal callback
list-objects-filter: mark unused parameters in virtual functions
diff: mark unused parameters in callbacks
xdiff: mark unused parameter in xdl_call_hunk_func()
xdiff: drop unused parameter in def_ff()
ws: drop unused parameter from ws_blank_line()
list-objects: drop process_gitlink() function
blob: drop unused parts of parse_blob_buffer()
ls-refs: use repository parameter to iterate refs
Junio C Hamano [Tue, 14 Feb 2023 22:15:51 +0000 (14:15 -0800)]
Merge branch 'pb/doc-orig-head' into maint-2.39
Document ORIG_HEAD a bit more.
* pb/doc-orig-head:
git-rebase.txt: add a note about 'ORIG_HEAD' being overwritten
revisions.txt: be explicit about commands writing 'ORIG_HEAD'
git-merge.txt: mention 'ORIG_HEAD' in the Description
git-reset.txt: mention 'ORIG_HEAD' in the Description
git-cherry-pick.txt: do not use 'ORIG_HEAD' in example
Junio C Hamano [Tue, 14 Feb 2023 22:15:51 +0000 (14:15 -0800)]
Merge branch 'ws/single-file-cone' into maint-2.39
The logic to see if we are using the "cone" mode by checking the
sparsity patterns has been tightened to avoid mistaking a pattern
that names a single file as specifying a cone.
* ws/single-file-cone:
dir: check for single file cone patterns
Junio C Hamano [Tue, 14 Feb 2023 22:15:50 +0000 (14:15 -0800)]
Merge branch 'jk/ext-diff-with-relative' into maint-2.39
"git diff --relative" did not mix well with "git diff --ext-diff",
which has been corrected.
* jk/ext-diff-with-relative:
diff: drop "name" parameter from prepare_temp_file()
diff: clean up external-diff argv setup
diff: use filespec path to set up tempfiles for ext-diff
Junio C Hamano [Tue, 14 Feb 2023 22:15:49 +0000 (14:15 -0800)]
Merge branch 'lk/line-range-parsing-fix' into maint-2.39
When given a pattern that matches an empty string at the end of a
line, the code to parse the "git diff" line-ranges fell into an
infinite loop, which has been corrected.
* lk/line-range-parsing-fix:
line-range: fix infinite loop bug with '$' regex
Junio C Hamano [Tue, 14 Feb 2023 22:15:49 +0000 (14:15 -0800)]
Merge branch 'rs/use-enhanced-bre-on-macos' into maint-2.39
Newer regex library macOS stopped enabling GNU-like enhanced BRE,
where '\(A\|B\)' works as alternation, unless explicitly asked with
the REG_ENHANCED flag. "git grep" now can be compiled to do so, to
retain the old behaviour.
* rs/use-enhanced-bre-on-macos:
use enhanced basic regular expressions on macOS
Junio C Hamano [Tue, 14 Feb 2023 22:15:49 +0000 (14:15 -0800)]
Merge branch 'jk/curl-avoid-deprecated-api' into maint-2.39
Deal with a few deprecation warning from cURL library.
* jk/curl-avoid-deprecated-api:
http: support CURLOPT_PROTOCOLS_STR
http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION
http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
Junio C Hamano [Tue, 14 Feb 2023 22:15:47 +0000 (14:15 -0800)]
Merge branch 'jk/avoid-redef-system-functions-2.30' into maint-2.39
Redefining system functions for a few functions did not follow our
usual "implement git_foo() and #define foo(args) git_foo(args)"
pattern, which has broken build for some folks.
* jk/avoid-redef-system-functions-2.30:
git-compat-util: undefine system names before redeclaring them
git-compat-util: avoid redefining system function names
Junio C Hamano [Tue, 14 Feb 2023 22:15:45 +0000 (14:15 -0800)]
Merge branch 'cw/ci-whitespace' into maint-2.39
CI updates. We probably want a clean-up to move the long shell
script embedded in yaml file into a separate file, but that can
come later.
* cw/ci-whitespace:
ci (check-whitespace): move to actions/checkout@v3
ci (check-whitespace): add links to job output
ci (check-whitespace): suggest fixes for errors
Jeff King [Sat, 11 Feb 2023 04:52:56 +0000 (23:52 -0500)]
doc/ls-remote: clarify pattern format
We document that you can specify "refs" to ls-remote, but we don't
explain any further than that they are "matched" as patterns. Since this
can be interpreted in a lot of ways, let's clarify that they are
tail-matched globs.
Likewise, let's use the word "patterns" to refer to them consistently,
rather than "refs" (both here and in the quick "-h" help), and mention
more explicitly that only one pattern needs to be matched (though there
is also an example already that shows this in action).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sat, 11 Feb 2023 02:44:13 +0000 (21:44 -0500)]
doc/ls-remote: cosmetic cleanups for examples
There are effectively three example commands and their output, but
they're smushed together with no extra whitespace. Let's add some blank
lines to make them more readable.
Likewise, the first example uses "./." to refer to the path of the
current repository, which is somewhat distracting. That may have been
necessary back in 2005 when it was added, but we can just say "." these
days.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Fri, 10 Feb 2023 20:20:30 +0000 (21:20 +0100)]
cache-tree: fix strbuf growth in prime_cache_tree_rec()
Use size_t to store the original length of the strbuf tree_len, as
that's the correct type.
Don't double the allocated size of the strbuf when adding a subdirectory
name. And the chance of the trailing slash fitting in the slack left by
strbuf_add() is very high, so stop pre-growing the strbuf at all.
Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Have the last users of "USE_THE_INDEX_COMPATIBILITY_MACROS" use the
underlying *_index() variants instead. Now all previous users of
"USE_THE_INDEX_COMPATIBILITY_MACROS" have been migrated away from the
wrapper macros, and if applicable to use the "USE_THE_INDEX_VARIABLE"
added in [1].
Let's leave the "index-compatibility.cocci" in place, even though it
won't be doing anything on "master". It will benefit any out-of-tree
code that need to use these compatibility macros. We can eventually
remove it.
1. bdafeae0b9c (cache.h & test-tool.h: add & use
"USE_THE_INDEX_VARIABLE", 2022-11-19)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove the redundant update_main_cache_tree() function, and make its
users use cache_tree_update() instead.
The behavior of populating the "the_index.cache_tree" if it wasn't
present already was needed when this function was introduced in [1],
but it hasn't been needed since [2]; The "cache_tree_update()" will
now lazy-allocate, so there's no need for the wrapper.
1. 996277c5206 (Refactor cache_tree_update idiom from commit,
2011-12-06)
2. fb0882648e0 (cache-tree: clean up cache_tree_update(), 2021-01-23)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*"
Add a trivial rule for "write_cache_as_tree" to
"index-compatibility.cocci", and apply it. This was left out of the
rules added in 0e6550a2c63 (cocci: add a
index-compatibility.pending.cocci, 2022-11-19) because this
compatibility wrapper lived in "cache-tree.h", not "cache.h"
But it's like the other "USE_THE_INDEX_COMPATIBILITY_MACROS", so let's
migrate it too.
The replacement of "USE_THE_INDEX_COMPATIBILITY_MACROS" here with
"USE_THE_INDEX_VARIABLE" is a manual change on top, now that these
files only use "&the_index", and don't need any compatibility
macros (or functions).
The wrapping of some argument lists is likewise manual, as coccinelle
would otherwise give us overly long argument lists.
The reason for putting the "O" in the cocci rule on the "-" and "+"
lines is because I couldn't get correct whitespacing otherwise,
i.e. I'd end up with "oid,&the_index", not "oid, &the_index".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apply the rule added in [1] to change "cache_name_pos" to
"index_name_pos", which allows us to get rid of another
"USE_THE_INDEX_COMPATIBILITY_MACROS" macro.
The replacement of "USE_THE_INDEX_COMPATIBILITY_MACROS" here with
"USE_THE_INDEX_VARIABLE" is a manual change on top, now that these
files only use "&the_index", and don't need any compatibility
macros (or functions).
1. 0e6550a2c63 (cocci: add a index-compatibility.pending.cocci,
2022-11-19)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
cocci & cache.h: fully apply "active_nr" part of index-compatibility
Apply the "active_nr" part of "index-compatibility.pending.cocci",
which was left out in [1] due to an in-flight conflict. As of [2] the
topic we conflicted with has been merged to "master", so we can fully
apply this rule.
builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"
Replace the "USE_THE_INDEX_COMPATIBILITY_MACROS" define with the
narrower "USE_THE_INDEX_VARIABLE". This could have been done in 07047d68294 (cocci: apply "pending" index-compatibility to some
"builtin/*.c", 2022-11-19), but I missed it at the time.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 9 Feb 2023 22:40:46 +0000 (14:40 -0800)]
Merge branch 'jk/httpd-test-updates'
Test update.
* jk/httpd-test-updates:
t/lib-httpd: increase ssl key size to 2048 bits
t/lib-httpd: drop SSLMutex config
t/lib-httpd: bump required apache version to 2.4
t/lib-httpd: bump required apache version to 2.2
Elijah Newren [Thu, 9 Feb 2023 09:11:46 +0000 (09:11 +0000)]
name-rev: fix names by dropping taggerdate workaround
Commit 7550424804 ("name-rev: include taggerdate in considering the best
name", 2016-04-22) introduced the idea of using taggerdate in the
criteria for selecting the best name. At the time, a certain commit in
linux.git -- namely, aed06b9cfcab -- was being named by name-rev as
v4.6-rc1~9^2~792
which, while correct, was very suboptimal. Some investigation found
that tweaking the MERGE_TRAVERSAL_WEIGHT to lower it could give
alternate answers such as
v3.13-rc7~9^2~14^2~42
or
v3.13~5^2~4^2~2^2~1^2~42
A manual solution involving looking at tagger dates came up with
v3.13-rc1~65^2^2~42
which is much nicer. That workaround was then implemented in name-rev.
Unfortunately, the taggerdate heuristic is causing bugs. I was pointed
to a case in a private repository where name-rev reports a name of the
form
v2022.10.02~86
when users expected to see one of the form
v2022.10.01~2
(I've modified the names and numbers a bit from the real testcase.) As
you can probably guess, v2022.10.01 was created after v2022.10.02 (by a
few hours), even though it pointed to an older commit. While the
condition is unusual even in the repository in question, it is not the
only problematic set of tags in that repository. The taggerdate logic
is causing problems.
Further, it turns out that this taggerdate heuristic isn't even helping
anymore. Due to the fix to naming logic in 3656f84278 ("name-rev:
prefer shorter names over following merges", 2021-12-04), we get
improved names without the taggerdate heuristic. For the original
commit of interest in linux.git, a modern git without the taggerdate
heuristic still provides the same optimal answer of interest, namely:
v3.13-rc1~65^2^2~42
So, the taggerdate is no longer providing benefit, and it is causing
problems. Simply get rid of it.
However, note that "taggerdate" as a variable is used to store things
besides a taggerdate these days. Ever since commit ef1e74065c
("name-rev: favor describing with tags and use committer date to
tiebreak", 2017-03-29), this has been used to store committer dates and
there it is used as a fallback tiebreaker (as opposed to a primary
criteria overriding effective distance calculations). We do not want to
remove that fallback tiebreaker, so not all instances of "taggerdate"
are removed in this change.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Andrei Rybak [Tue, 7 Feb 2023 23:42:59 +0000 (00:42 +0100)]
userdiff: support Java sealed classes
A new kind of class was added in Java 17 -- sealed classes.[1] This
feature includes several new keywords that may appear in a declaration
of a class. New modifiers before name of the class: "sealed" and
"non-sealed", and a clause after name of the class marked by keyword
"permits".
The current set of regular expressions in userdiff.c already allows the
modifier "sealed" and the "permits" clause, but not the modifier
"non-sealed", which is the first hyphenated keyword in Java.[2] Allow
hyphen in the words that precede the name of type to match the
"non-sealed" modifier.
In new input file "java-sealed" for the test t4018-diff-funcname.sh, use
a Java code comment for the marker "RIGHT". This workaround is needed,
because the name of the sealed class appears on the line of code that
has the "ChangeMe" marker.
[1] Detailed description in "JEP 409: Sealed Classes"
https://openjdk.org/jeps/409
[2] "JEP draft: Keyword Management for the Java Language"
https://openjdk.org/jeps/8223002
Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Reviewed-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Andrei Rybak [Tue, 7 Feb 2023 23:42:58 +0000 (00:42 +0100)]
userdiff: support Java record types
A new kind of class was added in Java 16 -- records.[1] The syntax of
records is similar to regular classes with one important distinction:
the name of the record class is followed by a mandatory list of
components. The list is enclosed in parentheses, it may be empty, and
it may immediately follow the name of the class or type parameters, if
any, with or without separating whitespace. For example:
public record Example(int i, String s) {
}
public record WithTypeParameters<A, B>(A a, B b, String s) {
}
record SpaceBeforeComponents (String comp1, int comp2) {
}
Support records in the builtin userdiff pattern for Java. Add "record"
to the alternatives of keywords for kinds of class.
Allowing matching various possibilities for the type parameters and/or
list of the components of a record has already been covered by the
preceding patch.
[1] detailed description is available in "JEP 395: Records"
https://openjdk.org/jeps/395
Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Reviewed-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Andrei Rybak [Tue, 7 Feb 2023 23:42:57 +0000 (00:42 +0100)]
userdiff: support Java type parameters
A class or interface in Java can have type parameters following the name
in the declared type, surrounded by angle brackets (paired less than and
greater than signs).[2] The type parameters -- `A` and `B` in the
examples -- may follow the class name immediately:
public class ParameterizedClass<A, B> {
}
or may be separated by whitespace:
public class SpaceBeforeTypeParameters <A, B> {
}
A part of the builtin userdiff pattern for Java matches declarations of
classes, enums, and interfaces. The regular expression requires at
least one whitespace character after the name of the declared type.
This disallows matching for opening angle bracket of type parameters
immediately after the name of the type. Mandatory whitespace after the
name of the type also disallows using the pattern in repositories with a
fairly common code style that puts braces for the body of a class on
separate lines:
class WithLineBreakBeforeOpeningBrace
{
}
Support matching Java code in more diverse code styles and declarations
of classes and interfaces with type parameters immediately following the
name of the type in the builtin userdiff pattern for Java. Do so by
just matching anything until the end of the line after the keywords for
the kind of type being declared.
[1] Since Java 5 released in 2004.
[2] Detailed description is available in the Java Language
Specification, sections "Type Variables" and "Parameterized Types":
https://docs.oracle.com/javase/specs/jls/se17/html/jls-4.html#jls-4.4
Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Reviewed-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Emily Shaffer [Wed, 8 Feb 2023 19:21:15 +0000 (20:21 +0100)]
hook: support a --to-stdin=<path> option
Expose the "path_to_stdin" API added in the preceding commit in the
"git hook run" command.
For now we won't be using this command interface outside of the tests,
but exposing this functionality makes it easier to test the hook
API. The plan is to use this to extend the "sendemail-validate"
hook[1][2].
Emily Shaffer [Wed, 8 Feb 2023 19:21:14 +0000 (20:21 +0100)]
sequencer: use the new hook API for the simpler "post-rewrite" call
Change the invocation of the "post-rewrite" hook added in 795160457db (sequencer (rebase -i): run the post-rewrite hook, if
needed, 2017-01-02) to use the new hook API.
This leaves the more complex "post-rewrite" invocation added in a87a6f3c98e (commit: move post-rewrite code to libgit, 2017-11-17)
here in sequencer.c unconverted.
Here we can pass in a file's via the "in" file descriptor, in that
case we don't have a file, but will need to write_in_full() to an "in"
provide by the API. Support for that will be added to the hook API in
the future, but we're not there yet.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Emily Shaffer [Wed, 8 Feb 2023 19:21:13 +0000 (20:21 +0100)]
hook API: support passing stdin to hooks, convert am's 'post-rewrite'
Convert the invocation of the 'post-rewrite' hook run by 'git am' to
use the hook.h library. To do this we need to add a "path_to_stdin"
member to "struct run_hooks_opt".
In our API this is supported by asking for a file path, rather
than by reading stdin. Reading directly from stdin would involve caching
the entire stdin (to memory or to disk) once the hook API is made to
support "jobs" larger than 1, along with support for executing N hooks
at a time (i.e. the upcoming config-based hooks).
Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Emily Shaffer [Wed, 8 Feb 2023 19:21:12 +0000 (20:21 +0100)]
run-command: allow stdin for run_processes_parallel
While it makes sense not to inherit stdin from the parent process to
avoid deadlocking, it's not necessary to completely ban stdin to
children. An informed user should be able to configure stdin safely. By
setting `some_child.process.no_stdin=1` before calling `get_next_task()`
we provide a reasonable default behavior but enable users to set up
stdin streaming for themselves during the callback.
`some_child.process.stdout_to_stderr`, however, remains unmodifiable by
`get_next_task()` - the rest of the run_processes_parallel() API depends
on child output in stderr.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 8 Feb 2023 17:14:42 +0000 (09:14 -0800)]
Merge branch 'ds/scalar-ignore-cron-error'
Allow "scalar" to warn but continue when its periodic maintenance
feature cannot be enabled.
* ds/scalar-ignore-cron-error:
scalar: only warn when background maintenance fails
t921*: test scalar behavior starting maintenance
t: allow 'scalar' in test_must_fail
Calvin Wan [Tue, 7 Feb 2023 18:12:27 +0000 (18:12 +0000)]
Documentation: clarify multiple pushurls vs urls
In a remote with multiple configured URLs, `git remote -v` shows the
correct url that fetch uses. However, `git config remote.<remote>.url`
returns the last defined url instead. This discrepancy can cause
confusion for users with a remote defined as such, since any url
defined after the first essentially acts as a pushurl.
Add documentation to clarify how fetch interacts with multiple urls
and how push interacts with multiple pushurls and urls.
Add test affirming interaction between fetch and multiple urls.
Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>