Commit Graph

20236 Commits

Author SHA1 Message Date
Derrick Stolee
4074d3c7e1 clone: set fetch.bundleURI if appropriate
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>
2023-01-31 08:57:48 -08:00
Derrick Stolee
7903efb717 bundle-uri: download in creationToken order
The creationToken heuristic provides an ordering on the bundles
advertised by a bundle list. Teach the Git client to download bundles
differently when this heuristic is advertised.

The bundles in the list are sorted by their advertised creationToken
values, then downloaded in decreasing order. This avoids the previous
strategy of downloading bundles in an arbitrary order and attempting
to apply them (likely failing in the case of required commits) until
discovering the order through attempted unbundling.

During a fresh 'git clone', it may make sense to download the bundles in
increasing order, since that would prevent the need to attempt
unbundling a bundle with required commits that do not exist in our empty
object store. The cost of testing an unbundle is quite low, and instead
the chosen order is optimizing for a future bundle download during a
'git fetch' operation with a non-empty object store.

Since the Git client continues fetching from the Git remote after
downloading and unbundling bundles, the client's object store can be
ahead of the bundle provider's object store. The next time it attempts
to download from the bundle list, it makes most sense to download only
the most-recent bundles until all tips successfully unbundle. The
strategy implemented here provides that short-circuit where the client
downloads a minimal set of bundles.

However, we are not satisfied by the naive approach of downloading
bundles until one successfully unbundles, expecting the earlier bundles
to successfully unbundle now. The example repository in t5558
demonstrates this well:

 ---------------- bundle-4

       4
      / \
 ----|---|------- bundle-3
     |   |
     |   3
     |   |
 ----|---|------- bundle-2
     |   |
     2   |
     |   |
 ----|---|------- bundle-1
      \ /
       1
       |
 (previous commits)

In this repository, if we already have the objects for bundle-1 and then
try to fetch from this list, the naive approach will fail. bundle-4
requires both bundle-3 and bundle-2, though bundle-3 will successfully
unbundle without bundle-2. Thus, the algorithm needs to keep this in
mind.

A later implementation detail will store the maximum creationToken seen
during such a bundle download, and the client will avoid downloading a
bundle unless its creationToken is strictly greater than that stored
value. For now, if the client seeks to download from an identical
bundle list since its previous download, it will download the
most-recent bundle then stop since its required commits are already in
the object store.

Add tests that exercise this behavior, but we will expand upon these
tests when incremental downloads during 'git fetch' make use of
creationToken values.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-31 08:57:48 -08:00
Derrick Stolee
512fccf8a5 bundle-uri: parse bundle.<id>.creationToken values
The previous change taught Git to parse the bundle.heuristic value,
especially when its value is "creationToken". Now, teach Git to parse
the bundle.<id>.creationToken values on each bundle in a bundle list.

Before implementing any logic based on creationToken values for the
creationToken heuristic, parse and print these values for testing
purposes.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-31 08:57:48 -08:00
Derrick Stolee
c93c3d2fa4 bundle-uri: parse bundle.heuristic=creationToken
The bundle.heuristic value communicates that the bundle list is
organized to make use of the bundle.<id>.creationToken values that may
be provided in the bundle list. Those values will create a total order
on the bundles, allowing the Git client to download them in a specific
order and even remember previously-downloaded bundles by storing the
maximum creation token value.

Before implementing any logic that parses or uses the
bundle.<id>.creationToken values, teach Git to parse the
bundle.heuristic value from a bundle list. We can use 'test-tool
bundle-uri' to print the heuristic value and verify that the parsing
works correctly.

As an extra precaution, create the internal 'heuristics' array to be a
list of (enum, string) pairs so we can iterate through the array entries
carefully, regardless of the enum values.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-31 08:57:48 -08:00
Derrick Stolee
7bc73e7b61 t5558: add tests for creationToken heuristic
As documented in the bundle URI design doc in 2da14fad8f (docs:
document bundle URI standard, 2022-08-09), the 'creationToken' member of
a bundle URI allows a bundle provider to specify a total order on the
bundles.

Future changes will allow the Git client to understand these members and
modify its behavior around downloading the bundles in that order. In the
meantime, create tests that add creation tokens to the bundle list. For
now, the Git client correctly ignores these unknown keys.

Create a new test helper function, test_remote_https_urls, which filters
GIT_TRACE2_EVENT output to extract a list of URLs passed to
git-remote-https child processes. This can be used to verify the order
of these requests as we implement the creationToken heuristic. For now,
we need to sort the actual output since the current client does not have
a well-defined order that it applies to the bundles.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-31 08:57:47 -08:00
Derrick Stolee
d9fd674c8b bundle: verify using check_connected()
When Git verifies a bundle to see if it is safe for unbundling, it first
looks to see if the prerequisite commits are in the object store. This
is an easy way to "fail fast" but it is not a sufficient check for
updating refs that guarantee closure under reachability. There could
still be issues if those commits are not reachable from the repository's
references. The repository only has guarantees that its object store is
closed under reachability for the objects that are reachable from
references.

