]> git.ipfire.org Git - thirdparty/git.git/log
thirdparty/git.git
6 months agoobject-name: fix reversed ordering with ":/<text>" revisions
Patrick Steinhardt [Fri, 6 Dec 2024 15:45:13 +0000 (16:45 +0100)] 
object-name: fix reversed ordering with ":/<text>" revisions

Recently it was reported [1] that "look for the youngest commit
reachable from any ref with log message that match the given
pattern" syntax (i.e. ':/<text>') started to return results in
reverse recency order. This regression was introduced in Git v2.47.0
and is caused by a memory leak fix done in 57fb139b5e (object-name:
fix leaking commit list items, 2024-08-01).

The intent of the identified commit is to stop modifying the commit list
provided by the caller such that the caller can properly free all commit
list items, including those that the called function might potentially
remove from the list. This was done by creating a copy of the passed-in
commit list and modifying this copy instead of the caller-provided list.

We already knew to create such a copy beforehand with the `backup` list,
which was used to clear the `ONELINE_SEEN` commit mark after we were
done. So the refactoring simply renamed that list to `copy` and started
to operate on that list instead. There is a gotcha though: the backup
list, and thus now also the copied list, is always being prepended to,
so the resulting list is in reverse order! The end result is that we
pop commits from the wrong end of the commit list, returning commits in
reverse recency order.

Fix the bug by appending to the list instead.

[1]: <CAKOEJdcPYn3O01p29rVa+xv=Qr504FQyKJeSB-Moze04ViCGGg@mail.gmail.com>

Reported-by: Aarni Koskela <aarni@valohai.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agocommit-reach: fix trivial memory leak when computing reachability
Patrick Steinhardt [Thu, 1 Aug 2024 10:41:15 +0000 (12:41 +0200)] 
commit-reach: fix trivial memory leak when computing reachability

We don't free the local `stack` commit list that we use to compute
reachability of multiple commits at once. Do so.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agoconvert: fix leaking config strings
Patrick Steinhardt [Thu, 1 Aug 2024 10:41:11 +0000 (12:41 +0200)] 
convert: fix leaking config strings

In `read_convert_config()`, we end up reading some string values into
variables. We don't free any potentially-existing old values though,
which will result in a memory leak in case the same key has been defined
multiple times.

Fix those leaks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agoentry: fix leaking pathnames during delayed checkout
Patrick Steinhardt [Thu, 1 Aug 2024 10:41:06 +0000 (12:41 +0200)] 
entry: fix leaking pathnames during delayed checkout

When filtering files during delayed checkout, we pass a string list to
`async_query_available_blobs()`. This list is initialized with NODUP,
and thus inserted strings will not be owned by the list. In the latter
function we then try to hand over ownership by passing an `xstrup()`'d
value to `string_list_insert()`. But this is not how this works: a NODUP
list does not take ownership of allocated strings and will never free
them for the caller.

Fix this issue by initializing the list as `DUP` instead and dropping
the explicit call to `xstrdup()`. This is okay to do given that this is
the single callsite of `async_query_available_blobs()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agoobject-name: fix leaking commit list items
Patrick Steinhardt [Thu, 1 Aug 2024 10:41:01 +0000 (12:41 +0200)] 
object-name: fix leaking commit list items

When calling `get_oid_oneline()`, we pass in a `struct commit_list` that
gets modified by the function. This creates a weird situation where the
commit list may sometimes be empty after returning, but sometimes it
will continue to carry additional commits. In those cases the remainder
of the list leaks.

Ultimately, the design where we only pass partial ownership to
`get_oid_oneline()` feels shoddy. Refactor the code such that we only
pass a constant pointer to the list, creating a local copy as needed.
Callers are thus always responsible for freeing the commit list, which
then allows us to plug a bunch of memory leaks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agot/test-repository: fix leaking repository
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:56 +0000 (12:40 +0200)] 
t/test-repository: fix leaking repository

The test-repository test helper zeroes out `the_repository` such that it
can be sure that our codebase only ends up using the supplied repository
that we initialize in the respective helper functions. This does cause
memory leaks though as the data that `the_repository` has been holding
onto is not referenced anymore.

Fix this by calling `repo_clear()` instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/credential-cache: fix trivial leaks
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:51 +0000 (12:40 +0200)] 
builtin/credential-cache: fix trivial leaks

There are two trivial leaks in git-credential-cache(1):

  - We leak the child process in `spawn_daemon()`. As we do not call
    `finish_command()` and instead let the created process daemonize, we
    have to clear the process manually.

  - We do not free the computed socket path in case it wasn't given via
    `--socket=`.

Plug both of these memory leaks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/worktree: fix leaking derived branch names
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:46 +0000 (12:40 +0200)] 
builtin/worktree: fix leaking derived branch names

There are several heuristics that git-worktree(1) uses to derive the
name of the newly created branch when not given explicitly. These
heuristics all allocate a new string, but we only end up freeing that
string in a subset of cases.

Fix the remaining cases where we didn't yet free the derived branch
names. While at it, also free `opt_track`, which is being populated via
an `OPT_PASSTHRU()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/shortlog: fix various trivial memory leaks
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:41 +0000 (12:40 +0200)] 
builtin/shortlog: fix various trivial memory leaks

There is a trivial memory leak in git-shortlog(1). Fix it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/rerere: fix various trivial memory leaks
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:36 +0000 (12:40 +0200)] 
builtin/rerere: fix various trivial memory leaks

There are multiple trivial memory leaks in git-rerere(1). Fix those.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/credential-store: fix leaking credential
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:31 +0000 (12:40 +0200)] 
builtin/credential-store: fix leaking credential

We never free credentials read by the credential store, leading to a
memory leak. Plug it.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/show-branch: fix several memory leaks
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:26 +0000 (12:40 +0200)] 
builtin/show-branch: fix several memory leaks

There are several memory leaks in git-show-branch(1). Fix them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/rev-parse: fix memory leak with `--parseopt`
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:21 +0000 (12:40 +0200)] 
builtin/rev-parse: fix memory leak with `--parseopt`

The `--parseopt` mode allows shell scripts to have the same option
parsing mode as we have in C builtins. It soaks up a set of option
descriptions via stdin and massages them into proper `struct option`s
that we can then use to parse a set of arguments.

We only partially free those options when done though, creating a memory
leak. Interestingly, we only end up free'ing the first option's help,
which is of course wrong.

Fix this by freeing all option's help fields as well as their `argh`
fields to plug this memory leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/stash: fix various trivial memory leaks
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:17 +0000 (12:40 +0200)] 
builtin/stash: fix various trivial memory leaks

There are multiple trivial memory leaks in git-stash(1). Fix those.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/remote: fix various trivial memory leaks
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:12 +0000 (12:40 +0200)] 
builtin/remote: fix various trivial memory leaks

There are multiple trivial memory leaks in git-remote(1). Fix those.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/remote: fix leaking strings in `branch_list`
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:07 +0000 (12:40 +0200)] 
builtin/remote: fix leaking strings in `branch_list`

The `struct string_list branch_list` is declared as `NODUP`, which makes
it not copy strings inserted into it. This causes memory leaks though,
as this means it also won't be responsible for _freeing_ inserted
strings. Thus, every branch we add to this will leak.

Fix this by marking the list as `DUP` instead and free the local copy we
have of the variable.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/ls-remote: fix leaking `pattern` strings
Patrick Steinhardt [Thu, 1 Aug 2024 10:40:02 +0000 (12:40 +0200)] 
builtin/ls-remote: fix leaking `pattern` strings

Users can pass patterns to git-ls-remote(1), which allows them to filter
the list of printed references. We assemble those patterns into an array
and prefix them with "*/", but never free either the array nor the
allocated strings.

Refactor the code to use a `struct strvec` instead of manually tracking
the strings in an array. Like this, we can easily use `strvec_clear()`
to release both the vector and the contained string for us, plugging the
leak.

Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/submodule--helper: fix leaking buffer in `is_tip_reachable`
Patrick Steinhardt [Thu, 1 Aug 2024 10:39:57 +0000 (12:39 +0200)] 
builtin/submodule--helper: fix leaking buffer in `is_tip_reachable`

The `rev` buffer in `is_tip_reachable()` is being populated with the
output of git-rev-list(1) -- if either the command fails or the buffer
contains any data, then the input commit is not reachable.

