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
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
Finally retire the scripted "git add -p/-i" implementation and have
everybody use the one reimplemented in C.
* ab/retire-scripted-add-p:
docs & comments: replace mentions of "git-add--interactive.perl"
add API: remove run_add_interactive() wrapper function
add: remove "add.interactive.useBuiltin" & Perl "git add--interactive"
Userdiff regexp update for Java language.
* ar/userdiff-java-update:
userdiff: support Java sealed classes
userdiff: support Java record types
userdiff: support Java type parameters
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()
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
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
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>
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>
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>
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>
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].
1. https://lore.kernel.org/git/ad152e25-4061-9955-d3e6-a2c8b1bd24e7@amd.com
2. https://lore.kernel.org/git/20230120012459.920932-1-michael.strawbridge@amd.com
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>
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
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>
Fix a memory leak that's been with us since d96855ff51 (merge-base:
teach "--fork-point" mode, 2013-10-23).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to the existing "squash_onto_name" added in [1] we need to
free() the xstrdup()'d "options.onto.name" added for "--keep-base" in
[2]..
1. 9dba809a69 (builtin rebase: support --root, 2018-09-04)
2. 414d924beb (rebase: teach rebase --keep-base, 2019-08-27)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make the replay_opts_release() function added in the preceding commit
non-static, and use it for freeing the "struct replay_opts"
constructed for "rebase" and "revert".
To safely call our new replay_opts_release() we'll need to stop
calling it in sequencer_remove_state(), and instead call it where we
allocate the "struct replay_opts" itself.
This is because in e.g. do_interactive_rebase() we construct a "struct
replay_opts" with "get_replay_opts()", and then call
"complete_action()". If we get far enough in that function without
encountering errors we'll call "pick_commits()" which (indirectly)
calls sequencer_remove_state() at the end.
But if we encounter errors anywhere along the way we'd punt out early,
and not free() the memory we allocated. Remembering whether we
previously called sequencer_remove_state() would be a hassle.
Using a FREE_AND_NULL() pattern would also work, as it would be safe
to call replay_opts_release() repeatedly. But let's fix this properly
instead, by having the owner of the data free() it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a memory leak that's been with us since this code was added in
ca02465b41 (push: use remote.$name.push as a refmap, 2013-12-03).
The "remote = remote_get(...)" added in the same commit would seem to
leak based only on the context here, but that function is a wrapper
for sticking the remotes we fetch into "the_repository->remote_state".
See fd3cb0501e (remote: move static variables into per-repository
struct, 2021-11-17) for the addition of code in repository.c that
free's the "remote" allocated here.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a memory leak that's been with us since this code was introduced
in [1]. Later in [2] we started using FLEX_ALLOC_MEM() to allocate the
"struct command *".
1. 575f497456 (Add first cut at "git-receive-pack", 2005-06-29)
2. eb1af2df0b (git-receive-pack: start parsing ref update commands,
2005-06-29)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Follow-up 465028e0e2 (merge: add missing strbuf_release(),
2021-10-07) and address the "msg" memory leak in this block. We could
free "&msg" before the "goto done" here, but even better is to avoid
allocating it in the first place.
By repeating the "Fast-forward" string here we can avoid using a
"struct strbuf" altogether.
Suggested-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We were leaking both the "struct strbuf" in prune_worktrees(), as well
as the "path" we got from should_prune_worktree(). Since these were
the only two uses of the "struct string_list" let's change it to a
"DUP" and push these to it with "string_list_append_nodup()".
For the string_list_append_nodup() we could also string_list_append()
the main_path.buf, and then strbuf_release(&main_path) right away. But
doing it this way avoids an allocation, as we already have the "struct
strbuf" prepared for appending to "kept".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In cmd_repack() when we hit an error, replace "return ret" with "goto
cleanup" to ensure we free the necessary data structures.
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix memory leaks resulting from a missing clear_pathspec().
- archive.c: Plug a leak in the "struct archiver_args", and
clear_pathspec() the "pathspec" member that the "parse_pathspec_arg()"
call in this function populates.
- builtin/clean.c: Fix a memory leak that's been with us since
893d839970 (clean: convert to use parse_pathspec, 2013-07-14).
- builtin/reset.c: Add clear_pathspec() calls to cmd_reset(),
including to the codepaths where we'd return early.
- builtin/stash.c: Call clear_pathspec() on the pathspec initialized
in push_stash().
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the "ab/various-leak-fixes" topic was merged in [1] only t6021
would fail if the tests were run in the
"GIT_TEST_PASSING_SANITIZE_LEAK=check" mode, i.e. to check whether we
marked all leak-free tests with "TEST_PASSES_SANITIZE_LEAK=true".
Since then we've had various tests starting to pass under
SANITIZE=leak. Let's mark those as passing, this is when they started
to pass, narrowed down with "git bisect":
- t5317-pack-objects-filter-objects.sh: In
faebba436e (list-objects-filter: plug pattern_list leak, 2022-12-01).
- t3210-pack-refs.sh, t5613-info-alternate.sh,
t7403-submodule-sync.sh: In 189e97bc4b (diff: remove parseopts member
from struct diff_options, 2022-12-01).
- t1408-packed-refs.sh: In ab91f6b7c4 (Merge branch
'rs/diff-parseopts', 2022-12-19).
- t0023-crlf-am.sh, t4152-am-subjects.sh, t4254-am-corrupt.sh,
t4256-am-format-flowed.sh, t4257-am-interactive.sh,
t5403-post-checkout-hook.sh: In a658e881c1 (am: don't pass strvec to
apply_parse_options(), 2022-12-13)
- t1301-shared-repo.sh, t1302-repo-version.sh: In b07a819c05 (reflog:
clear leftovers in reflog_expiry_cleanup(), 2022-12-13).
- t1304-default-acl.sh, t1410-reflog.sh,
t5330-no-lazy-fetch-with-commit-graph.sh, t5502-quickfetch.sh,
t5604-clone-reference.sh, t6014-rev-list-all.sh,
t7701-repack-unpack-unreachable.sh: In b0c61be320 (Merge branch
'rs/reflog-expiry-cleanup', 2022-12-26)
- t3800-mktag.sh, t5302-pack-index.sh, t5306-pack-nobase.sh,
t5573-pull-verify-signatures.sh, t7612-merge-verify-signatures.sh: In
69bbbe484b (hash-object: use fsck for object checks, 2023-01-18).
- t1451-fsck-buffer.sh: In 8e4309038f (fsck: do not assume
NUL-termination of buffers, 2023-01-19).
- t6501-freshen-objects.sh: In abf2bb895b (Merge branch
'jk/hash-object-fsck', 2023-01-30)
1. 9ea1378d04 (Merge branch 'ab/various-leak-fixes', 2022-12-14)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since [1] first released with Git v2.37.0 the built-in version of "add
-i" has been the default. That built-in implementation was added in
[2], first released with Git v2.25.0.
At this point enough time has passed to allow for finding any
remaining bugs in this new implementation, so let's remove the
fallback code.
As with similar migrations for "stash"[3] and "rebase"[4] we're
keeping a mention of "add.interactive.useBuiltin" in the
documentation, but adding a warning() to notify any outstanding users
that the built-in is now the default. As with [5] and [6] we should
follow-up in the future and eventually remove that warning.
1. 0527ccb1b5 (add -i: default to the built-in implementation,
2021-11-30)
2. f83dff60a7 (Start to implement a built-in version of `git add
--interactive`, 2019-11-13)
3. 8a2cd3f512 (stash: remove the stash.useBuiltin setting,
2020-03-03)
4. d03ebd411c (rebase: remove the rebase.useBuiltin setting,
2019-03-18)
5. deeaf5ee07 (stash: remove documentation for `stash.useBuiltin`,
2022-01-27)
6. 9bcde4d531 (rebase: remove transitory rebase.useBuiltin setting &
env, 2021-03-23)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To match present day coding guiding codelines let's:
- use <<-EOF, so we can indent all lines to the
the same level for this test
- use <<\EOF to notify the reader that no interpolation
is expected in the body
Signed-off-by: Kostya Farber <kostya.farber@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* maint-2.35:
Git 2.35.7
Git 2.34.7
http: support CURLOPT_PROTOCOLS_STR
http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION
http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
Git 2.33.7
Git 2.32.6
Git 2.31.7
Git 2.30.8
apply: fix writing behind newly created symbolic links
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
clone: delay picking a transport until after get_repo_path()
t5619: demonstrate clone_local() with ambiguous transport
* maint-2.34:
Git 2.34.7
http: support CURLOPT_PROTOCOLS_STR
http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION
http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
Git 2.33.7
Git 2.32.6
Git 2.31.7
Git 2.30.8
apply: fix writing behind newly created symbolic links
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
clone: delay picking a transport until after get_repo_path()
t5619: demonstrate clone_local() with ambiguous transport
* maint-2.33:
Git 2.33.7
Git 2.32.6
Git 2.31.7
Git 2.30.8
apply: fix writing behind newly created symbolic links
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
clone: delay picking a transport until after get_repo_path()
t5619: demonstrate clone_local() with ambiguous transport
* maint-2.32:
Git 2.32.6
Git 2.31.7
Git 2.30.8
apply: fix writing behind newly created symbolic links
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
clone: delay picking a transport until after get_repo_path()
t5619: demonstrate clone_local() with ambiguous transport
* maint-2.31:
Git 2.31.7
Git 2.30.8
apply: fix writing behind newly created symbolic links
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
clone: delay picking a transport until after get_repo_path()
t5619: demonstrate clone_local() with ambiguous transport
* maint-2.30:
Git 2.30.8
apply: fix writing behind newly created symbolic links
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
clone: delay picking a transport until after get_repo_path()
t5619: demonstrate clone_local() with ambiguous transport
Fix a vulnerability (CVE-2023-23946) that allows crafted input to trick
`git apply` into writing files outside of the working tree.
* ps/apply-beyond-symlink:
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
"git rebase" often ignored incompatible options instead of
complaining, which has been corrected.
* en/rebase-incompatible-opts:
rebase: provide better error message for apply options vs. merge config
rebase: put rebase_options initialization in single place
rebase: fix formatting of rebase --reapply-cherry-picks option in docs
rebase: clarify the OPT_CMDMODE incompatibilities
rebase: add coverage of other incompatible options
rebase: fix incompatiblity checks for --[no-]reapply-cherry-picks
rebase: fix docs about incompatibilities with --root
rebase: remove --allow-empty-message from incompatible opts
rebase: flag --apply and --merge as incompatible
rebase: mark --update-refs as requiring the merge backend
When writing files git-apply(1) initially makes sure that none of the
files it is about to create are behind a symlink:
```
$ git init repo
Initialized empty Git repository in /tmp/repo/.git/
$ cd repo/
$ ln -s dir symlink
$ git apply - <<EOF
diff --git a/symlink/file b/symlink/file
new file mode 100644
index 0000000..e69de29
EOF
error: affected file 'symlink/file' is beyond a symbolic link
```
This safety mechanism is crucial to ensure that we don't write outside
of the repository's working directory. It can be fooled though when the
patch that is being applied creates the symbolic link in the first
place, which can lead to writing files in arbitrary locations.
Fix this by checking whether the path we're about to create is
beyond a symlink or not. Tightening these checks like this should be
fine as we already have these precautions in Git as explained
above. Ideally, we should update the check we do up-front before
starting to reflect the computed changes to the working tree so that
we catch this case as well, but as part of embargoed security work,
adding an equivalent check just before we try to write out a file
should serve us well as a reasonable first step.
Digging back into history shows that this vulnerability has existed
since at least Git v2.9.0. As Git v2.8.0 and older don't build on my
system anymore I cannot tell whether older versions are affected, as
well.
Reported-by: Joern Schneeweisz <jschneeweisz@gitlab.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Recent versions of openssl will refuse to work with 1024-bit RSA keys,
as they are considered insecure. I didn't track down the exact version
in which the defaults were tightened, but the Debian-package openssl 3.0
on my system yields:
$ LIB_HTTPD_SSL=1 ./t5551-http-fetch-smart.sh -v -i
[...]
SSL Library Error: error:0A00018F:SSL routines::ee key too small
1..0 # SKIP web server setup failed
This could probably be overcome with configuration, but that's likely
to be a headache (especially if it requires touching /etc/openssl).
Let's just pick a key size that's less outrageously out of date.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The SSL config enabled by setting LIB_HTTPD_SSL does not work with
Apache versions greater than 2.2, as more recent versions complain about
the SSLMutex directive. According to
https://httpd.apache.org/docs/current/upgrading.html:
Directives AcceptMutex, LockFile, RewriteLock, SSLMutex,
SSLStaplingMutex, and WatchdogMutexPath have been replaced with a
single Mutex directive. You will need to evaluate any use of these
removed directives in your 2.2 configuration to determine if they can
just be deleted or will need to be replaced using Mutex.
Deleting this line will just use the system default, which seems
sensible. The original came as part of faa4bc35a0 (http-push: add
regression tests, 2008-02-27), but no specific reason is given there (or
on the mailing list) for its presence.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apache 2.4 has been out since early 2012, almost 11 years. And its
predecessor, 2.2, has been out of support since its last release in
2017, over 5 years ago. The last mention on the mailing list was from
around the same time, in this thread:
https://lore.kernel.org/git/20171231023234.21215-1-tmz@pobox.com/
We can probably assume that 2.4 is available everywhere. And the stakes
are fairly low, as the worst case is that such a platform would skip the
http tests.
This lets us clean up a few minor version checks in the config file, but
also revert f1f2b45be0 (tests: adjust the configuration for Apache 2.2,
2016-05-09). Its technique isn't _too_ bad, but certainly required a bit
more explanation than the 2.4 version it replaced. I manually confirmed
that the test in t5551 still behaves as expected (if you replace
"cadabra" with "foo", the server correctly rejects the request).
It will also help future patches which will no longer have to deal with
conditional config for this old version.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Apache 2.2 was released in 2005, almost 18 years ago. We can probably
assume that people are running a version at least that old (and the
stakes for removing it are fairly low, as the worst case is that they
would not run the http tests against their ancient version).
Dropping support for the older versions cleans up the config file a
little, and will also enable us to bump the required version further
(with more cleanups) in a future patch.
Note that the file actually checks for version 2.1. In apache's
versioning scheme, odd numbered versions are for development and even
numbers are for stable releases. So 2.1 and 2.2 are effectively the same
from our perspective.
Older versions would just fail to start, which would generally cause us
to skip the tests. However, we do have version detection code in
lib-httpd.sh which produces a nicer error message, so let's update that,
too. I didn't bother handling the case of "3.0", etc. Apache has been on
2.x for 21 years, with no signs of bumping the major version. And if
they eventually do, I suspect there will be enough breaking changes that
we'd need to update more than just the numeric version check. We can
worry about that hypothetical when it happens.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The creationToken heuristic uses a different mechanism for downloading
bundles from the "standard" approach. Specifically: it uses a concrete
order based on the creationToken values and attempts to download as few
bundles as possible. It also modifies local config to store a value for
future fetches to avoid downloading bundles, if possible.
However, if any of the individual bundles has a failed download, then
the logic for the ordering comes into question. It is important to avoid
infinite loops, assigning invalid creation token values in config, but
also to be opportunistic as possible when downloading as many bundles as
seem appropriate.
These tests were used to inform the implementation of
fetch_bundles_by_token() in bundle-uri.c, but are being added
independently here to allow focusing on faulty downloads. There may be
more cases that could be added that result in modifications to
fetch_bundles_by_token() as interesting data shapes reveal themselves in
real scenarios.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a bundle list specifies the "creationToken" heuristic, the Git
client downloads the list and then starts downloading bundles in
descending creationToken order. This process stops as soon as all
downloaded bundles can be applied to the repository (because all
required commits are present in the repository or in the downloaded
bundles).
When checking the same bundle list twice, this strategy requires
downloading the bundle with the maximum creationToken again, which is
wasteful. The creationToken heuristic promises that the client will not
have a use for that bundle if its creationToken value is at most the
previous creationToken value.
To prevent these wasteful downloads, create a fetch.bundleCreationToken
config setting that the Git client sets after downloading bundles. This
value allows skipping that maximum bundle download when this config
value is the same value (or larger).
To test that this works correctly, we can insert some "duplicate"
fetches into existing tests and demonstrate that only the bundle list is
downloaded.
The previous logic for downloading bundles by creationToken worked even
if the bundle list was empty, but now we have logic that depends on the
first entry of the list. Terminate early in the (non-sensical) case of
an empty bundle list.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a user specifies a URI via 'git clone --bundle-uri', that URI may
be a bundle list that advertises a 'bundle.heuristic' value. In that
case, the Git client stores a 'fetch.bundleURI' config value storing
that URI.
Teach 'git fetch' to check for this config value and download bundles
from that URI before fetching from the Git remote(s). Likely, the bundle
provider has configured a heuristic (such as "creationToken") that will
allow the Git client to download only a portion of the bundles before
continuing the fetch.
Since this URI is completely independent of the remote server, we want
to be sure that we connect to the bundle URI before creating a
connection to the Git remote. We do not want to hold a stateful
connection for too long if we can avoid it.
To test that this works correctly, extend the previous tests that set
'fetch.bundleURI' to do follow-up fetches. The bundle list is updated
incrementally at each phase to demonstrate that the heuristic avoids
downloading older bundles. This includes the middle fetch downloading
the objects in bundle-3.bundle from the Git remote, and therefore not
needing that bundle in the third fetch.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Bundle providers may organize their bundle lists in a way that is
intended to improve incremental fetches, not just initial clones.
However, they do need to state that they have organized with that in
mind, or else the client will not expect to save time by downloading
bundles after the initial clone. This is done by specifying a
bundle.heuristic value.
There are two types of bundle lists: those at a static URI and those
that are advertised from a Git remote over protocol v2.
The new fetch.bundleURI config value applies for static bundle URIs that
are not advertised over protocol v2. If the user specifies a static URI
via 'git clone --bundle-uri', then Git can set this config as a reminder
for future 'git fetch' operations to check the bundle list before
connecting to the remote(s).
For lists provided over protocol v2, we will want to take a different
approach and create a property of the remote itself by creating a
remote.<id>.* type config key. That is not implemented in this change.
Later changes will update 'git fetch' to consume this option.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