Thus, the code in verify_bundle() has previously had the additional
check that all prerequisite commits are reachable from repository
references. This is done via a revision walk from all references,
stopping only if all prerequisite commits are discovered or all commits
are walked. This uses a custom walk to verify_bundle().

This check is more strict than what Git applies to fetched pack-files.
In the fetch case, Git guarantees that the new references are closed
under reachability by walking from the new references until walking
commits that are reachable from repository refs. This is done through
the well-used check_connected() method.

To better align with the restrictions required by 'git fetch',
reimplement this check in verify_bundle() to use check_connected(). This
also simplifies the code significantly.

The previous change added a test that verified the behavior of 'git
bundle verify' and 'git bundle unbundle' in this case, and the error
messages looked like this:

  error: Could not read <missing-commit>
  fatal: Failed to traverse parents of commit <extant-commit>

However, by changing the revision walk slightly within check_connected()
and using its quiet mode, we can omit those messages. Instead, we get
only this message, tailored to describing the current state of the
repository:

  error: some prerequisite commits exist in the object store,
         but are not connected to the repository's history

(Line break added here for the commit message formatting, only.)

While this message does not include any object IDs, there is no
guarantee that those object IDs would help the user diagnose what is
going on, as they could be separated from the prerequisite commits by
some distance. At minimum, this situation describes the situation in a
more informative way than the previous error messages.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-31 08:57:47 -08:00
Derrick Stolee
e72171f085 bundle: test unbundling with incomplete history
When verifying a bundle, Git checks first that all prerequisite commits
exist in the object store, then adds an additional check: those
prerequisite commits must be reachable from references in the
repository.

This check is stronger than what is checked for refs being added during
'git fetch', which simply guarantees that the new refs have a complete
history up to the point where it intersects with the current reachable
history.

However, we also do not have any tests that check the behavior under
this condition. Create a test that demonstrates its behavior.

In order to construct a broken history, perform a shallow clone of a
repository with a linear history, but whose default branch ('base') has
a single commit, so dropping the shallow markers leaves a complete
history from that reference. However, the 'tip' reference adds a
shallow commit whose parent is missing in the cloned repository. Trying
to unbundle a bundle with the 'tip' as a prerequisite will succeed past
the object store check and move into the reachability check.

The two errors that are reported are of this form:

  error: Could not read <missing-commit>
  fatal: Failed to traverse parents of commit <present-commit>

These messages are not particularly helpful for the person running the
unbundle command, but they do prevent the command from succeeding.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-31 08:57:47 -08:00
Jeff King
d4e241a145 test-bundle-uri: drop unused variables
Commit 70b9c10373 (bundle-uri client: add helper for testing server,
2022-12-22) added a cmd_ls_remote() function which contains "uploadpack"
and "server_options" variables. Neither of these variables is ever
modified after being initialized, so the code to handle non-NULL and
non-empty values is impossible to reach.

While in theory we might add command-line parsing to set these, let's
drop the dead code for now in the name of cleanliness. It's easy enough
to add it back later if need be.

Noticed by Coverity.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-06 21:34:49 +09:00
Derrick Stolee
876094ac16 clone: unbundle the advertised bundles
A previous change introduced the transport methods to acquire a bundle
list from the 'bundle-uri' protocol v2 command, when advertised _and_
when the client has chosen to enable the feature.

Teach Git to download and unbundle the data advertised by those bundles
during 'git clone'. This takes place between the ref advertisement and
the object data download, and stateful connections will linger while
the client downloads bundles. In the future, we should consider closing
the remote connection during this process.

Also, since the --bundle-uri option exists, we do not want to mix the
advertised bundles with the user-specified bundles.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-25 16:24:24 +09:00
Derrick Stolee
ebc3947955 bundle-uri: allow relative URLs in bundle lists
Bundle providers may want to distribute that data across multiple CDNs.
This might require a change in the base URI, all the way to the domain
name. If all bundles require an absolute URI in their 'uri' value, then
every push to a CDN would require altering the table of contents to
match the expected domain and exact location within it.

Allow a bundle list to specify a relative URI for the bundles. This URI
is based on where the client received the bundle list. For a list
provided in the 'bundle-uri' protocol v2 command, the Git remote URI is
the base URI. Otherwise, the bundle list was provided from an HTTP URI
not using the Git protocol, and that URI is the base URI. This allows
easier distribution of bundle data.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-25 16:24:24 +09:00
Derrick Stolee
738dc7d4a5 bundle-uri: serve bundle.* keys from config
Implement the "bundle-uri" protocol v2 capability by populating the
key=value packet lines from the local Git config. The list of bundles is
provided from the keys beginning with "bundle.".