The buffer isn't used for anything else, but neither do we free it,
causing a memory leak. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/submodule--helper: fix leaking clone depth parameter
Patrick Steinhardt [Thu, 1 Aug 2024 10:42:59 +0000 (12:42 +0200)] 
builtin/submodule--helper: fix leaking clone depth parameter

The submodule helper supports a `--depth` parameter for both its "add"
and "clone" subcommands, which in both cases end up being forwarded to
git-clone(1). But while the former subcommand uses an `OPT_INTEGER()` to
parse the depth, the latter uses `OPT_STRING()`. Consequently, it is
possible to pass non-integer input to "--depth" when calling the "clone"
subcommand, where the value will then ultimately cause git-clone(1) to
bail out.

Besides the fact that the parameter verification should happen earlier,
the submodule helper infrastructure also internally tracks the depth via
a string. This requires us to convert the integer in the "add"
subcommand into an allocated string, and this string ultimately leaks.

Refactor the code to consistently track the clone depth as an integer.
This plugs the memory leak, simplifies the code and allows us to use
`OPT_INTEGER()` instead of `OPT_STRING()`, validating the input before
we shell out to git--clone(1).

Original-patch-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/name-rev: fix various trivial memory leaks
Patrick Steinhardt [Thu, 1 Aug 2024 10:38:35 +0000 (12:38 +0200)] 
builtin/name-rev: fix various trivial memory leaks

There are several structures that we don't release after
`cmd_name_rev()` is done. Plug those leaks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/describe: fix trivial memory leak when describing blob
Patrick Steinhardt [Thu, 1 Aug 2024 10:38:30 +0000 (12:38 +0200)] 
builtin/describe: fix trivial memory leak when describing blob

We never free the `struct strvec args` variable in `describe_blob()`,
which thus causes a memory leak. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/describe: fix leaking array when running diff-index
Patrick Steinhardt [Thu, 1 Aug 2024 10:38:25 +0000 (12:38 +0200)] 
builtin/describe: fix leaking array when running diff-index

When running git-describe(1) with `--dirty`, we will set up a `struct
rev_info` with arguments for git-diff-index(1). The way we assemble the
arguments it causes two memory leaks though:

  - We never release the `struct strvec`.

  - `setup_revisions()` may end up removing some entries from the
    `strvec`, which we wouldn't free even if we released the struct.

While we could plug those leaks, this is ultimately unnecessary as the
arguments we pass are part of a static array anyway. So instead,
refactor the code to drop the `struct strvec` and just pass this static
array directly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/describe: fix memory leak with `--contains=`
Patrick Steinhardt [Thu, 1 Aug 2024 10:38:20 +0000 (12:38 +0200)] 
builtin/describe: fix memory leak with `--contains=`

When calling `git describe --contains=`, we end up invoking
`cmd_name_rev()` with some munged argv array. This array may contain
allocated strings and furthermore will likely be modified by the called
function. This results in two memory leaks:

  - First, we leak the array that we use to assemble the arguments.

  - Second, we leak the allocated strings that we may have put into the
    array.

Fix those leaks by creating a separate copy of the array that we can
hand over to `cmd_name_rev()`. This allows us to free all strings
contained in the `strvec`, as the original vector will not be modified
anymore.

Furthermore, free both the `strvec` and the copied array to fix the
first memory leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/log: fix leaking branch name when creating cover letters
Patrick Steinhardt [Thu, 1 Aug 2024 10:38:16 +0000 (12:38 +0200)] 
builtin/log: fix leaking branch name when creating cover letters

When calling `make_cover_letter()` without a branch name, we try to
derive the branch name by calling `find_branch_name()`. But while this
function returns an allocated string, we never free the result and thus
have a memory leak. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
10 months agobuiltin/replay: plug leaking `advance_name` variable
Patrick Steinhardt [Thu, 1 Aug 2024 10:38:11 +0000 (12:38 +0200)] 
builtin/replay: plug leaking `advance_name` variable

The `advance_name` variable can either contain a static string when
parsed via the `--advance` command line option or it may be an allocated
string when set via `determine_replay_mode()`. Because we cannot be sure
whether it is allocated or not we just didn't free it at all, resulting
in a memory leak.

Split up the variables such that we can track the static and allocated
strings separately and then free the allocated one to fix the memory
leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoGit 2.46-rc2 v2.46.0-rc2
Junio C Hamano [Tue, 23 Jul 2024 23:54:19 +0000 (16:54 -0700)] 
Git 2.46-rc2

Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoMerge branch 'ps/ref-storage-migration-fix'
Junio C Hamano [Tue, 23 Jul 2024 23:54:34 +0000 (16:54 -0700)] 
Merge branch 'ps/ref-storage-migration-fix'

Hotfix for a topic already in -rc.

* ps/ref-storage-migration-fix:
  refs: fix format migration on Cygwin

11 months agoMerge branch 'js/doc-markup-updates-fix'
Junio C Hamano [Tue, 23 Jul 2024 23:54:34 +0000 (16:54 -0700)] 
Merge branch 'js/doc-markup-updates-fix'

Work around asciidoctor's css that renders `monospace` material
in the SYNOPSIS section of manual pages as block elements.

* js/doc-markup-updates-fix:
  Doc: fix Asciidoctor css workaround
  asciidoctor: fix `synopsis` rendering

11 months agoMerge branch 'ja/doc-markup-updates-fix'
Junio C Hamano [Tue, 23 Jul 2024 23:54:33 +0000 (16:54 -0700)] 
Merge branch 'ja/doc-markup-updates-fix'

Fix documentation mark-up regression in 2.45.

* ja/doc-markup-updates-fix:
  doc: git-clone fix discrepancy between asciidoc and asciidoctor

11 months agoMerge branch 'ds/midx-write-repack-fix'
Junio C Hamano [Tue, 23 Jul 2024 23:54:33 +0000 (16:54 -0700)] 
Merge branch 'ds/midx-write-repack-fix'

Repacking a repository with multi-pack index started making stupid
pack selections in Git 2.45, which has been corrected.

* ds/midx-write-repack-fix:
  midx-write: revert use of --stdin-packs
  t5319: add failing test case for repack/expire

11 months agoDoc: fix Asciidoctor css workaround
Junio C Hamano [Mon, 22 Jul 2024 21:17:55 +0000 (14:17 -0700)] 
Doc: fix Asciidoctor css workaround

The previous step introduced docinfo.html to be used to tweak the
CSS used by the asciidoctor, that by default renders <code> inside
<pre> as a block element, breaking the SYNOPSIS section of a few
pages that adopted a new convention we use since Git 2.45.

But in this project, HTML files are all generated.  We do not force
any human to write HTML by hand, which is an unusual and cruel
punishment.  "*.html" is in the .gitignore file, and "make clean"
removes them.  Having a tracked .html file makes "make clean" make
the tree dirty by removing the tracked docinfo.html file.

Let's do an obvious, minimum and stupid workaround to generate that
file at runtime instead.  The mark-up is being rethought in a major
way for the next development cycle, and the CSS workaround we added
in the previous step may have to adjusted, possibly in a large way,
anyway.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agorefs: fix format migration on Cygwin
Patrick Steinhardt [Tue, 23 Jul 2024 12:31:28 +0000 (14:31 +0200)] 
refs: fix format migration on Cygwin

It was reported that t1460-refs-migrate.sh fails when using Cygwin with
errors like the following:

    error: could not link file '.git/ref_migration.sr9pEF/reftable' to '.git/reftable': Permission denied

As some debugging surfaced, the root cause of this is that some files of
the newly-initialized ref store are still open when the target format is
the "reftable" format, and Cygwin refuses to rename open files.

Fix this issue by closing the new ref store before renaming its files
into place. This is a slight change in behaviour compared to before,
where we kept the new ref store open and then updated the repository's
ref store to point to it.

While we could re-open the new ref store after we have moved files
around, this is ultimately unnecessary. We know that the only user of
`repo_migrate_ref_storage_format()` is the git-refs(1) command, and it
won't access the ref store after it has been migrated anyway. So
reinitializing the ref store would be a waste of time. Regardless of
that it is still sensible to leave the repository in a consistent state.
But instead of reinitializing the ref store, we can simply unset the
repo's ref store altogether and let `get_main_ref_store()` lazily
initialize the new ref store as required.

Reported-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoasciidoctor: fix `synopsis` rendering
Johannes Schindelin [Mon, 22 Jul 2024 20:25:49 +0000 (20:25 +0000)] 
asciidoctor: fix `synopsis` rendering

Since 76880f0510c (doc: git-clone: apply new documentation formatting
guidelines, 2024-03-29), the synopsis of `git clone`'s manual page is
rendered differently than before; Its parent commit did the same for
`git init`.

The result looks quite nice. When rendered with AsciiDoc, that is. When
rendered using AsciiDoctor and displayed in a graphical web browser such
as Firefox, Chrome, Edge, etc, the result is quite unpleasant to my eye,
reading something like this:

SYNOPSIS

 git clone
  [
 --template=
 <template-directory>]
  [
 -l
 ] [
 -s
 ] [
 --no-hardlinks
 ] [
 -q
 ] [
[... continuing like this ...]

The reason is that AsciiDoctor's default style sheet contains this (see
https://github.com/asciidoctor/asciidoctor/blob/854923b15533/src/stylesheets/asciidoctor.css#L519-L521
for context):

pre > code {
  display: block;
}

It is this `display: block` that forces the parts that are enclosed in
`<code>` tags (such as the `git clone` or the `--template=` part) to be
rendered on their own line.

Side note: This seems not to affect console web browsers like `lynx` or
`w3m`, most likely because most style sheet directions cannot be
respected in text terminals and therefore they seem to punt on style
sheets altogether.

To fix this, let's apply the method recommended by AsciiDoctor in
https://docs.asciidoctor.org/asciidoctor/latest/html-backend/default-stylesheet/#customize-docinfo
to partially override AsciiDoctor's default style sheet so that the
`<code>` sections of the synopsis are no longer each rendered on their
own, individual lines.

This fixes https://github.com/git-for-windows/git/issues/5063.

Even on the Git home page, where AsciiDoctor's default stylesheet is
_not_ used, this change resulted in some unpleasant rendering where not
only the font is changed for the `<code>` sections of the synopsis, but
padding and a different background color make the visual impression
quite uneven. This has been addressed in the meantime, via
https://github.com/git/git-scm.com/commit/a492d0565512.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agodoc: git-clone fix discrepancy between asciidoc and asciidoctor
Jean-Noël Avila [Sat, 20 Jul 2024 17:34:13 +0000 (17:34 +0000)] 
doc: git-clone fix discrepancy between asciidoc and asciidoctor

Asciidoc.py does not have the concept of generalized roles, whereas
asciidoctor interprets [foo]`blah` as blah with role foo in the
synopsis, making in effect foo disappear in the output. Note that
square brackets not directly followed by an inline markup do not
define a role, which is why we do not have the issue on other parts of
the documentation.

In order to get a consistant result across asciidoctor and
asciidoc.py, the hack is to use the {empty} entity
to split the bracket part from the inline format part.

Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agomidx-write: revert use of --stdin-packs
Derrick Stolee [Thu, 18 Jul 2024 19:55:46 +0000 (19:55 +0000)] 
midx-write: revert use of --stdin-packs

This reverts b7d6f23a171 (midx-write.c: use `--stdin-packs` when
repacking, 2024-04-01) and then marks the test created in the previous
change as passing.

The fundamental issue with the reverted change is that the focus on
pack-files separates the object selection from how the multi-pack-index
selects a single pack-file for an object ID with multiple copies among
the tracked pack-files.

The change was made with the intention of improving delta compression in
the resulting pack-file, but that can be resolved with the existing
object list mechanism. There are other potential pitfalls of doing an
object walk at this time if the repository is a blobless partial clone,
and that will require additional testing on top of the one that changes
here.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Acked-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agot5319: add failing test case for repack/expire
Derrick Stolee [Thu, 18 Jul 2024 19:55:45 +0000 (19:55 +0000)] 
t5319: add failing test case for repack/expire

Git 2.45.0 included the change b7d6f23a171 (midx-write.c: use
`--stdin-packs` when repacking, 2024-04-01) which caused the 'git
multi-pack-index repack' command to use 'git pack-objects --stdin-packs'
instead of listing the objects to repack. While this change was
motivated by efficient cross-process communication and the ability to
improve delta compression, it breaks a fundamental function of the
'incremental-repack' task that is enabled by default in Scalar clones or
Git repositories that run 'git maintenance start'.

The 'incremental-repack' task performs a two-step process of the
'expire' and 'repack' subcommands of the 'git multi-pack-index' builtin.
The 'expire' command removes any pack-files listed in the
multi-pack-index but without any referenced objects. The 'repack' task
then finds a batch of pack-files to repack and sends their objects to
'git pack-objects'. Both the pack-files chosen for the batch and the
objects chosen to repack are based on the ones that the multi-pack-index
references. Objects that appear in a pack-file but have a duplicate copy
in a newer pack-file are not considered in this case. Since the
multi-pack-index references only the newest copy of an object, this
allows the next 'incremental-repack' task to remove the pack-files in
the next 'expire' task. This delay is intentional due to how Windows
handles may block deletion of files with open read handles.

However, the mentioned commit changed this behavior to divorce the set
of objects referenced by the multi-pack-index and instead use a set of
"included" and "excluded" pack-files in the 'git pack-objects' builtin.
When a pack-file is selected as "included", only the objects it contains
but are not in any "excluded" pack-files are considered for repacking.
This has led to client repositories failing to remove old pack-files as
they still have some referenced objects. This grows over time until the
point that Git is trying to repack the same pack-files over and over.

For now, create a test case that demonstrates the expected behavior, but
also fails in its final line. The setup here it attempting to recreate a
typical situation for a repository that uses a blobless partial clone.
There would be a large initial pack-file from the clone that is never
selected in the 'repack' batch. There are other pack-files that have a
combination of new objects from incremental fetches and possibly blobs
that are not connected to those incremental fetches; these blobs could
be filled in from commands like 'git checkout' or 'git blame'. The
pack-files also have some overlap on purpose so test-1 has some
duplicates in test-2 and test-2 has some duplicates in test-3.

At the end of the test, the test-2 pack-file still exists though it
should have been expired. This test will pass when reverting the
offending commit.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoGit 2.46-rc1 v2.46.0-rc1
Junio C Hamano [Thu, 18 Jul 2024 15:23:53 +0000 (08:23 -0700)] 
Git 2.46-rc1

Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoMerge branch 'jk/am-retry'
Junio C Hamano [Thu, 18 Jul 2024 15:30:27 +0000 (08:30 -0700)] 
Merge branch 'jk/am-retry'

Test fix as a follow-up to an already graduated topic.

* jk/am-retry:
  t4153: stop redirecting input from /dev/zero

11 months agoMerge branch 'tb/pseudo-merge-reachability-bitmap'
Junio C Hamano [Thu, 18 Jul 2024 15:30:27 +0000 (08:30 -0700)] 
Merge branch 'tb/pseudo-merge-reachability-bitmap'

Doc update.

* tb/pseudo-merge-reachability-bitmap:
  Documentation/gitpacking: make sample configs listing blocks

11 months agoMerge branch 'ps/pseudo-ref-terminology'
Junio C Hamano [Thu, 18 Jul 2024 15:30:26 +0000 (08:30 -0700)] 
Merge branch 'ps/pseudo-ref-terminology'

Doc update.

* ps/pseudo-ref-terminology:
  Documentation/glossary: fix double word

11 months agoMerge branch 'tb/doc-max-tree-depth-fix'
Junio C Hamano [Thu, 18 Jul 2024 15:30:26 +0000 (08:30 -0700)] 
Merge branch 'tb/doc-max-tree-depth-fix'

Doc update.

* tb/doc-max-tree-depth-fix:
  Documentation: fix default value for core.maxTreeDepth

11 months agoMerge branch 'ch/refs-without-the-repository-fix'
Junio C Hamano [Thu, 18 Jul 2024 15:30:25 +0000 (08:30 -0700)] 
Merge branch 'ch/refs-without-the-repository-fix'

Comment fix.

* ch/refs-without-the-repository-fix:
  refs: correct the version numbers in a comment

11 months agoPost 2.46-rc0 batch #3
Junio C Hamano [Wed, 17 Jul 2024 17:47:05 +0000 (10:47 -0700)] 
Post 2.46-rc0 batch #3

Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoMerge branch 'js/unit-test-oidtree-cmake-fix'
Junio C Hamano [Wed, 17 Jul 2024 17:47:27 +0000 (10:47 -0700)] 
Merge branch 'js/unit-test-oidtree-cmake-fix'

Build fix.

* js/unit-test-oidtree-cmake-fix:
  cmake: fix build of `t-oidtree`

11 months agoMerge branch 'js/var-git-shell-path'
Junio C Hamano [Wed, 17 Jul 2024 17:47:27 +0000 (10:47 -0700)] 
Merge branch 'js/var-git-shell-path'