In the future, we may want to filter this list to be more specific to
the exact known keys that the server intends to share, but for
flexibility at the moment we will assume that the config values are
well-formed.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-25 16:24:24 +09:00
Ævar Arnfjörð Bjarmason
70b9c10373 bundle-uri client: add helper for testing server
Add a 'test-tool bundle-uri ls-remote' command. This is a thin wrapper
for issuing protocol v2 "bundle-uri" commands to a server, and to the
parsing routines in bundle-uri.c.

In the "git clone" case we'll have already done the handshake(),
but not here. Add an extra case to check for this handshake in
get_bundle_uri() for ease of use for future callers.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-25 16:24:24 +09:00
Ævar Arnfjörð Bjarmason
7cce9074a7 bundle-uri client: add boolean transfer.bundleURI setting
The yet-to-be introduced client support for bundle-uri will always
fall back on a full clone, but we'd still like to be able to ignore a
server's bundle-uri advertisement entirely.

The new transfer.bundleURI config option defaults to 'false', but a user
can set it to 'true' to enable checking for bundle URIs from the origin
Git server using protocol v2.

Co-authored-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-25 16:24:23 +09:00
Ævar Arnfjörð Bjarmason
0cfde740f0 clone: request the 'bundle-uri' command when available
Set up all the needed client parts of the 'bundle-uri' protocol v2
command, without actually doing anything with the bundle URIs.

If the server says it supports 'bundle-uri' teach Git to issue the
'bundle-uri' command after the 'ls-refs' during 'git clone'. The
returned key=value pairs are passed to the bundle list code which is
tested using a different ingest mechanism in t5750-bundle-uri-parse.sh.

At this point, Git does nothing with that bundle list. It will not
download any of the bundles. That will come in a later change after
these protocol bits are finalized.

The no-op client is initially used only by 'git clone' to test the basic
functionality, and eventually will bootstrap the initial download of Git
objects during a fresh clone. The bundle URI client will not be
integrated into other fetches until a mechanism is created to select a
subset of bundles for download.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-25 16:24:23 +09:00
Ævar Arnfjörð Bjarmason
8f788eb8b7 t: create test harness for 'bundle-uri' command
The previous change allowed for a Git server to advertise the
'bundle-uri' command as a capability based on the
uploadPack.advertiseBundleURIs config option. Create a set of tests that
check that this capability is advertised using 'git ls-remote'.

In order to test this functionality across three protocols (file, git,
and http), create lib-bundle-uri-protocol.sh to generalize the tests,
allowing the other test scripts to set an environment variable and
otherwise inherit the setup and tests from this script.

The tests currently only test that the 'bundle-uri' command is
advertised or not. Other actions will be tested as the Git client learns
to request the 'bundle-uri' command and parse its response.

To help with URI escaping, specifically for file paths with a space in
them, extract a 'sed' invocation from t9199-git-svn-info.sh into a
helper function for use here, too.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-25 16:24:23 +09:00
Ævar Arnfjörð Bjarmason
8b8d9a2298 protocol v2: add server-side "bundle-uri" skeleton
Add a skeleton server-side implementation of a new "bundle-uri" command
to protocol v2. This will allow conforming clients to optionally seed
their initial clones or incremental fetches from URLs containing
"*.bundle" files created with "git bundle create".

This change only performs the basic boilerplate of advertising a new
protocol v2 capability. The new 'bundle-uri' capability allows a client
to request a list of bundles. Right now, the server only returns a flush
packet, which corresponds to an empty advertisement. The bundle.* config
namespace describes which key-value pairs will be communicated across
this interface in future updates.

The critical bit right now is that the new boolean
uploadPack.adverstiseBundleURIs config value signals whether or not this
capability should be advertised at all.

An earlier version of this patch [1] used a different transfer format
than the "key=value" pairs in the current implementation. The change was
made to unify the protocol v2 command with the bundle lists provided by
independent bundle servers. Further, the standard allows for the server
to advertise a URI that contains a bundle list. This allows users
automatically discovering bundle providers that are loosely associated
with the origin server, but without the origin server knowing exactly
which bundles are currently available.

[1] https://lore.kernel.org/git/RFC-patch-v2-01.13-2fc87ce092b-20220311T155841Z-avarab@gmail.com/

The very-deep headings needed to be modified to stop at level 4 due to
documentation build issues. These were not recognized in earlier builds
since the file was previously in the Documentation/technical/ directory
and was built in a different way. With its current location, the
heavily-nested details were causing build issues and they are now
replaced with a bulletted list of details.

Co-authored-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-25 16:24:23 +09:00
Taylor Blau
561f3948a5 Merge branch 'do/modernize-t7001'
Modernize test script to avoid "test -f" and friends.

* do/modernize-t7001:
  t7001-mv.sh: modernizing test script using functions