"git var GIT_SHELL_PATH" should report the path to the shell used
to spawn external commands, but it didn't do so on Windows, which
has been corrected.

* js/var-git-shell-path:
  var(win32): do report the GIT_SHELL_PATH that is actually used
  run-command: declare the `git_shell_path()` function globally
  run-command(win32): resolve the path to the Unix shell early
  mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too
  win32: override `fspathcmp()` with a directory separator-aware version
  strvec: declare the `strvec_push_nodup()` function globally
  run-command: refactor getting the Unix shell path into its own function

11 months agoMerge branch 'ps/doc-http-empty-cookiefile'
Junio C Hamano [Wed, 17 Jul 2024 17:47:26 +0000 (10:47 -0700)] 
Merge branch 'ps/doc-http-empty-cookiefile'

What happens when http.cookieFile gets the special value "" has
been clarified in the documentation.

* ps/doc-http-empty-cookiefile:
  doc: update http.cookieFile with in-memory cookie processing

11 months agoMerge branch 'kn/push-empty-fix'
Junio C Hamano [Wed, 17 Jul 2024 17:47:26 +0000 (10:47 -0700)] 
Merge branch 'kn/push-empty-fix'

"git push '' HEAD:there" used to hit a BUG(); it has been corrected
to die with "fatal: bad repository ''".

* kn/push-empty-fix:
  builtin/push: call set_refspecs after validating remote

11 months agoMerge branch 'jc/http-cookiefile'
Junio C Hamano [Wed, 17 Jul 2024 17:47:25 +0000 (10:47 -0700)] 
Merge branch 'jc/http-cookiefile'

The http.cookieFile and http.saveCookies configuration variables
have a few values that need to be avoided, which are now ignored
with warning messages.

* jc/http-cookiefile:
  http.c: cookie file tightening

11 months agoMerge branch 'jk/test-body-in-here-doc'
Junio C Hamano [Wed, 17 Jul 2024 17:47:24 +0000 (10:47 -0700)] 
Merge branch 'jk/test-body-in-here-doc'

The test framework learned to take the test body not as a single
string but as a here-document.

* jk/test-body-in-here-doc:
  t/.gitattributes: ignore whitespace in chainlint expect files
  t: convert some here-doc test bodies
  test-lib: allow test snippets as here-docs
  chainlint.pl: add tests for test body in heredoc
  chainlint.pl: recognize test bodies defined via heredoc
  chainlint.pl: check line numbers in expected output
  chainlint.pl: force CRLF conversion when opening input files
  chainlint.pl: do not spawn more threads than we have scripts
  chainlint.pl: only start threads if jobs > 1
  chainlint.pl: add test_expect_success call to test snippets

11 months agoMerge branch 'rj/test-sanitize-leak-log-fix'
Junio C Hamano [Wed, 17 Jul 2024 17:47:24 +0000 (10:47 -0700)] 
Merge branch 'rj/test-sanitize-leak-log-fix'

Tests that use GIT_TEST_SANITIZE_LEAK_LOG feature got their exit
status inverted, which has been corrected.

* rj/test-sanitize-leak-log-fix:
  test-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by default
  test-lib: fix GIT_TEST_SANITIZE_LEAK_LOG

11 months agoDocumentation: fix default value for core.maxTreeDepth
Taylor Blau [Wed, 17 Jul 2024 13:17:31 +0000 (09:17 -0400)] 
Documentation: fix default value for core.maxTreeDepth

When `core.maxTreeDepth` was originally introduced via be20128bfa (add
core.maxTreeDepth config, 2023-08-31), its default value was 4096.

There have since been a couple of updates to its default value that were
not reflected in the documentation for `core.maxTreeDepth`:

  - 4d5693ba05 (lower core.maxTreeDepth default to 2048, 2023-08-31)
  - b64d78ad02 (max_tree_depth: lower it for MSVC to avoid stack
    overflows, 2023-11-01)

Commit 4d5693ba05 lowers the default to 2048 for platforms with smaller
stack sizes, and commit b64d78ad02 lowers the default even further when
Git is compiled with MSVC.

Neither of these changes were reflected in the documentation, which I
noticed while merging newer releases back into GitHub's private fork
(which contained the original implementation of `core.maxTreeDepth`).

Update the documentation to reflect what the platform-specific default
values are.

Noticed-by: Keith W. Campbell <keithc@ca.ibm.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoDocumentation/glossary: fix double word
Martin Ågren [Wed, 17 Jul 2024 10:54:29 +0000 (12:54 +0200)] 
Documentation/glossary: fix double word

Remove a spurious "that".

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoDocumentation/gitpacking: make sample configs listing blocks
Martin Ågren [Wed, 17 Jul 2024 10:54:28 +0000 (12:54 +0200)] 
Documentation/gitpacking: make sample configs listing blocks

This document contains a few sample config snippets. At least with
Asciidoctor, the section headers are rendered *more* indented than the
variables that follow:

       [bitmapPseudoMerge "all"]
    pattern = "refs/"
    ...

To address this, wrap these listings in AsciiDoc listing blocks. Remove
the indentation from the section headings. This is similar to how we
handle such sample config elsewhere, e.g., in config.txt.

While we're here, fix the nearby "wiht" typo.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agot4153: stop redirecting input from /dev/zero
Jeff King [Wed, 17 Jul 2024 07:00:50 +0000 (03:00 -0400)] 
t4153: stop redirecting input from /dev/zero

Commit 852a171018 (am: let command-line options override saved options,
2015-08-04) redirected a few "git am" invocations from /dev/zero, even
though it did not expect "am" to read the input. This was necessary at
the time because those tests used test_terminal, and as described in
18d8c26930 (test_terminal: redirect child process' stdin to a pty,
2015-08-04):

  Note that due to the way the code is structured, the child's stdin
  pseudo-tty will be closed when we finish reading from our stdin. This
  means that in the common case, where our stdin is attached to /dev/null,
  the child's stdin pseudo-tty will be closed immediately. Some operations
  like isatty(), which git-am uses, require the file descriptor to be
  open, and hence if the success of the command depends on such functions,
  test_terminal's stdin should be redirected to a source with large amount
  of data to ensure that the child's stdin is not closed, e.g.

              test_terminal git am --3way </dev/zero

But we later dropped the use of test_terminal in 53ce2e3f0a (am: add
explicit "--retry" option, 2024-06-06). That commit dropped one of the
redirections from /dev/zero but not the other.

In theory the remaining one should not cause any problems, but it turns
out that at least one platform (NonStop) does not have /dev/zero at all.
We never noticed before because it also did not pass the TTY prereq,
meaning these tests were not run at all there until 53ce2e3f0a.

So let's drop the useless /dev/zero mention. There are others in the
test suite, but they are run only for tests marked with EXPENSIVE (so
not typically by default).

Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoPost 2.46-rc0 batch #2
Junio C Hamano [Tue, 16 Jul 2024 18:18:39 +0000 (11:18 -0700)] 
Post 2.46-rc0 batch #2

Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoMerge branch 'bc/gitfaq-more'
Junio C Hamano [Tue, 16 Jul 2024 18:18:58 +0000 (11:18 -0700)] 
Merge branch 'bc/gitfaq-more'

A handful of entries are added to the GitFAQ document.

* bc/gitfaq-more:
  doc: mention that proxies must be completely transparent
  gitfaq: add entry about syncing working trees
  gitfaq: give advice on using eol attribute in gitattributes
  gitfaq: add documentation on proxies

11 months agoMerge branch 'bc/http-proactive-auth'
Junio C Hamano [Tue, 16 Jul 2024 18:18:57 +0000 (11:18 -0700)] 
Merge branch 'bc/http-proactive-auth'

The http transport can now be told to send request with
authentication material without first getting a 401 response.

* bc/http-proactive-auth:
  http: allow authenticating proactively

11 months agoMerge branch 'jc/where-is-bash-for-ci'
Junio C Hamano [Tue, 16 Jul 2024 18:18:57 +0000 (11:18 -0700)] 
Merge branch 'jc/where-is-bash-for-ci'

Shell script clean-up.

* jc/where-is-bash-for-ci:
  ci: unify bash calling convention

11 months agoMerge branch 'ds/advice-sparse-index-expansion'
Junio C Hamano [Tue, 16 Jul 2024 18:18:56 +0000 (11:18 -0700)] 
Merge branch 'ds/advice-sparse-index-expansion'

A new warning message is issued when a command has to expand a
sparse index to handle working tree cruft that are outside of the
sparse checkout.

* ds/advice-sparse-index-expansion:
  advice: warn when sparse index expands

11 months agoMerge branch 'cb/send-email-sanitize-trailer-addresses'
Junio C Hamano [Tue, 16 Jul 2024 18:18:56 +0000 (11:18 -0700)] 
Merge branch 'cb/send-email-sanitize-trailer-addresses'

Address-looking strings found on the trailer are now placed on the
Cc: list after running through sanitize_address by "git send-email".
* cb/send-email-sanitize-trailer-addresses:
  git-send-email: use sanitized address when reading mbox body

11 months agoMerge branch 'en/ort-inner-merge-error-fix'
Junio C Hamano [Tue, 16 Jul 2024 18:18:55 +0000 (11:18 -0700)] 
Merge branch 'en/ort-inner-merge-error-fix'

The "ort" merge backend saw one bugfix for a crash that happens
when inner merge gets killed, and assorted code clean-ups.

* en/ort-inner-merge-error-fix:
  merge-ort: fix missing early return
  merge-ort: convert more error() cases to path_msg()
  merge-ort: upon merge abort, only show messages causing the abort
  merge-ort: loosen commented requirements
  merge-ort: clearer propagation of failure-to-function from merge_submodule
  merge-ort: fix type of local 'clean' var in handle_content_merge ()
  merge-ort: maintain expected invariant for priv member
  merge-ort: extract handling of priv member into reusable function

11 months agorefs: correct the version numbers in a comment
Christian Hesse [Tue, 16 Jul 2024 09:55:44 +0000 (11:55 +0200)] 
refs: correct the version numbers in a comment

The paragraph talks about a change made in c8f815c2 (refs: remove
functions without ref store, 2024-05-07), which is v2.46.0-rc0~119^2
and will be published as part of v2.46, not v2.45.

Signed-off-by: Christian Hesse <mail@eworm.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoPost 2.46-rc0 batch #1
Junio C Hamano [Mon, 15 Jul 2024 17:11:12 +0000 (10:11 -0700)] 
Post 2.46-rc0 batch #1

Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoMerge branch 'cp/unit-test-reftable-record'
Junio C Hamano [Mon, 15 Jul 2024 17:11:43 +0000 (10:11 -0700)] 
Merge branch 'cp/unit-test-reftable-record'

A test in reftable library has been rewritten using the unit test
framework.

* cp/unit-test-reftable-record:
  t-reftable-record: add tests for reftable_log_record_compare_key()
  t-reftable-record: add tests for reftable_ref_record_compare_name()
  t-reftable-record: add index tests for reftable_record_is_deletion()
  t-reftable-record: add obj tests for reftable_record_is_deletion()
  t-reftable-record: add log tests for reftable_record_is_deletion()
  t-reftable-record: add ref tests for reftable_record_is_deletion()
  t-reftable-record: add comparison tests for obj records
  t-reftable-record: add comparison tests for index records
  t-reftable-record: add comparison tests for ref records
  t-reftable-record: add reftable_record_cmp() tests for log records
  t: move reftable/record_test.c to the unit testing framework

11 months agoMerge branch 'jc/disable-push-nego-for-deletion'
Junio C Hamano [Mon, 15 Jul 2024 17:11:43 +0000 (10:11 -0700)] 
Merge branch 'jc/disable-push-nego-for-deletion'

"git push" that pushes only deletion gave an unnecessary and
harmless error message when push negotiation is configured, which
has been corrected.

* jc/disable-push-nego-for-deletion:
  push: avoid showing false negotiation errors

11 months agoMerge branch 'ri/doc-show-branch-fix'
Junio C Hamano [Mon, 15 Jul 2024 17:11:43 +0000 (10:11 -0700)] 
Merge branch 'ri/doc-show-branch-fix'

Docfix.

* ri/doc-show-branch-fix:
  doc: fix the max number of branches shown by "show-branch"

11 months agoMerge branch 'tb/dev-build-pedantic-fix'
Junio C Hamano [Mon, 15 Jul 2024 17:11:42 +0000 (10:11 -0700)] 
Merge branch 'tb/dev-build-pedantic-fix'

Developer build procedure fix.

* tb/dev-build-pedantic-fix:
  config.mak.dev: fix typo when enabling -Wpedantic

11 months agoMerge branch 'rs/clang-format-updates'
Junio C Hamano [Mon, 15 Jul 2024 17:11:42 +0000 (10:11 -0700)] 
Merge branch 'rs/clang-format-updates'

Custom control structures we invented more recently have been
taught to the clang-format file.

* rs/clang-format-updates:
  clang-format: include kh_foreach* macros in ForEachMacros

11 months agoMerge branch 'am/gitweb-feed-use-committer-date'
Junio C Hamano [Mon, 15 Jul 2024 17:11:41 +0000 (10:11 -0700)] 
Merge branch 'am/gitweb-feed-use-committer-date'

GitWeb update to use committer date consistently in rss/atom feeds.

* am/gitweb-feed-use-committer-date:
  gitweb: rss/atom change published/updated date to committer date

11 months agoMerge branch 'jk/tests-without-dns'
Junio C Hamano [Mon, 15 Jul 2024 17:11:41 +0000 (10:11 -0700)] 
Merge branch 'jk/tests-without-dns'

Test suite has been taught not to unnecessarily rely on DNS failing
a bogus external name.

* jk/tests-without-dns:
  t/lib-bundle-uri: use local fake bundle URLs
  t5551: do not confirm that bogus url cannot be used
  t5553: use local url for invalid fetch

11 months agoMerge branch 'gt/unit-test-oidmap'
Junio C Hamano [Mon, 15 Jul 2024 17:11:40 +0000 (10:11 -0700)] 
Merge branch 'gt/unit-test-oidmap'

An existing test of oidmap API has been rewritten with the
unit-test framework.

* gt/unit-test-oidmap:
  t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c

11 months agoMerge branch 'as/describe-broken-refresh-index-fix'
Junio C Hamano [Mon, 15 Jul 2024 17:11:40 +0000 (10:11 -0700)] 
Merge branch 'as/describe-broken-refresh-index-fix'

"git describe --dirty --broken" forgot to refresh the index before
seeing if there is any chang, ("git describe --dirty" correctly did
so), which has been corrected.

* as/describe-broken-refresh-index-fix:
  describe: refresh the index when 'broken' flag is used

11 months agoMerge branch 'rj/t0613-no-longer-leaks'
Junio C Hamano [Mon, 15 Jul 2024 17:11:39 +0000 (10:11 -0700)] 
Merge branch 'rj/t0613-no-longer-leaks'

A test that no longer leaks has been marked as such.

* rj/t0613-no-longer-leaks:
  t0613: mark as leak-free

11 months agoMerge branch 'rj/t0612-no-longer-leaks'
Junio C Hamano [Mon, 15 Jul 2024 17:11:39 +0000 (10:11 -0700)] 
Merge branch 'rj/t0612-no-longer-leaks'

A test that no longer leaks has been marked as such.

* rj/t0612-no-longer-leaks:
  t0612: mark as leak-free

11 months agovar(win32): do report the GIT_SHELL_PATH that is actually used
Johannes Schindelin [Sat, 13 Jul 2024 21:08:24 +0000 (21:08 +0000)] 
var(win32): do report the GIT_SHELL_PATH that is actually used

On Windows, Unix-like paths like `/bin/sh` make very little sense. In
the best case, they simply don't work, in the worst case they are
misinterpreted as absolute paths that are relative to the drive
associated with the current directory.

To that end, Git does not actually use the path `/bin/sh` that is
recorded e.g. when `run_command()` is called with a Unix shell
command-line. Instead, as of 776297548e (Do not use SHELL_PATH from
build system in prepare_shell_cmd on Windows, 2012-04-17), it
re-interprets `/bin/sh` as "look up `sh` on the `PATH` and use the
result instead".

This is the logic users expect to be followed when running `git var
GIT_SHELL_PATH`.

However, when 1e65721227 (var: add support for listing the shell,
2023-06-27) introduced support for `git var GIT_SHELL_PATH`, Windows was
not special-cased as above, which is why it outputs `/bin/sh` even
though that disagrees with what Git actually uses.

Let's fix this by using the exact same logic as `prepare_shell_cmd()`,
adjusting the Windows-specific `git var GIT_SHELL_PATH` test case to
verify that it actually finds a working executable.