2022-11-14 19:53:31 -05:00
Taylor Blau
be4ac3b197 Merge branch 'rs/no-more-run-command-v'
Simplify the run-command API.

* rs/no-more-run-command-v:
  replace and remove run_command_v_opt()
  replace and remove run_command_v_opt_cd_env_tr2()
  replace and remove run_command_v_opt_tr2()
  replace and remove run_command_v_opt_cd_env()
  use child_process members "args" and "env" directly
  use child_process member "args" instead of string array variable
  sequencer: simplify building argument list in do_exec()
  bisect--helper: factor out do_bisect_run()
  bisect: simplify building "checkout" argument list
  am: simplify building "show" argument list
  run-command: fix return value comment
  merge: remove always-the-same "verbose" arguments
2022-11-08 17:15:12 -05:00
Taylor Blau
3e9303dc8e Merge branch 'rs/archive-filter-error-once'
"git archive" mistakenly complained twice about a missing executable,
which has been corrected.

* rs/archive-filter-error-once:
  archive-tar: report filter start error only once
2022-11-08 17:15:09 -05:00
Taylor Blau
ec9a46af4f Merge branch 'ma/drop-redundant-diagnostic'
A redundant diagnostic message is dropped from test_path_is_missing().

* ma/drop-redundant-diagnostic:
  test-lib-functions: drop redundant diagnostic print
2022-11-08 17:15:06 -05:00
Taylor Blau
15df8418a5 Merge branch 'jk/ref-filter-parsing-bugs'
Various tests exercising the transfer.credentialsInUrl configuration
are taught to avoid making requests which require resolving localhost
to reduce CI-flakiness.

* jk/ref-filter-parsing-bugs:
  ref-filter: fix parsing of signatures with CRLF and no body
  ref-filter: fix parsing of signatures without blank lines
2022-11-08 17:14:52 -05:00
Taylor Blau
bdd42e34e3 Merge branch 'es/mark-gc-cruft-as-experimental'
Enable gc.cruftpacks by default for those who opt into
feature.experimental setting.

* es/mark-gc-cruft-as-experimental:
  config: let feature.experimental imply gc.cruftPacks=true
  gc: add tests for --cruft and friends
2022-11-08 17:14:48 -05:00
Debra Obondo
7cccf5b6c9 t7001-mv.sh: modernizing test script using functions
Test script to verify the presence/absence of files, paths, directories,
symlinks and other features in 'git mv' command are using the command
format:

'test (-e|f|d|h|...)'

Replace them with helper functions of format:

'test_path_is_*'

Replacing idiomatic helper functions:

'! test_path_is_*'

with

'test_path_is_missing'

This uses values of 'test_path_bar' in place of '! test_path_foo' to
bring in the helpful factor of indicating the failure of tests after the
mv command has been used, that is, it echoes if the feature/test_path
exists.

Signed-off-by: Debra Obondo <debraobondo@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-04 17:58:23 -04:00
Jeff King
8e1c5fcf28 ref-filter: fix parsing of signatures with CRLF and no body
This commit fixes a bug when parsing tags that have CRLF line endings, a
signature, and no body, like this (the "^M" are marking the CRs):

  this is the subject^M
  -----BEGIN PGP SIGNATURE-----^M
  ^M
  ...some stuff...^M
  -----END PGP SIGNATURE-----^M

When trying to find the start of the body, we look for a blank line
separating the subject and body. In this case, there isn't one. But we
search for it using strstr(), which will find the blank line in the
signature.

In the non-CRLF code path, we check whether the line we found is past
the start of the signature, and if so, put the body pointer at the start
of the signature (effectively making the body empty). But the CRLF code
path doesn't catch the same case, and we end up with the body pointer in
the middle of the signature field. This has two visible problems:

  - printing %(contents:subject) will show part of the signature, too,
    since the subject length is computed as (body - subject)

  - the length of the body is (sig - body), which makes it negative.
    Asking for %(contents:body) causes us to cast this to a very large
    size_t when we feed it to xmemdupz(), which then complains about
    trying to allocate too much memory.

These are essentially the same bugs fixed in the previous commit, except
that they happen when there is a CRLF blank line in the signature,
rather than no blank line at all. Both are caused by the refactoring in
9f75ce3d8f (ref-filter: handle CRLF at end-of-line more gracefully,
2020-10-29).

We can fix this by doing the same "sigstart" check that we do in the
non-CRLF case. And rather than repeat ourselves, we can just use
short-circuiting OR to collapse both cases into a single conditional.
I.e., rather than:

  if (strstr("\n\n"))
    ...found blank, check if it's in signature...
  else if (strstr("\r\n\r\n"))
    ...found blank, check if it's in signature...
  else
    ...no blank line found...

we can collapse this to:

  if (strstr("\n\n")) ||
      strstr("\r\n\r\n")))
    ...found blank, check if it's in signature...
  else
    ...no blank line found...