Reported-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agorun-command: declare the `git_shell_path()` function globally
Johannes Schindelin [Sat, 13 Jul 2024 21:08:23 +0000 (21:08 +0000)] 
run-command: declare the `git_shell_path()` function globally

The intention is to use it in `git var GIT_SHELL_PATH`, therefore we
need this function to stop being file-local only.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agorun-command(win32): resolve the path to the Unix shell early
Johannes Schindelin [Sat, 13 Jul 2024 21:08:22 +0000 (21:08 +0000)] 
run-command(win32): resolve the path to the Unix shell early

In 776297548e (Do not use SHELL_PATH from build system in
prepare_shell_cmd on Windows, 2012-04-17), the hard-coded path to the
Unix shell was replaced by passing `sh` instead when executing Unix
shell scripts in Git.

This was done because the hard-coded path to the Unix shell is incorrect
on Windows because it not only is a Unix-style absolute path instead of
a Windows one, but Git uses the runtime prefix feature on Windows, i.e.
the correct path cannot be hard-coded.

Naturally, the `sh` argument will be resolved to the full path of said
executable eventually.

To help fixing the bug where `git var GIT_SHELL_PATH` currently does not
reflect that logic, but shows that incorrect hard-coded Unix-style
absolute path, let's resolve the full path to the `sh` executable early
in the `git_shell_path()` function so that we can use it in `git var`,
too, and be sure that the output is equivalent to what `run_command()`
does when it is asked to execute a command-line using a Unix shell.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agomingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too
Johannes Schindelin [Sat, 13 Jul 2024 21:08:21 +0000 (21:08 +0000)] 
mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too

Whether the full path to the MSYS2 Bash is specified using backslashes
or forward slashes, in either case the command-line arguments need to be
quoted in the MSYS2-specific manner instead of using regular Win32
command-line quoting rules.

In preparation for `prepare_shell_cmd()` to use the full path to
`sh.exe` (with forward slashes for consistency), let's teach the
`is_msys2_sh()` function about this; Otherwise 5580.4 'clone with
backslashed path' would fail once `prepare_shell_cmd()` uses the full
path instead of merely `sh`.

This patch relies on the just-introduced fix where `fspathcmp()` handles
backslashes and forward slashes as equivalent on Windows.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agowin32: override `fspathcmp()` with a directory separator-aware version
Johannes Schindelin [Sat, 13 Jul 2024 21:08:20 +0000 (21:08 +0000)] 
win32: override `fspathcmp()` with a directory separator-aware version

On Windows, the backslash is the directory separator, even if the
forward slash can be used, too, at least since Windows NT.

This means that the paths `a/b` and `a\b` are equivalent, and
`fspathcmp()` needs to be made aware of that fact.

Note that we have to override both `fspathcmp()` and `fspathncmp()`, and
the former cannot be a mere pre-processor constant that transforms calls
to `fspathcmp(a, b)` into `fspathncmp(a, b, (size_t)-1)` because the
function `report_collided_checkout()` in `unpack-trees.c` wants to
assign `list.cmp = fspathcmp`.

Also note that `fspatheq()` does _not_ need to be overridden because it
calls `fspathcmp()` internally.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agostrvec: declare the `strvec_push_nodup()` function globally
Johannes Schindelin [Sat, 13 Jul 2024 21:08:19 +0000 (21:08 +0000)] 
strvec: declare the `strvec_push_nodup()` function globally

This function differs from `strvec_push()` in that it takes ownership of
the allocated string that is passed as second argument.

This is useful when appending elements to the string array that have
been freshly allocated and serve no further other purpose after that.

Without declaring this function globally, call sites would allocate the
memory, only to have `strvec_push()` duplicate the string, and then the
first copy would need to be released. Having this function globally
avoids that kind of unnecessary work.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agorun-command: refactor getting the Unix shell path into its own function
Johannes Schindelin [Sat, 13 Jul 2024 21:08:18 +0000 (21:08 +0000)] 
run-command: refactor getting the Unix shell path into its own function

This encapsulates the platform-specific logic better.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agocmake: fix build of `t-oidtree`
Johannes Schindelin [Fri, 12 Jul 2024 20:34:10 +0000 (20:34 +0000)] 
cmake: fix build of `t-oidtree`

When the `oidtree` test helper was turned into a unit test, a new
`lib-oid` source file was added as dependency. This was only done in the
Makefile so far, but also needs to be done in the CMake definition.

This is a companion of ed548408723d (t/: migrate helper/test-oidtree.c
to unit-tests/t-oidtree.c, 2024-06-08).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agobuiltin/push: call set_refspecs after validating remote
Karthik Nayak [Thu, 11 Jul 2024 09:39:54 +0000 (11:39 +0200)] 
builtin/push: call set_refspecs after validating remote

When an end-user runs "git push" with an empty string for the remote
repository name, e.g.

    $ git push '' main

"git push" fails with a BUG(). Even though this is a nonsense request
that we want to fail, we shouldn't hit a BUG().  Instead we want to give
a sensible error message, e.g., 'bad repository'".

This is because since 9badf97c42 (remote: allow resetting url list,
2024-06-14), we reset the remote URL if the provided URL is empty. When
a user of 'remotes_remote_get' tries to fetch a remote with an empty
repo name, the function initializes the remote via 'make_remote'. But
the remote is still not a valid remote, since the URL is empty, so it
tries to add the URL alias using 'add_url_alias'. This in-turn will call
'add_url', but since the URL is empty we call 'strvec_clear' on the
`remote->url`. Back in 'remotes_remote_get', we again check if the
remote is valid, which fails, so we return 'NULL' for the 'struct
remote *' value.

The 'builtin/push.c' code, calls 'set_refspecs' before validating the
remote. This worked with empty repo names earlier since we would get a
remote, albeit with an empty URL. With the new changes, we get a 'NULL'
remote value, this causes the check for remote to fail and raises the
BUG in 'set_refspecs'.

Do a simple fix by doing remote validation first. Also add a test to
validate the bug fix. With this, we can also now directly pass remote to
'set_refspecs' instead of it trying to lazily obtain it.

Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoGit 2.46-rc0 v2.46.0-rc0
Junio C Hamano [Fri, 12 Jul 2024 15:40:36 +0000 (08:40 -0700)] 
Git 2.46-rc0

Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agoMerge branch 'rs/simplify-submodule-helper-super-prefix-invocation'
Junio C Hamano [Fri, 12 Jul 2024 15:41:57 +0000 (08:41 -0700)] 
Merge branch 'rs/simplify-submodule-helper-super-prefix-invocation'

Code clean-up.

* rs/simplify-submodule-helper-super-prefix-invocation:
  submodule--helper: use strvec_pushf() for --super-prefix

11 months agoMerge branch 'as/pathspec-h-typofix'
Junio C Hamano [Fri, 12 Jul 2024 15:41:57 +0000 (08:41 -0700)] 
Merge branch 'as/pathspec-h-typofix'

Typofix.

* as/pathspec-h-typofix:
  pathspec: fix typo "glossary-context.txt" -> "glossary-content.txt"

11 months agodoc: update http.cookieFile with in-memory cookie processing
Piotr Szlazak [Thu, 11 Jul 2024 08:36:48 +0000 (08:36 +0000)] 
doc: update http.cookieFile with in-memory cookie processing

Documentation only mentions how to read cookies from the given file
and how to save them to the file using http.saveCookies.

But underlying libcURL allows the HTTP cookies used only in memory;
cookies from the server will be accepted and sent back in successive
requests within same connection, by using an empty string as the
filename.  Document this.

Signed-off-by: Piotr Szlazak <piotr.szlazak@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agotest-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by default
Rubén Justo [Thu, 11 Jul 2024 14:10:51 +0000 (23:10 +0900)] 
test-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by default

As we currently describe in t/README, it can happen that:

    Some tests run "git" (or "test-tool" etc.) without properly checking
    the exit code, or git will invoke itself and fail to ferry the
    abort() exit code to the original caller.

Therefore, GIT_TEST_SANITIZE_LEAK_LOG=true is needed to be set to
capture all memory leaks triggered by our tests.

It seems unnecessary to force users to remember this option, as
forgetting it could lead to missed memory leaks.

We could solve the problem by making it "true" by default, but that
might suggest we think "false" makes sense, which isn't the case.

Therefore, the best approach is to remove the option entirely while
maintaining the capability to detect memory leaks in blind spots of our
tests.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agot/.gitattributes: ignore whitespace in chainlint expect files
Jeff King [Wed, 10 Jul 2024 08:47:04 +0000 (04:47 -0400)] 
t/.gitattributes: ignore whitespace in chainlint expect files