The tests show the problem and the fix. Though it wasn't broken, I
included contents:signature here to make sure it still behaves as
expected, but note the shell hackery needed to make it work. A
less-clever option would be to skip using test_atom and just "append_cr
>expected" ourselves.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-02 21:36:04 -04:00
Jeff King
b01e1c7ef0 ref-filter: fix parsing of signatures without blank lines
When ref-filter is asked to show %(content:subject), etc, we end up in
find_subpos() to parse out the three major parts: the subject, the body,
and the signature (if any).

When searching for the blank line between the subject and body, if we
don't find anything, we try to treat the whole message as the subject,
with no body. But our idea of "the whole message" needs to take into
account the signature, too. Since 9f75ce3d8f (ref-filter: handle CRLF at
end-of-line more gracefully, 2020-10-29), the code instead goes all the
way to the end of the buffer, which produces confusing output.

Here's an example. If we have a tag message like this:

  this is the subject
  -----BEGIN SSH SIGNATURE-----
  ...some stuff...
  -----END SSH SIGNATURE-----

then the current parser will put the start of the body at the end of the
whole buffer. This produces two buggy outcomes:

  - since the subject length is computed as (body - subject), showing
    %(contents:subject) will print both the subject and the signature,
    rather than just the single line

  - since the body length is computed as (sig - body), and the body now
    starts _after_ the signature, we end up with a negative length!
    Fortunately we never access out-of-bounds memory, because the
    negative length is fed to xmemdupz(), which casts it to a size_t,
    and xmalloc() bails trying to allocate an absurdly large value.

    In theory it would be possible for somebody making a malicious tag
    to wrap it around to a more reasonable value, but it would require a
    tag on the order of 2^63 bytes. And even if they did, all they get
    is an out of bounds string read. So the security implications are
    probably not interesting.

We can fix both by correctly putting the start of the body at the same
index as the start of the signature (effectively making the body empty).

Note that this is a real issue with signatures generated with gpg.format
set to "ssh", which would look like the example above. In the new tests
here I use a hard-coded tag message, for a few reasons:

  - regardless of what the ssh-signing code produces now or in the
    future, we should be testing this particular case

  - skipping the actual signature makes the tests simpler to write (and
    allows them to run on more systems)

  - t6300 has helpers for working with gpg signatures; for the purposes
    of this bug, "BEGIN PGP" is just as good a demonstration, and this
    simplifies the tests

Curiously, the same issue doesn't happen with real gpg signatures (and
there are even existing tests in t6300 with cover this). Those have a
blank line between the header and the content, like:

  this is the subject
  -----BEGIN PGP SIGNATURE-----

  ...some stuff...
  -----END PGP SIGNATURE-----

Because we search for the subject/body separator line with a strstr(),
we find the blank line in the signature, even though it's outside of
what we'd consider the body. But that puts us unto a separate code path,
which realizes that we're now in the signature and adjusts the line back
to "sigstart". So this patch is basically just making the "no line found
at all" case match that. And note that "sigstart" is always defined (if
there is no signature, it points to the end of the buffer as you'd
expect).

Reported-by: Martin Englund <martin@englund.nu>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-02 21:36:04 -04:00
Johannes Schindelin
db8016b43f t5516/t5601: be less strict about the number of credential warnings
It is unclear as to _why_, but under certain circumstances the warning
about credentials being passed as part of the URL seems to be swallowed
by the `git remote-https` helper in the Windows jobs of Git's CI builds.

Since it is not actually important how many times Git prints the
warning/error message, as long as it prints it at least once, let's just
make the test a bit more lenient and test for the latter instead of the
former, which works around these CI issues.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 16:35:05 -04:00
Jeff King
762521e8a5 t5516: move plaintext-password tests from t5601 and t5516
Commit 6dcbdc0d66 (remote: create fetch.credentialsInUrl config,
2022-06-06) added tests for our handling of passwords in URLs. Since the
obvious URL to be affected is git-over-http, the tests use http. However
they don't set up a test server; they just try to access
https://localhost, assuming it will fail (because the nothing is
listening there).

This causes some possible problems:

  - There might be a web server running on localhost, and we do not
    actually want to connect to that.

  - The DNS resolver, or the local firewall, might take a substantial
    amount of time (or forever, whichever comes first) to fail to
    connect, slowing down the tests cases unnecessarily.

  - Since there's no server, our tests for "allow" and "warn" still
    expect the clone/fetch/push operations to fail, even though in the
    real world we'd expect these to succeed. We scrape stderr to see
    what happened, but it's not as robust as a more realistic test.

Let's instead move these to t5551, which is all about testing http and
where we have a real server. That eliminates any issues with contacting
a strange URL, and lets the "allow" and "warn" tests confirm that the
operation actually succeeds.