The ".expect" files in t/chainlint/ are snippets of expected output from
the chainlint script, and do not necessarily conform to our usual code
style. Especially with the recent change to retain line numbers, blank
lines in the input script end up with trailing whitespace as we print
"3 " for line 3, for example. The point of these files is to match the
output verbatim, so let's not complain about the trailing spaces.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agot: convert some here-doc test bodies
Jeff King [Wed, 10 Jul 2024 08:39:31 +0000 (04:39 -0400)] 
t: convert some here-doc test bodies

The t1404 script checks a lot of output from Git which contains single
quotes. Because the test snippets are themselves wrapped in the same
single-quotes, we have to resort to using $SQ to match them.  This is
error-prone and makes the tests harder to read.

Instead, let's use the new here-doc feature added in the previous
commit, which lets us write anything in the test body we want (except
the here-doc end marker on a line by itself, of course).

Note that we do use "\" in our marker to avoid interpolation (which is
the whole point). But we don't use "<<-", as we want to preserve
whitespace in the snippet (and running with "-v" before and after shows
that we produce the exact same output, except with the ugly $SQ
references fixed).

I just converted every test here, even though only some of them use
$SQ. But it would be equally correct to mix-and-match styles if we don't
mind the inconsistency.

I've also converted a few tests in t0600 which were moved from t1404 (I
had written this patch before they were moved, but it seemed worth
porting over the changes rather than losing them).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agotest-lib: allow test snippets as here-docs
Jeff King [Wed, 10 Jul 2024 08:39:29 +0000 (04:39 -0400)] 
test-lib: allow test snippets as here-docs

Most test snippets are wrapped in single quotes, like:

  test_expect_success 'some description' '
          do_something
  '

This sometimes makes the snippets awkward to write, because you can't
easily use single quotes within them. We sometimes work around this with
$SQ, or by loosening regexes to use "." instead of a literal quote, or
by using double quotes when we'd prefer to use single-quotes (and just
adding extra backslash-escapes to avoid interpolation).

This commit adds another option: feeding the snippet via the function's
stdin. This doesn't conflict with anything the snippet would want to do,
because we always redirect its stdin from /dev/null anyway (which we'll
continue to do).

A few notes on the implementation:

  - it would be nice to push this down into test_run_, but we can't, as
    test_expect_success and test_expect_failure want to see the actual
    script content to report it for verbose-mode. A helper function
    limits the amount of duplication in those callers here.

  - The helper function is a little awkward to call, as you feed it the
    name of the variable you want to set. The more natural thing in
    shell would be command substitution like:

      body=$(body_or_stdin "$2")

    but that loses trailing whitespace. There are tricks around this,
    like:

      body=$(body_or_stdin "$2"; printf .)
      body=${body%.}

    but we'd prefer to keep such tricks in the helper, not in each
    caller.

  - I implemented the helper using a sequence of "read" calls. Together
    with "-r" and unsetting the IFS, this preserves incoming whitespace.
    An alternative is to use "cat" (which then requires the gross "."
    trick above). But this saves us a process, which is probably a good
    thing. The "read" builtin does use more read() syscalls than
    necessary (one per byte), but that is almost certainly a win over a
    separate process.

    Both are probably slower than passing a single-quoted string, but
    the difference is lost in the noise for a script that I converted as
    an experiment.

  - I handle test_expect_success and test_expect_failure here. If we
    like this style, we could easily extend it to other spots (e.g.,
    lazy_prereq bodies) on top of this patch.

  - even though we are using "local", we have to be careful about our
    variable names. Within test_expect_success, any variable we declare
    with local will be seen as local by the test snippets themselves (so
    it wouldn't persist between tests like normal variables would).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agochainlint.pl: add tests for test body in heredoc
Jeff King [Wed, 10 Jul 2024 08:39:16 +0000 (04:39 -0400)] 
chainlint.pl: add tests for test body in heredoc

The chainlint.pl script recently learned about the upcoming:

  test_expect_success 'some test' - <<\EOT
TEST_BODY
  EOT

syntax, where TEST_BODY should be checked in the usual way. Let's make
sure this works by adding a few tests. The "here-doc-body" file tests
the basic syntax, including an embedded here-doc which we should still
be able to recognize.

Likewise the "here-doc-body-indent" checks the same thing, but using the
"<<-" operator. We wouldn't expect this to be used normally, but we
would not want to accidentally miss a body that uses it. The
"pathological" variant checks the opposite: we don't get confused by an
indented tag within the here-doc body.

The "here-doc-double" tests the handling of two here-doc tags on the
same line. This is not something we'd expect anybody to do in practice,
but the code was written defensively to handle this, so let's make sure
it works.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agochainlint.pl: recognize test bodies defined via heredoc
Eric Sunshine [Wed, 10 Jul 2024 08:38:31 +0000 (04:38 -0400)] 
chainlint.pl: recognize test bodies defined via heredoc

In order to check tests for semantic problems, chainlint.pl scans test
scripts, looking for tests defined as:

    test_expect_success [prereq] title '
        body
    '

where `body` is a single string which is then treated as a standalone
chunk of code and "linted" to detect semantic issues. (The same happens
for `test_expect_failure` definitions.)

The introduction of test definitions in which the test body is instead
presented via a heredoc rather than as a single string creates a blind
spot in the linting process since such invocations are not recognized by
chainlint.pl.

Prepare for this new style by also recognizing tests defined as:

    test_expect_success [prereq] title - <<\EOT
        body
    EOT

A minor complication is that chainlint.pl has never considered heredoc
bodies significant since it doesn't scan them for semantic problems,
thus it has always simply thrown them away. However, with the new
`test_expect_success` calling sequence, heredoc bodies become
meaningful, thus need to be captured.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agochainlint.pl: check line numbers in expected output
Jeff King [Wed, 10 Jul 2024 08:37:55 +0000 (04:37 -0400)] 
chainlint.pl: check line numbers in expected output

While working on chainlint.pl recently, we introduced some bugs that
showed incorrect line numbers in the output. But it was hard to notice,
since we sanitize the output by removing all of the line numbers! It
would be nice to retain these so we can catch any regressions.

The main reason we sanitize is for maintainability: we concatenate all
of the test snippets into a single file, so it's hard for each ".expect"
file to know at which offset its test input will be found. We can handle
that by storing the per-test line numbers in the ".expect" files, and
then dynamically offsetting them as we build the concatenated test and
expect files together.

The changes to the ".expect" files look like tedious boilerplate, but it
actually makes adding new tests easier. You can now just run:

  perl chainlint.pl chainlint/foo.test |
  tail -n +2 >chainlint/foo.expect

to save the output of the script minus the comment headers (after
checking that it is correct, of course). Whereas before you had to strip
the line numbers. The conversions here were done mechanically using
something like the script above, and then spot-checked manually.

It would be possible to do all of this in shell via the Makefile, but it
gets a bit complicated (and requires a lot of extra processes). Instead,
I've written a short perl script that generates the concatenated files
(we already depend on perl, since chainlint.pl uses it). Incidentally,
this improves a few other things:

  - we incorrectly used $(CHAINLINTTMP_SQ) inside a double-quoted
    string. So if your test directory required quoting, like:

       make "TEST_OUTPUT_DIRECTORY=/tmp/h'orrible"

    we'd fail the chainlint tests.

  - the shell in the Makefile didn't handle &&-chaining correctly in its
    loops (though in practice the "sed" and "cat" invocations are not
    likely to fail).

  - likewise, the sed invocation to strip numbers was hiding the exit
    code of chainlint.pl itself. In practice this isn't a big deal;
    since there are linter violations in the test files, we expect it to
    exit non-zero. But we could later use exit codes to distinguish
    serious errors from expected ones.

  - we now use a constant number of processes, instead of scaling with
    the number of test scripts. So it should be a little faster (on my
    machine, "make check-chainlint" goes from 133ms to 73ms).

There are some alternatives to this approach, but I think this is still
a good intermediate step:

  1. We could invoke chainlint.pl individually on each test file, and
     compare it to the expected output (and possibly using "make" to
     avoid repeating already-done checks). This is a much bigger change
     (and we'd have to figure out what to do with the "# LINT" lines in
     the inputs). But in this case we'd still want the "expect" files to
     be annotated with line numbers. So most of what's in this patch
     would be needed anyway.

  2. Likewise, we could run a single chainlint.pl and feed it all of the
     scripts (with "--jobs=1" to get deterministic output). But we'd
     still need to annotate the scripts as we did here, and we'd still
     need to either assemble the "expect" file, or break apart the
     script output to compare to each individual ".expect" file.

So we may pursue those in the long run, but this patch gives us more
robust tests without too much extra work or moving in a useless
direction.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agochainlint.pl: force CRLF conversion when opening input files
Jeff King [Wed, 10 Jul 2024 08:37:30 +0000 (04:37 -0400)] 
chainlint.pl: force CRLF conversion when opening input files

The lexer in chainlint.pl can't handle CRLF line endings; it complains
about an internal error in scan_token() if we see one. For example, in
our Windows CI environment:

  $ perl chainlint.pl chainlint/for-loop.test | cat -v
  Thread 2 terminated abnormally: internal error scanning character '^M'

This doesn't break "make check-chainlint" (yet), because we assemble a
concatenated input by passing the contents of each file through "sed".
And the "sed" we use will strip out the CRLFs. But the next patch is
going to rework this a bit, which does break check-chainlint on Windows.
Plus it's probably nicer to folks on Windows who might work on chainlint
itself and write new tests.

In theory we could fix the parser to handle this, but it's not really
worth the trouble. We should be able to ask the input layer to translate
the line endings for us. In fact, I'd expect this to happen by default,
as perl's documentation claims Win32 uses the ":unix:crlf" PERLIO layer
by default ("unix" here just refers to using read/write syscalls, and
then "crlf" layers the translation on top). However, this doesn't seem
to be the case in our Windows CI environment. I didn't dig into the
exact reason, but it is perhaps because we are using an msys build of
perl rather than a "true" Win32 build.

At any rate, it is easy-ish to just ask explicitly for the conversion.
In the above example, setting PERLIO=crlf in the environment is enough
to make it work. Curiously, though, this doesn't work when invoking
chainlint via "make". Again, I didn't dig into it, but it may have to do
with msys programs calling Windows programs or vice versa.

We can make it work consistently by just explicitly asking for CRLF
translation when we open the files. This will even work on non-Windows
platforms, though we wouldn't really expect to find CRLF files there.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agochainlint.pl: do not spawn more threads than we have scripts
Jeff King [Wed, 10 Jul 2024 08:35:57 +0000 (04:35 -0400)] 
chainlint.pl: do not spawn more threads than we have scripts

The chainlint.pl script spawns worker threads to check many scripts in
parallel. This is good if you feed it a lot of scripts. But if you give
it few (or one), then the overhead of spawning the threads dominates. We
can easily notice that we have fewer scripts than threads and scale back
as appropriate.

This patch reduces the time to run:

  time for i in chainlint/*.test; do
perl chainlint.pl $i
  done >/dev/null

on my system from ~4.1s to ~1.1s, where I have 8+8 cores.

As with the previous patch, this isn't the usual way we run chainlint
(we feed many scripts at once, which is why it supports threading in the
first place). So this won't make a big difference in the real world, but
it may help us out in the future, and it makes experimenting with and
debugging the chainlint tests a bit more pleasant.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agochainlint.pl: only start threads if jobs > 1
Jeff King [Wed, 10 Jul 2024 08:35:13 +0000 (04:35 -0400)] 
chainlint.pl: only start threads if jobs > 1

If the system supports threads, chainlint.pl will always spawn worker
threads to do the real work. But when --jobs=1, this is pointless, since
we could just do the work in the main thread. And spawning even a single
thread has a high overhead. For example, on my Linux system, running:

  for i in chainlint/*.test; do
perl chainlint.pl --jobs=1 $i
  done >/dev/null

takes ~1.7s without this patch, and ~1.1s after. We don't usually spawn
a bunch of individual chainlint.pl processes (instead we feed several
scripts at once, and the parallelism outweighs the setup cost). But it's
something we've considered doing, and since we already have fallback
code for systems without thread support, it's pretty easy to make this
work.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agochainlint.pl: add test_expect_success call to test snippets
Jeff King [Wed, 10 Jul 2024 08:34:48 +0000 (04:34 -0400)] 
chainlint.pl: add test_expect_success call to test snippets

The chainlint tests are a series of individual files, each holding a
test body. The "make check-chainlint" target assembles them into a
single file, adding a "test_expect_success" function call around each.
Let's instead include that function call in the files themselves. This
is a little more boilerplate, but has several advantages:

  1. You can now run chainlint manually on snippets with just "perl
     chainlint.perl chainlint/foo.test". This can make developing and
     debugging a little easier.

  2. Many of the tests implicitly relied on the syntax of the lines
     added by the Makefile (in particular the use of single-quotes).
     This assumption is much easier to see when the single-quotes are
     alongside the test body.

  3. We had no way to test how the chainlint program handled
     various test_expect_success lines themselves. Now we'll be able to
     check variations.

The change to the .test files was done mechanically, using the same
test names they would have been assigned by the Makefile (this is
important to match the expected output). The Makefile has the minimal
change to drop the extra lines; there are more cleanups possible but a
future patch in this series will rewrite this substantially anyway.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agohttp.c: cookie file tightening
Junio C Hamano [Tue, 9 Jul 2024 23:03:48 +0000 (16:03 -0700)] 
http.c: cookie file tightening

The http.cookiefile configuration variable is used to call
curl_easy_setopt() to set CURLOPT_COOKIEFILE and if http.savecookies
is set, the same value is used for CURLOPT_COOKIEJAR.  The former is
used only to read cookies at startup, the latter is used to write
cookies at the end.

The manual pages https://curl.se/libcurl/c/CURLOPT_COOKIEFILE.html
and https://curl.se/libcurl/c/CURLOPT_COOKIEJAR.html talk about two
interesting special values.

 * "" (an empty string) given to CURLOPT_COOKIEFILE means not to
   read cookies from any file upon startup.

 * It is not specified what "" (an empty string) given to
   CURLOPT_COOKIEJAR does; presumably open a file whose name is an
   empty string and write cookies to it?  In any case, that is not
   what we want to see happen, ever.

 * "-" (a dash) given to CURLOPT_COOKIEFILE makes cURL read cookies
   from the standard input, and given to CURLOPT_COOKIEJAR makes
   cURL write cookies to the standard output.  Neither of which we
   want ever to happen.

So, let's make sure we avoid these nonsense cases.  Specifically,
when http.cookies is set to "-", ignore it with a warning, and when
it is set to "" and http.savecookies is set, ignore http.savecookies
with a warning.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
11 months agohttp: allow authenticating proactively
brian m. carlson [Wed, 10 Jul 2024 00:01:55 +0000 (00:01 +0000)] 
http: allow authenticating proactively

When making a request over HTTP(S), Git only sends authentication if it
receives a 401 response.  Thus, if a repository is open to the public
for reading, Git will typically never ask for authentication for fetches
and clones.

However, there may be times when a user would like to authenticate
nevertheless.  For example, a forge may give higher rate limits to users
who authenticate because they are easier to contact in case of excessive
use.  Or it may be useful for a known heavy user, such as an internal
service, to proactively authenticate so its use can be monitored and, if
necessary, throttled.

Let's make this possible with a new option, "http.proactiveAuth".  This
option specifies a type of authentication which can be used to
authenticate against the host in question.  This is necessary because we
lack the WWW-Authenticate header to provide us details; similarly, we
cannot accept certain types of authentication because we require
information from the server, such as a nonce or challenge, to
successfully authenticate.

If we're in auto mode and we got a username and password, set the
authentication scheme to Basic.  libcurl will not send authentication
proactively unless there's a single choice of allowed authentication,
and we know in this case we didn't get an authtype entry telling us what
scheme to use, or we would have taken a different codepath and written
the header ourselves.  In any event, of the other schemes that libcurl
supports, Digest and NTLM require a nonce or challenge, which means that
they cannot work with proactive auth, and GSSAPI does not use a username
and password at all, so Basic is the only logical choice among the
built-in options.

Note that the existing http_proactive_auth variable signifies proactive
auth if there are already credentials, which is different from the
functionality we're adding, which always seeks credentials even if none
are provided.  Nonetheless, t5540 tests the existing behavior for
WebDAV-based pushes to an open repository without credentials, so we
preserve it.  While at first this may seem an insecure and bizarre
decision, it may be that authentication is done with TLS certificates,
in which case it might actually provide a quite high level of security.
Expand the variable to use an enum to handle the additional cases and a
helper function to distinguish our new cases from the old ones.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>