It's not quite a verbatim move for a few reasons:

  - we can drop the LIBCURL dependency; it's already part of
    lib-httpd.sh

  - we'll use HTTPD_URL_USER_PASS, etc, instead of our fake URL. To
    avoid repetition, we'll add a few extra variables.

  - the "https://username:@localhost" test uses a funny URL that
    lib-httpd.sh doesn't provide. We'll similarly construct it in a
    variable. Note that we're hard-coding the lib-httpd username here,
    but t5551 already does that everywhere.

  - for the "domain:port" test, the URL provided by lib-httpd is fine,
    since our test server will always be on an exotic port. But we'll
    confirm in the test that this is so.

  - since our message-matching is done via grep, I simplified it to use
    a regex, rather than trying to massage lib-httpd's variables.
    Arguably this makes it more readable, too, while retaining the bits
    we care about: the fatal/warning distinction, the "uses plaintext"
    message, and the fact that the password was redacted.

  - we'll use the /auth/ path for the repo, which shows that we are
    indeed making use of the auth information when needed.

  - we'll also use /smart/; most of these tests could be done via /dumb/
    in t5550, but setting up pushes there requires extra effort and
    dependencies. The smart protocol is what most everyone is using
    these days anyway.

This patch is my own, but I stole the analysis and a few bits of the
commit message from a patch by Johannes Schindelin.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-01 16:35:05 -04:00
Martin Ågren
cc8f95c042 test-lib-functions: drop redundant diagnostic print
`test_path_is_missing` was introduced back in 2caf20c52b ("test-lib:
user-friendly alternatives to test [-d|-f|-e]", 2010-08-10). It took the
path that was supposed to be missing, as well as an optional "diagnosis"
that would be echoed if the path was found to be alive.

Commit 45a2686441 ("test-lib-functions: remove bug-inducing
"diagnostics" helper param", 2021-02-12) dropped this diagnostic
functionality from several `test_path_is_foo` helpers, but note how it
tweaked the README entry on `test_path_is_missing` without actually
adjusting its implementation.

Commit e7884b353b ("test-lib-functions: assert correct parameter count",
2021-02-12) then followed up by asserting that we get just a single
argument.

This history leaves us in a state where we assert that we have exactly
one argument, then go on to anyway check for arguments, echoing them
all. It's clear that we can simplify this code. We should also note that
we run `ls -ld "$1"`, so printing the filename a second time doesn't
really buy us anything. Thus, we can drop the whole `if` block as
redundant.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-10-31 21:12:09 -04:00
Taylor Blau
b1e3dd68ee Merge branch 'en/merge-tree-sequence'
"git merge-tree --stdin" is a new way to request a series of merges
and report the merge results.

* en/merge-tree-sequence:
  merge-tree: support multiple batched merges with --stdin
  merge-tree: update documentation for differences in -z output
2022-10-30 21:04:44 -04:00
Taylor Blau
d32dd8add5 Merge branch 'ds/bundle-uri-3'
Define the logical elements of a "bundle list", data structure to
store them in-core, format to transfer them, and code to parse
them.

* ds/bundle-uri-3:
  bundle-uri: suppress stderr from remote-https
  bundle-uri: quiet failed unbundlings
  bundle: add flags to verify_bundle()
  bundle-uri: fetch a list of bundles
  bundle: properly clear all revision flags
  bundle-uri: limit recursion depth for bundle lists
  bundle-uri: parse bundle list in config format
  bundle-uri: unit test "key=value" parsing
  bundle-uri: create "key=value" line parsing
  bundle-uri: create base key-value pair parsing
  bundle-uri: create bundle_list struct and helpers
  bundle-uri: use plain string in find_temp_filename()
2022-10-30 21:04:44 -04:00
Taylor Blau
bf0d9d0d34 Merge branch 'rj/branch-do-not-exit-with-minus-one-status'
"git branch --edit-description" can exit with status -1 which is
not a good practice; it learned to use 1 as everybody else instead.

* rj/branch-do-not-exit-with-minus-one-status:
  branch: error code with --edit-description
2022-10-30 21:04:43 -04:00
Taylor Blau
0c025612d4 Merge branch 'rj/branch-copy-rename-error-codepath-cleanup'
Code simplification.

* rj/branch-copy-rename-error-codepath-cleanup:
  branch: error copying or renaming a detached HEAD
2022-10-30 21:04:43 -04:00
Taylor Blau
c41ec63ef5 Merge branch 'tb/cap-patch-at-1gb'
"git apply" limits its input to a bit less than 1 GiB.

* tb/cap-patch-at-1gb:
  apply: reject patches larger than ~1 GiB
2022-10-30 21:04:43 -04:00
Taylor Blau
969230b64f Merge branch 'en/ort-dir-rename-and-symlink-fix'
Merging a branch with directory renames into a branch that changes
the directory to a symlink was mishandled by the ort merge
strategy, which has been corrected.

* en/ort-dir-rename-and-symlink-fix:
  merge-ort: fix bug with dir rename vs change dir to symlink
2022-10-30 21:04:43 -04:00
Taylor Blau
a23e0b69e2 Merge branch 'pb/subtree-split-and-merge-after-squashing-tag-fix'
A bugfix to "git subtree" in its split and merge features.

* pb/subtree-split-and-merge-after-squashing-tag-fix:
  subtree: fix split after annotated tag was squashed merged
  subtree: fix squash merging after annotated tag was squashed merged
  subtree: process 'git-subtree-split' trailer in separate function
  subtree: use named variables instead of "$@" in cmd_pull
  subtree: define a variable before its first use in 'find_latest_squash'
  subtree: prefix die messages with 'fatal'
  subtree: add 'die_incompatible_opt' function to reduce duplication
  subtree: use 'git rev-parse --verify [--quiet]' for better error messages
  test-lib-functions: mark 'test_commit' variables as 'local'
2022-10-30 21:04:43 -04:00
Taylor Blau
8851c4b065 Merge branch 'pw/rebase-reflog-fixes'
Fix some bugs in the reflog messages when rebasing and changes the
reflog messages of "rebase --apply" to match "rebase --merge" with
the aim of making the reflog easier to parse.

* pw/rebase-reflog-fixes:
  rebase: cleanup action handling
  rebase --abort: improve reflog message
  rebase --apply: make reflog messages match rebase --merge
  rebase --apply: respect GIT_REFLOG_ACTION
  rebase --merge: fix reflog message after skipping
  rebase --merge: fix reflog when continuing
  t3406: rework rebase reflog tests
  rebase --apply: remove duplicated code
2022-10-30 21:04:43 -04:00
Taylor Blau
003f815dd9 Merge branch 'pw/rebase-keep-base-fixes'
"git rebase --keep-base" used to discard the commits that are
already cherry-picked to the upstream, even when "keep-base" meant
that the base, on top of which the history is being rebuilt, does
not yet include these cherry-picked commits.  The --keep-base
option now implies --reapply-cherry-picks and --no-fork-point
options.

* pw/rebase-keep-base-fixes:
  rebase --keep-base: imply --no-fork-point
  rebase --keep-base: imply --reapply-cherry-picks
  rebase: factor out branch_base calculation
  rebase: rename merge_base to branch_base
  rebase: store orig_head as a commit
  rebase: be stricter when reading state files containing oids
  t3416: set $EDITOR in subshell
  t3416: tighten two tests
2022-10-30 21:04:42 -04:00
Taylor Blau
e5be3c632a Merge branch 'jh/trace2-timers-and-counters'
Two new facilities, "timer" and "counter", are introduced to the
trace2 API.

* jh/trace2-timers-and-counters:
  trace2: add global counter mechanism
  trace2: add stopwatch timers
  trace2: convert ctx.thread_name from strbuf to pointer
  trace2: improve thread-name documentation in the thread-context
  trace2: rename the thread_name argument to trace2_thread_start
  api-trace2.txt: elminate section describing the public trace2 API
  tr2tls: clarify TLS terminology
  trace2: use size_t alloc,nr_open_regions in tr2tls_thread_ctx
2022-10-30 21:04:42 -04:00
Taylor Blau
c112d8d9c2 Merge branch 'tb/shortlog-group'
"git shortlog" learned to group by the "format" string.

* tb/shortlog-group:
  shortlog: implement `--group=committer` in terms of `--group=<format>`
  shortlog: implement `--group=author` in terms of `--group=<format>`
  shortlog: extract `shortlog_finish_setup()`
  shortlog: support arbitrary commit format `--group`s
  shortlog: extract `--group` fragment for translation
  shortlog: make trailer insertion a noop when appropriate
  shortlog: accept `--date`-related options
2022-10-30 21:04:42 -04:00
Taylor Blau
c88895e67b Merge branch 'jk/repack-tempfile-cleanup'
The way "git repack" creared temporary files when it received a
signal was prone to deadlocking, which has been corrected.

* jk/repack-tempfile-cleanup:
  t7700: annotate cruft-pack failure with ok=sigpipe
  repack: drop remove_temporary_files()
  repack: use tempfiles for signal cleanup
  repack: expand error message for missing pack files
  repack: populate extension bits incrementally
  repack: convert "names" util bitfield to array
2022-10-30 21:04:42 -04:00
Taylor Blau
160314e625 Merge branch 'jz/patch-id'
A new "--include-whitespace" option is added to "git patch-id", and
existing bugs in the internal patch-id logic that did not match
what "git patch-id" produces have been corrected.

* jz/patch-id:
  builtin: patch-id: remove unused diff-tree prefix
  builtin: patch-id: add --verbatim as a command mode
  patch-id: fix patch-id for mode changes
  builtin: patch-id: fix patch-id with binary diffs
  patch-id: use stable patch-id for rebases
  patch-id: fix stable patch id for binary / header-only
2022-10-30 21:04:41 -04:00
René Scharfe
1e4ea950f7 archive-tar: report filter start error only once
A missing tar filter is reported by start_command() using error(), but
also by its caller, write_tar_filter_archive(), using die():

   $ git -c tar.invalid.command=foo archive --format=invalid HEAD
   error: cannot run foo: No such file or directory
   fatal: unable to start 'foo' filter: No such file or directory

The second message contains all relevant information and even says that
the failed command was intended to be used as a filter.  Silence the
first one because it's redundant.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-10-30 19:50:43 -04:00
René Scharfe
ddbb47fde9 replace and remove run_command_v_opt()
Replace the remaining calls of run_command_v_opt() with run_command()
calls and explict struct child_process variables.  This is more verbose,
but not by much overall.  The code becomes more flexible, e.g. it's easy
to extend to conditionally add a new argument.

Then remove the now unused function and its own flag names, simplifying
the run-command API.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-10-30 14:04:51 -04:00
René Scharfe
4120294cbf use child_process member "args" instead of string array variable
Use run_command() with a struct child_process variable and populate its
"args" member directly instead of building a string array and passing it
to run_command_v_opt().  This avoids the use of magic index numbers and
makes simplifies the possible addition of more arguments in the future.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-10-30 14:04:39 -04:00
Junio C Hamano
7d5a4d86a6 Merge branch 'tb/diffstat-with-utf8-strwidth'
"git diff --stat" etc. were invented back when everything was ASCII
and strlen() was a way to measure the display width of a string;
adjust them to compute the display width assuming UTF-8 pathnames.

* tb/diffstat-with-utf8-strwidth:
  diff: leave NEEDWORK notes in show_stats() function
  diff.c: use utf8_strwidth() to count display width
2022-10-28 11:26:55 -07:00
Junio C Hamano
330135ac81 Merge branch 'mm/git-pm-try-catch-syntax-fix'
Fix a longstanding syntax error in Git.pm error codepath.

* mm/git-pm-try-catch-syntax-fix:
  Git.pm: trust rev-parse to find bare repositories
  Git.pm: add semicolon after catch statement
2022-10-28 11:26:54 -07:00
Junio C Hamano
c5dd7773e1 Merge branch 'tb/remove-unused-pack-bitmap'
When creating a multi-pack bitmap, remove per-pack bitmap files
unconditionally as they will never be consulted.

* tb/remove-unused-pack-bitmap:
  builtin/repack.c: remove redundant pack-based bitmaps
2022-10-28 11:26:54 -07:00
Junio C Hamano
7b9b634ca5 Merge branch 'ab/doc-synopsis-and-cmd-usage'
The short-help text shown by "git cmd -h" and the synopsis text
shown at the beginning of "git help cmd" have been made more
consistent.

* ab/doc-synopsis-and-cmd-usage: (34 commits)
  tests: assert consistent whitespace in -h output
  tests: start asserting that *.txt SYNOPSIS matches -h output
  doc txt & -h consistency: make "worktree" consistent
  worktree: define subcommand -h in terms of command -h
  reflog doc: list real subcommands up-front
  doc txt & -h consistency: make "commit" consistent
  doc txt & -h consistency: make "diff-tree" consistent
  doc txt & -h consistency: use "[<label>...]" for "zero or more"
  doc txt & -h consistency: make "annotate" consistent
  doc txt & -h consistency: make "stash" consistent
  doc txt & -h consistency: add missing options
  doc txt & -h consistency: use "git foo" form, not "git-foo"
  doc txt & -h consistency: make "bundle" consistent
  doc txt & -h consistency: make "read-tree" consistent
  doc txt & -h consistency: make "rerere" consistent
  doc txt & -h consistency: add missing options and labels
  doc txt & -h consistency: make output order consistent
  doc txt & -h consistency: add or fix optional "--" syntax
  doc txt & -h consistency: fix mismatching labels
  doc SYNOPSIS & -h: use "-" to separate words in labels, not "_"
  ...
2022-10-28 11:26:54 -07:00
Junio C Hamano
246eedf2bc Merge branch 'js/cmake-updates'
Update to build procedure with VS using CMake/CTest.

* js/cmake-updates:
  cmake: increase time-out for a long-running test
  cmake: avoid editing t/test-lib.sh
  add -p: avoid ambiguous signed/unsigned comparison
  cmake: copy the merge tools for testing
  cmake: make it easier to diagnose regressions in CTest runs
2022-10-27 14:51:53 -07:00
Junio C Hamano
702bb4baea Merge branch 'nw/t1002-cleanup'
Code clean-up in test.

* nw/t1002-cleanup:
  t1002: modernize outdated conditional
2022-10-27 14:51:53 -07:00