The dumb HTTP transport tries to read the remote HEAD reference by
downloading the "HEAD" file and then parsing it via `http_fetch_ref()`.
This function will either parse the file as an object ID in case it is
exactly `the_hash_algo->hexsz` long, or otherwise it will check whether
the reference starts with "ref :" and parse it as a symbolic ref.
This is broken when parsing detached HEADs of a remote SHA256 repository
because we never update `the_hash_algo` to the discovered remote object
hash. Consequently, `the_hash_algo` will always be the fallback SHA1
hash algorithm, which will cause us to fail parsing HEAD altogteher when
it contains a SHA256 object ID.
Fix this issue by setting up `the_hash_algo` via `repo_set_hash_algo()`.
While at it, let's make the expected SHA1 fallback explicit in our code,
which also addresses an upcoming issue where we are going to remove the
SHA1 fallback for `the_hash_algo`.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
attr: fix BUG() when parsing attrs outside of repo
If either the `--attr-source` option or the `GIT_ATTR_SOURCE` envvar are
set, then `compute_default_attr_source()` will try to look up the value
as a treeish. It is possible to hit that function while outside of a Git
repository though, for example when using `git grep --no-index`. In that
case, Git will hit a bug because we try to look up the main ref store
outside of a repository.
Handle the case gracefully and detect when we try to look up an attr
source without a repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `default_attr_source()` function lazily computes the attr source
supposedly once, only. This is done via a static variable `attr_source`
that contains the resolved object ID of the attr source's tree. If the
variable is the null object ID then we try to look up the attr source,
otherwise we skip over it.
This approach is flawed though: the variable will never be set to
anything else but the null object ID in case there is no attr source.
Consequently, we re-compute the information on every call. And in the
worst case, when we silently ignore bad trees, this will cause us to try
and look up the treeish every single time.
Improve this by introducing a separate variable `has_attr_source` to
track whether we already computed the attr source and, if so, whether we
have an attr source or not.
This also allows us to convert the `ignore_bad_attr_tree` to not be
static anymore as the code will only be executed once anyway.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
parse-options-cb: only abbreviate hashes when hash algo is known
The `OPT__ABBREV()` option can be used to add an option that abbreviates
object IDs. When given a length longer than `the_hash_algo->hexsz`, then
it will instead set the length to that maximum length.
It may not always be guaranteed that we have `the_hash_algo` initialized
properly as the hash algorithm can only be set up after we have set up
`the_repository`. In that case, the hash would always be truncated to
the hex length of SHA1, which may not be what the user desires.
In practice it's not a problem as all commands that use `OPT__ABBREV()`
also have `RUN_SETUP` set and thus cannot work without a repository.
Consequently, both `the_repository` and `the_hash_algo` would be
properly set up.
Regardless of that, harden the code to not truncate the length when we
didn't set up a repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
While `validate_headref()` is only called from `is_git_directory()` in
"setup.c", it is currently implemented in "path.c". Move it over such
that it becomes clear that it is only really used during setup in order
to discover repositories.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
path: harden validation of HEAD with non-standard hashes
The `validate_headref()` function takes a path to a supposed "HEAD" file
and checks whether its format is something that we understand. It is
used as part of our repository discovery to check whether a specific
directory is a Git directory or not.
Part of the validation is a check for a detached HEAD that contains a
plain object ID. To do this validation we use `get_oid_hex()`, which
relies on `the_hash_algo`. At this point in time the hash algo cannot
yet be initialized though because we didn't yet read the Git config.
Consequently, it will always be the SHA1 hash algorithm.
In practice this works alright because `get_oid_hex()` only ends up
checking whether the prefix of the buffer is a valid object ID. And
because SHA1 is shorter than SHA256, the function will successfully
parse SHA256 object IDs, as well.
It is somewhat fragile though and not really the intent to only check
for SHA1. With this in mind, harden the code to use `get_oid_hex_any()`
to check whether the "HEAD" file parses as any known hash.
One might be hard pressed to tighten the check even further and fully
validate the file contents, not only the prefix. In practice though that
wouldn't make a lot of sense as it could be that the repository uses a
hash function that produces longer hashes than SHA256, but which the
current version of Git doesn't understand yet. We'd still want to detect
the repository as proper Git repository in that case, and we will fail
eventually with a proper error message that the hash isn't understood
when trying to set up the repository format.
It follows that we could just leave the current code intact, as in
practice the code change doesn't have any user visible impact. But it
also prepares us for `the_hash_algo` being unset when there is no
repository.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 7 May 2024 05:50:29 +0000 (22:50 -0700)]
Merge branch 'ps/the-index-is-no-more' into ps/undecided-is-not-necessarily-sha1
* ps/the-index-is-no-more:
repository: drop `initialize_the_repository()`
repository: drop `the_index` variable
builtin/clone: stop using `the_index`
repository: initialize index in `repo_init()`
builtin: stop using `the_index`
t/helper: stop using `the_index`
The `test-tool` recently learned to run the unit tests. To this end, it
needs to link with `test-lib.c`, which was done in the `Makefile`, and
this patch does it in the CMake definition, too.
This is a companion of 44400f58407e (t0080: turn t-basic unit test into
a helper, 2024-02-02).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Josh Steadmon [Mon, 6 May 2024 19:57:37 +0000 (12:57 -0700)]
ci: use test-tool as unit test runner on Windows
Although the previous commit changed t/Makefile to run unit tests
alongside shell tests, the Windows CI still needs a separate unit-tests
step due to how the test sharding works.
We want to avoid using `prove` as a test running on Windows due to
performance issues [1], so use the new test-tool runner instead.
Jeff King [Mon, 6 May 2024 19:57:36 +0000 (12:57 -0700)]
t/Makefile: run unit tests alongside shell tests
Add a wrapper script to allow `prove` to run both shell tests and unit
tests from a single invocation. This avoids issues around running prove
twice in CI, as discussed in [1].
Additionally, this moves the unit tests into the main dev workflow, so
that errors can be spotted more quickly. Accordingly, we remove the
separate unit tests step for Linux CI. (We leave the Windows CI
unit-test step as-is, because the sharding scheme there involves
selecting specific test files rather than running `make test`.)
Josh Steadmon [Mon, 6 May 2024 19:57:35 +0000 (12:57 -0700)]
unit tests: add rule for running with test-tool
In the previous commit, we added support in test-tool for running
collections of unit tests. Now, add rules in t/Makefile for running in
this way.
This new rule can be executed from the top-level Makefile via
`make DEFAULT_UNIT_TEST_TARGET=unit-tests-test-tool unit-tests`, or by
setting DEFAULT_UNIT_TEST_TARGET in config.mak.
Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Josh Steadmon [Mon, 6 May 2024 19:57:34 +0000 (12:57 -0700)]
test-tool run-command testsuite: support unit tests
Teach the testsuite runner in `test-tool run-command testsuite` how to
run unit tests: if TEST_SHELL_PATH is not set, run the programs directly
from CWD, rather than defaulting to "sh" as an interpreter.
With this change, you can now use test-tool to run the unit tests:
$ make
$ cd t/unit-tests/bin
$ ../../helper/test-tool run-command testsuite
This should be helpful on Windows to allow running tests without
requiring Perl (for `prove`), as discussed in [1] and [2].
This again breaks backwards compatibility, as it is now required to set
TEST_SHELL_PATH properly for executing shell scripts, but again, as
noted in [2], there are no longer any such invocations in our codebase.
`test-tool run-command testsuite` currently assumes that it will only be
running the shell test suite, and therefore filters out anything that
does not match a hardcoded pattern of "t[0-9][0-9][0-9][0-9]-*.sh".
Later in this series, we'll adapt `test-tool run-command testsuite` to
also support unit tests, which do not follow the same naming conventions
as the shell tests, so this hardcoded pattern is inconvenient.
Since `testsuite` also allows specifying patterns on the command-line,
let's just remove this pattern. As noted in [1], there are no longer any
uses of `testsuite` in our codebase, it should be OK to break backwards
compatibility in this case. We also add a new filter to avoid trying to
execute "." and "..", so that users who wish to execute every test in a
directory can do so without specifying a pattern.
Josh Steadmon [Mon, 6 May 2024 19:57:32 +0000 (12:57 -0700)]
test-tool run-command testsuite: get shell from env
When running tests through `test-tool run-command testsuite`, we
currently hardcode `sh` as the command interpreter. As discussed in [1],
this is incorrect, and we should be using the shell set in
TEST_SHELL_PATH instead.
Add a shell_path field in struct testsuite so that we can pass this to
the task runner callback. If this is non-null, we'll use it as the
argv[0] of the subprocess. Otherwise, we'll just execute the test
program directly. We will use this feature in a later commit to enable
running binary executable unit tests.
However, for now when setting up the struct testsuite in testsuite(),
use the value of TEST_SHELL_PATH if it's set, otherwise keep the
original behavior by defaulting to `sh`.
Josh Steadmon [Mon, 6 May 2024 19:57:31 +0000 (12:57 -0700)]
t0080: turn t-basic unit test into a helper
While t/unit-tests/t-basic.c uses the unit-test framework added in e137fe3b29 (unit tests: add TAP unit test framework, 2023-11-09), it is
not a true unit test in that it intentionally fails in order to exercise
various codepaths in the unit-test framework. Thus, we intentionally
exclude it when running unit tests through the various t/Makefile
targets. Instead, it is executed by t0080-unit-test-output.sh, which
verifies its output follows the TAP format expected for the various
pass, skip, or fail cases.
As such, it makes more sense for t-basic to be a helper item for
t0080-unit-test-output.sh, so let's move it to
t/helper/test-example-tap.c and adjust Makefiles as necessary.
Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Newer versions of Ubuntu have dropped Python 2 starting with Ubuntu
23.04. By default though, our CI setups will try to use that Python
version on all Ubuntu-based jobs except for the "linux-gcc" one.
We didn't notice this issue due to two reasons:
- The "ubuntu:latest" tag always points to the latest LTS release.
Until a few weeks ago this was Ubuntu 22.04, which still had Python
2.
- Our Docker-based CI jobs had their own script to install
dependencies until 9cdeb34b96 (ci: merge scripts which install
dependencies, 2024-04-12), where we didn't even try to install
Python at all for many of them.
Since the CI refactorings have originally been implemented, Ubuntu
24.04 was released, and it being an LTS versions means that the "latest"
tag now points to that Python-2-less version. Consequently, those jobs
that use "ubuntu:latest" broke.
Address this by using Python 2 on Ubuntu 20.04, only, whereas we use
Python 3 on all other Ubuntu jobs. Eventually, we should think about
dropping support for Python 2 completely.
Reported-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Øystein Walle [Mon, 6 May 2024 18:23:17 +0000 (20:23 +0200)]
Documentation: Mention that refspecs are explained elsewhere
The syntax for refspecs are explained in more detail in documention for
git-fetch and git-push. Give a hint to the user too look there more fore
information
Signed-off-by: Øystein Walle <oystwa@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 6 May 2024 16:40:31 +0000 (09:40 -0700)]
format-patch: run range-diff with larger creation-factor
We see too often that a range-diff added to format-patch output
shows too many "unmatched" patches. This is because the default
value for creation-factor is set to a relatively low value.
It may be justified for other uses (like you have a yet-to-be-sent
new iteration of your series, and compare it against the 'seen'
branch that has an older iteration, probably with the '--left-only'
option, to pick out only your patches while ignoring the others) of
"range-diff" command, but when the command is run as part of the
format-patch, the user _knows_ and expects that the patches in the
old and the new iterations roughly correspond to each other, so we
can and should use a much higher default.
Until now, `git config -h` would have printed help for the old-style
syntax. Now that all modes have proper subcommands though it is
preferable to instead display the subcommand help.
Drop the `NO_INTERNAL_HELP` flag to do so. While at it, drop the help
mismatch in t0450 and add the `--get-colorbool` option to the usage such
that git-config(1)'s synopsis and `git config -h` match.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
While git-config(1) has several modes, those modes are not exposed with
subcommands but instead by specifying action flags like `--unset` or
`--list`. This user interface is not really in line with how our more
modern commands work, where it is a lot more customary to say e.g. `git
remote list`. Furthermore, to add to the confusion, git-config(1) also
allows the user to request modes implicitly by just specifying the
correct number of arguments. Thus, `git config foo.bar` will retrieve
the value of "foo.bar" while `git config foo.bar baz` will set it to
"baz".
Overall, this makes for a confusing interface that could really use a
makeover. It hurts discoverability of what you can do with git-config(1)
and is comparatively easy to get wrong. Converting the command to have
subcommands instead would go a long way to help address these issues.
One concern in this context is backwards compatibility. Luckily, we can
introduce subcommands without breaking backwards compatibility at all.
This is because all the implicit modes of git-config(1) require that the
first argument is a properly formatted config key. And as config keys
_must_ have a dot in their name, any value without a dot would have been
discarded by git-config(1) previous to this change. Thus, given that
none of the subcommands do have a dot, they are unambiguous.
Introduce the first such new subcommand, which is "git config list". To
retain backwards compatibility we only conditionally use subcommands and
will fall back to the old syntax in case no subcommand was detected.
This should help to transition to the new-style syntax until we
eventually deprecate and remove the old-style syntax.
Note that the way we handle this we're duplicating some functionality
across old and new syntax. While this isn't pretty, it helps us to
ensure that there really is no change in behaviour for the old syntax.
Amend tests such that we run them both with old and new style syntax.
As tests are now run twice, state from the first run may be still be
around in the second run and thus cause tests to fail. Add cleanup logic
as required to fix such tests.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/config: pull out function to handle config location
There's quite a bunch of options to git-config(1) that allow the user to
specify which config location to use when reading or writing config
options. The logic to handle this is thus by necessity also quite
involved.
Pull it out into a separate function so that we can reuse it in
subsequent commits which introduce proper subcommands.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/config: use `OPT_CMDMODE()` to specify modes
The git-config(1) command has various different modes which are
accessible via e.g. `--get-urlmatch` or `--unset-all`. These modes are
declared with `OPT_BIT()`, which causes two minor issues:
- The respective modes also have a negated form `--no-get-urlmatch`,
which is unintended.
- We have to manually handle exclusiveness of the modes.
Switch these options to instead use `OPT_CMDMODE()`, which is made
exactly for this usecase. Remove the now-unneeded check that only a
single mode is given, which is now handled by the parse-options
interface.
While at it, format optional placeholders for arguments to conform to
our style guidelines by using `[<placeholder>]`.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/config: move "fixed-value" option to correct group
The `--fixed-value` option can be used to alter how the value-pattern
parameter is interpreted for the various actions of git-config(1). But
while it is an option, it is currently listed as part of the actions
group, which is wrong.
Move the option to the "Other" group, which hosts the various options
known to git-config(1).
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
config: clarify memory ownership when preparing comment strings
The ownership of memory returned when preparing a comment string is
quite intricate: when the returned value is different than the passed
value, then the caller is responsible to free the memory. This is quite
subtle, and it's even easier to miss because the returned value is in
fact a `const char *`.
Adapt the function to always return either `NULL` or a newly allocated
string. The function is called at most once per git-config(1), so it's
not like this micro-optimization really matters. Thus, callers are now
always responsible for freeing the value.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sun, 5 May 2024 10:20:03 +0000 (12:20 +0200)]
diff: fix --exit-code with external diff
You can ask the diff machinery to let the exit code indicate whether
there are changes, e.g. with --exit-code. It as two ways to calculate
that bit: The quick one assumes blobs with different hashes have
different content, and the more elaborate way actually compares the
contents, possibly applying transformations like ignoring whitespace.
Always use the slower path by setting the flag diff_from_contents,
because any of the files could have an external diff driver set via an
attribute, which might consider binary differences irrelevant, like e.g.
René Scharfe [Sun, 5 May 2024 10:19:56 +0000 (12:19 +0200)]
diff: report unmerged paths as changes in run_diff_cmd()
You can ask the diff machinery to let the exit code indicate whether
there are changes, e.g. with --quiet. It as two ways to calculate that
bit: The quick one assumes blobs with different hashes have different
content, and the more elaborate way actually compares the contents,
possibly applying transformations like ignoring whitespace.
The quick way considers an unmerged file to be a change and reports
exit code 1, which makes sense.
The slower path uses the struct diff_options member found_changes to
indicate whether the blobs differ even with the transformations applied.
It's not set for unmerged files, though, resulting in exit code 0.
Set found_changes in run_diff_cmd() for unmerged files, for a consistent
exit code of 1 if there's an unmerged file, regardless of whether
whitespace is ignored.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ivan Tse [Sat, 4 May 2024 03:04:08 +0000 (03:04 +0000)]
refs: return conflict error when checking packed refs
The TRANSACTION_NAME_CONFLICT error code refers to a failure to create a
ref due to a name conflict with another ref. An example of this is a
directory/file conflict such as ref names A/B and A.
"git fetch" uses this error code to more accurately describe the error
by recommending to the user that they try running "git remote prune" to
remove any old refs that are deleted by the remote which would clear up
any directory/file conflicts.
This helpful error message is not displayed when the conflicted ref is
stored in packed refs. This change fixes this by ensuring error return
code consistency in `lock_raw_ref`.
Signed-off-by: Ivan Tse <ivan.tse1@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 8 Apr 2024 23:36:05 +0000 (16:36 -0700)]
Makefile(s): do not enforce "all indents must be done with tab"
Our top-level Makefile follows our generic whitespace rule
established by the top-level .gitattributes file that does not
enforce indent-with-non-tab rule by default, but git-gui is set up
to enforce indent-with-non-tab by default. With the upcoming change
to GNU make, we no longer can reject (and worse, "fix") a patch that
adds whitespace indented lines to the Makefile, so loosen the rule
there for git-gui/Makefile, too.
Taylor Blau [Mon, 8 Apr 2024 15:51:44 +0000 (11:51 -0400)]
Makefile(s): avoid recipe prefix in conditional statements
In GNU Make commit 07fcee35 ([SV 64815] Recipe lines cannot contain
conditional statements, 2023-05-22) and following, conditional
statements may no longer be preceded by a tab character (which Make
refers to as the recipe prefix).
There are a handful of spots in our various Makefile(s) which will break
in a future release of Make containing 07fcee35. For instance, trying to
compile the pre-image of this patch with the tip of make.git results in
the following:
$ make -v | head -1 && make
GNU Make 4.4.90
config.mak.uname:842: *** missing 'endif'. Stop.
The kernel addressed this issue in 82175d1f9430 (kbuild: Replace tabs
with spaces when followed by conditionals, 2024-01-28). Address the
issues in Git's tree by applying the same strategy.
When a conditional word (ifeq, ifneq, ifdef, etc.) is preceded by one or
more tab characters, replace each tab character with 8 space characters
with the following:
Reported-by: Dario Gjorgjevski <dario.gjorgjevski@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
The French word "aperçu", meaning "view" or "preview", contains only a
single letter "p". Remove the extra letter, which is an obvious typo.
Reported-by: Léonard Michelet <leonard@lebasic.com> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Taylor Blau [Fri, 3 May 2024 19:12:36 +0000 (15:12 -0400)]
attr.c: move ATTR_MAX_FILE_SIZE check into read_attr_from_buf()
Commit 3c50032ff52 (attr: ignore overly large gitattributes files,
2022-12-01) added a defense-in-depth check to ensure that .gitattributes
blobs read from the index do not exceed ATTR_MAX_FILE_SIZE (100 MB).
But there were two cases added shortly after 3c50032ff52 was written
which do not apply similar protections:
- 47cfc9bd7d0 (attr: add flag `--source` to work with tree-ish,
2023-01-14)
- 4723ae1007f (attr.c: read attributes in a sparse directory,
2023-08-11) added a similar
Ensure that we refuse to process a .gitattributes blob exceeding
ATTR_MAX_FILE_SIZE when reading from either an arbitrary tree object or
a sparse directory. This is done by pushing the ATTR_MAX_FILE_SIZE check
down into the low-level `read_attr_from_buf()`.
In doing so, plug a leak in `read_attr_from_index()` where we would
accidentally leak the large buffer upon detecting it is too large to
process.
(Since `read_attr_from_buf()` handles a NULL buffer input, we can remove
a NULL check before calling it in `read_attr_from_index()` as well).
Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Fri, 3 May 2024 17:21:07 +0000 (12:21 -0500)]
gitlab-ci: add whitespace error check
GitLab CI does not have a job to check for whitespace errors introduced
by a set of changes. Reuse the existing generic `whitespace-check.sh` to
create the job for GitLab pipelines.
Note that the `$CI_MERGE_REQUEST_TARGET_BRANCH_SHA` variable is only
available in GitLab merge request pipelines and therefore the CI job is
configured to only run as part of those pipelines.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Fri, 3 May 2024 17:21:06 +0000 (12:21 -0500)]
ci: make the whitespace report optional
The `check-whitespace` CI job generates a formatted output file
containing whitespace error information. As not all CI providers support
rendering a formatted summary, make its generation optional.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Fri, 3 May 2024 17:21:05 +0000 (12:21 -0500)]
ci: separate whitespace check script
The `check-whitespace` CI job is only available as a GitHub action. To
help enable this job with other CI providers, first separate the logic
performing the whitespace check into its own script. In subsequent
commits, this script is further generalized allowing its reuse.
Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Fri, 3 May 2024 17:21:04 +0000 (12:21 -0500)]
github-ci: fix link to whitespace error
When the `check-whitespace` CI job detects whitespace errors, a
formatted summary of the issue is generated. This summary contains links
to the commits and blobs responsible for the whitespace errors. The
generated links for blobs do not work and result in a 404.
Instead of using the reference name in the link, use the commit ID
directly. This fixes the broken link and also helps enable future
generalization of the script for other CI providers by removing one of
the GitHub specific CI variables used.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Fri, 3 May 2024 17:21:03 +0000 (12:21 -0500)]
ci: pre-collapse GitLab CI sections
Sections of CI output defined by `begin_group()` and `end_group()` are
expanded in GitLab pipelines by default. This can make CI job output
rather noisy and harder to navigate. Update the behavior for GitLab
pipelines to now collapse sections by default.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
James Liu [Fri, 3 May 2024 07:17:06 +0000 (17:17 +1000)]
advice: add --no-advice global option
Advice hints must be disabled individually by setting the relevant
advice.* variables to false in the Git configuration. For server-side
and scripted usages of Git where hints can be a hindrance, it can be
cumbersome to maintain configuration to ensure all advice hints are
disabled in perpetuity. This is a particular concern in tests, where
new or changed hints can result in failed assertions.
Add a --no-advice global option to disable all advice hints from being
displayed. This is independent of the toggles for individual advice
hints. Use an internal environment variable (GIT_ADVICE) to ensure this
configuration is propagated to the usage site, even if it executes in a
subprocess.
Signed-off-by: James Liu <james@jamesliu.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>
James Liu [Fri, 3 May 2024 07:17:05 +0000 (17:17 +1000)]
doc: add spacing around paginate options
Make the documentation page consistent with the usage string printed by
"git help git" and consistent with the description of "[-v | --version]"
option.
Signed-off-by: James Liu <james@jamesliu.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 3 May 2024 15:34:27 +0000 (08:34 -0700)]
stop using HEAD for attributes in bare repository by default
With 23865355 (attr: read attributes from HEAD when bare repo,
2023-10-13), we started to use the HEAD tree as the default
attribute source in a bare repository. One argument for such a
behaviour is that it would make things like "git archive" run in
bare and non-bare repositories for the same commit consistent.
This changes was merged to Git 2.43 but without an explicit mention
in its release notes.
It turns out that this change destroys performance of shallowly
cloning from a bare repository. As the "server" installations are
expected to be mostly bare, and "git pack-objects", which is the
core of driving the other side of "git clone" and "git fetch" wants
to see if a path is set not to delta with blobs from other paths via
the attribute system, the change forces the server side to traverse
the tree of the HEAD commit needlessly to find if each and every
paths the objects it sends out has the attribute that controls the
deltification. Given that (1) most projects do not configure such
an attribute, and (2) it is dubious for the server side to honor
such an end-user supplied attribute anyway, this was a poor choice
of the default.
To mitigate the current situation, let's revert the change that uses
the tree of HEAD in a bare repository by default as the attribute
source. This will help most people who have been happy with the
behaviour of Git 2.42 and before.
Two things to note:
* If you are stuck with versions of Git 2.43 or newer, that is
older than the release this fix appears in, you can explicitly
set the attr.tree configuration variable to point at an empty
tree object, i.e.
* If you like the behaviour we are reverting, you can explicitly
set the attr.tree configuration variable to HEAD, i.e.
$ git config attr.tree HEAD
The right fix for this is to optimize the code paths that allow
accesses to attributes in tree objects, but that is a much more
involved change and is left as a longer-term project, outside the
scope of this "first step" fix.
Mike Hommey [Fri, 3 May 2024 09:14:27 +0000 (18:14 +0900)]
win32: fix building with NO_UNIX_SOCKETS
After 2406bf5f (Win32: detect unix socket support at runtime,
2024-04-03), it fails with:
compat/mingw.c:4160:5: error: no previous prototype for function 'mingw_have_unix_sockets' [-Werror,-Wmissing-prototypes]
4160 | int mingw_have_unix_sockets(void)
| ^
because the prototype is behind `ifndef NO_UNIX_SOCKETS`.
Signed-off-by: Mike Hommey <mh@glandium.org> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 2 May 2024 20:15:57 +0000 (13:15 -0700)]
t/lib-chunk: work around broken "mv" on some vintage of macOS
When the destination is read-only, "mv" on some version of macOS
asks whether to replace the destination even though in the test its
stdin is not a terminal (and thus doesn't conform to POSIX[1]).
The helper to corrupt a chunk-file is designed to work on the
files like commit-graph and multi-pack-index files that are
generally read-only, so use "mv -f" to work around this issue.
Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 2 May 2024 04:54:26 +0000 (04:54 +0000)]
trailer: document parse_trailers() usage
Explain how to use parse_trailers(), because earlier we made the
trailer_info struct opaque. That is, because clients can no longer peek
inside it, we should give them guidance about how the (pointer to the)
opaque struct can still be useful to them.
Rename "head" struct to "trailer_objects" to make the wording of the new
comments a bit easier to read (because "head" itself doesn't really have
any domain-specific meaning here).
Signed-off-by: Linus Arver <linus@ucla.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 2 May 2024 04:54:24 +0000 (04:54 +0000)]
trailer: make trailer_info struct private
In 13211ae23f (trailer: separate public from internal portion of
trailer_iterator, 2023-09-09) we moved trailer_info behind an anonymous
struct to discourage use by trailer.h API users. However it still left
open the possibility of external use of trailer_info itself. Now that
there are no external users of trailer_info, we can make this struct
private.
Make this struct private by putting its definition inside trailer.c.
This has two benefits:
(1) it makes the surface area of the public facing
interface (trailer.h) smaller, and
(2) external API users are unable to peer inside this struct (because
it is only ever exposed as an opaque pointer).
There are a few disadvantages:
(A) every time the member of the struct is accessed an extra pointer
dereference must be done, and
(B) for users of trailer_info outside trailer.c, this struct can no
longer be allocated on the stack and may only be allocated on the
heap (because its definition is hidden away in trailer.c) and
appropriately deallocated by the user, and
(C) without good documentation on the API, the opaque struct is
hostile to programmers by going opposite to the "Show me your
data structures, and I won't usually need your code; it'll
be obvious." mantra [2].
(The disadvantages have already been observed in the two preparatory
commits that precede this one.) This commit believes that the benefits
outweigh the disadvantages for designing APIs, as explained below.
Making trailer_info private exposes existing deficiencies in the API.
This is because users of this struct had full access to its internals,
so there wasn't much need to actually design it to be "complete" in the
sense that API users only needed to use what was provided by the API.
For example, the location of the trailer block (start/end offsets
relative to the start of the input text) was accessible by looking at
these struct members directly. Now that the struct is private, we have
to expose new API functions to allow clients to access this
information (see builtin/interpret-trailers.c).
The idea in this commit to hide implementation details behind an "opaque
pointer" is also known as the "pimpl" (pointer to implementation) idiom
in C++ and is a common pattern in that language (where, for example,
abstract classes only have pointers to concrete classes).
However, the original inspiration to use this idiom does not come from
C++, but instead the book "C Interfaces and Implementations: Techniques
for Creating Reusable Software" [1]. This book recommends opaque
pointers as a good design principle for designing C libraries, using the
term "interface" as the functions defined in *.h (header) files and
"implementation" as the corresponding *.c file which define the
interfaces.
The book says this about opaque pointers:
... clients can manipulate such pointers freely, but they can’t
dereference them; that is, they can’t look at the innards of the
structure pointed to by them. Only the implementation has that
privilege. Opaque pointers hide representation details and help
catch errors.
In our case, "struct trailer_info" is now hidden from clients, and the
ways in which this opaque pointer can be used is limited to the richness
of <trailer.h>. In other words, <trailer.h> exclusively controls exactly
how "trailer_info" pointers are to be used.
[1] Hanson, David R. "C Interfaces and Implementations: Techniques for
Creating Reusable Software". Addison Wesley, 1997. p. 22
[2] Raymond, Eric S. "The Cathedral and the Bazaar: Musings on Linux and
Open Source by an Accidental Revolutionary". O'Reilly, 1999.
Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Linus Arver <linus@ucla.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 2 May 2024 04:54:23 +0000 (04:54 +0000)]
trailer: make parse_trailers() return trailer_info pointer
This is the second and final preparatory commit for making the
trailer_info struct private to the trailer implementation.
Make trailer_info_get() do the actual work of allocating a new
trailer_info struct, and return a pointer to it. Because
parse_trailers() wraps around trailer_info_get(), it too can return this
pointer to the caller. From the trailer API user's perspective, the call
to trailer_info_new() can be replaced with parse_trailers(); do so in
interpret-trailers.
Because trailer_info_new() is no longer called by interpret-trailers,
remove this function from the trailer API.
With this change, we no longer allocate trailer_info on the stack ---
all uses of it are via a pointer where the actual data is always
allocated at runtime through trailer_info_new(). Make
trailer_info_release() free this dynamically allocated memory.
Finally, due to the way the function signatures of parse_trailers() and
trailer_info_get() have changed, update the callsites in
format_trailers_from_commit() and trailer_iterator_init() accordingly.
Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Linus Arver <linus@ucla.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 2 May 2024 04:54:22 +0000 (04:54 +0000)]
interpret-trailers: access trailer_info with new helpers
Instead of directly accessing trailer_info members, access them
indirectly through new helper functions exposed by the trailer API.
This is the first of two preparatory commits which will allow us to
use the so-called "pimpl" (pointer to implementation) idiom for the
trailer API, by making the trailer_info struct private to the trailer
implementation (and thus hidden from the API).
Helped-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Linus Arver <linus@ucla.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 2 May 2024 04:54:21 +0000 (04:54 +0000)]
sequencer: use the trailer iterator
Instead of calling "trailer_info_get()", which is a low-level function
in the trailers implementation (trailer.c), call
trailer_iterator_advance(), which was specifically designed for public
consumption in f0939a0eb1 (trailer: add interface for iterating over
commit trailers, 2020-09-27).
Avoiding "trailer_info_get()" means we don't have to worry about options
like "no_divider" (relevant for parsing trailers). We also don't have to
check for things like "info.trailer_start == info.trailer_end" to see
whether there were any trailers (instead we can just check to see
whether the iterator advanced at all).
Note how we have to use "iter.raw" in order to get the same behavior as
before when we iterated over the unparsed string array (char **trailers)
in trailer_info.
Signed-off-by: Linus Arver <linus@ucla.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 2 May 2024 04:54:20 +0000 (04:54 +0000)]
trailer: teach iterator about non-trailer lines
Previously the iterator did not iterate over non-trailer lines. This was
somewhat unfortunate, because trailer blocks could have non-trailer
lines in them since 146245063e (trailer: allow non-trailers in trailer
block, 2016-10-21), which was before the iterator was created in f0939a0eb1 (trailer: add interface for iterating over commit trailers,
2020-09-27).
So if trailer API users wanted to iterate over all lines in a trailer
block (including non-trailer lines), they could not use the iterator and
were forced to use the lower-level trailer_info struct directly (which
provides a raw string array that includes all lines in the trailer
block).
Change the iterator's behavior so that we also iterate over non-trailer
lines, instead of skipping over them. The new "raw" member of the
iterator allows API users to access previously inaccessible non-trailer
lines. Reword the variable "trailer" to just "line" because this
variable can now hold both trailer lines _and_ non-trailer lines.
The new "raw" member is important because anyone currently not using the
iterator is using trailer_info's raw string array directly to access
lines to check what the combined key + value looks like. If we didn't
provide a "raw" member here, iterator users would have to re-construct
the unparsed line by concatenating the key and value back together again
Linus Arver [Thu, 2 May 2024 04:54:19 +0000 (04:54 +0000)]
trailer: add unit tests for trailer iterator
Test the number of trailers found by the iterator (to be more precise,
the parsing mechanism which the iterator just walks over) when given
some arbitrary log message.
We test the iterator because it is a public interface function exposed
by the trailer API (we generally don't want to test internal
implementation details which are, unlike the API, subject to drastic
changes).
Signed-off-by: Linus Arver <linus@ucla.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Beat Bolli [Thu, 2 May 2024 11:03:31 +0000 (13:03 +0200)]
color: add support for 12-bit RGB colors
RGB color parsing currently supports 24-bit values in the form #RRGGBB.
As in Cascading Style Sheets (CSS [1]), also allow to specify an RGB color
using only three digits with #RGB.
In this shortened form, each of the digits is – again, as in CSS –
duplicated to convert the color to 24 bits, e.g. #f1b specifies the same
color as #ff11bb.
In color.h, remove the '0x' prefix in the example to match the actual
syntax.
Junio C Hamano [Wed, 1 May 2024 17:22:59 +0000 (10:22 -0700)]
rev-parse: document how --is-* options work outside a repository
When "git rev-parse" is run with the "--is-inside-work-tree" option
and friends outside a Git repository, the command exits with a
non-zero status and says "fatal: not a repository". While it is not
wrong per-se, in the sense that it is useless to learn if we are
inside or outside a working tree in the first place when we are not
even in a repository, it could be argued that they should emit
"false" and exit with status 0, as they cannot possibly be "true".
As the current behaviour has been with us for a decade or more
since it was introduced in Git 1.5.3 timeframe, it is too late to
change it.
And arguably, the current behaviour is easier to use if you want to
distinguish among three states, i.e.,
(1) the cwd is not controlled by Git at all
(2) the cwd is inside a working tree
(3) the cwd is not inside a working tree (e.g., .git/hooks/)
with a single invocation of the command by doing
if inout=$(git rev-parse --is-inside-work-tree)
then
case "$inout" in
true) : in a working tree ;;
false) : not in a working tree ;;
esac
else
: not in a repository
fi
So, let's document clearly that the command will die() when run
outside a repository in general, unless in some special cases like
when the command is in the --parseopt mode.
While at it, update the introductory text that makes it sound as if
the primary operating mode is the only operating mode of the
command, which was written long before we added "--parseopt" and
"--sq-quote" modes.
René Scharfe [Wed, 1 May 2024 10:26:16 +0000 (12:26 +0200)]
diff-lib: stop calling diff_setup_done() in do_diff_cache()
d44e5267ea (diff-lib: plug minor memory leaks in do_diff_cache(),
2020-11-14) added the call to diff_setup_done() to release the memory
of the parseopt member of struct diff_options that repo_init_revisions()
had allocated via repo_diff_setup() and prep_parse_options().
189e97bc4b (diff: remove parseopts member from struct diff_options,
2022-12-01) did away with that allocation; diff_setup_done() doesn't
release any memory anymore. So stop calling this function on the blank
diffopt member before it is overwritten, as this is no longer necessary.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
D. Ben Knoble [Tue, 30 Apr 2024 21:53:15 +0000 (21:53 +0000)]
completion: zsh: stop leaking local cache variable
Completing commands like "git rebase" in one repository will leak the
local __git_repo_path into the shell's environment so that completing
commands after changing to a different repository will give the old
repository's references (or none at all).
The bug report on the mailing list [1] suggests one simple way to observe
this yourself:
Enter the following commands from some directory:
mkdir a b b/c
for d (a b); git -C $d init && git -C $d commit --allow-empty -m init
cd a
git branch foo
pushd ../b/c
git branch bar
Now type these:
git rebase <TAB>… # completion for bar available; C-c to abort
declare -p __git_repo_path # outputs /path/to/b/.git
popd
git branch # outputs foo, main
git rebase <TAB>… # completion candidates are bar, main!
Ideally, the last typed <TAB> should be yielding foo, main.
Commit beb6ee7163 (completion: extract repository discovery from
__gitdir(), 2017-02-03) anticipated this problem by marking
__git_repo_path as local in __git_main and __gitk_main for Bash
completion but did not give the same mark to _git for Zsh completion.
Thus make __git_repo_path local for Zsh completion, too.
Junio C Hamano [Tue, 30 Apr 2024 21:49:45 +0000 (14:49 -0700)]
Merge branch 'js/for-each-repo-keep-going'
A scheduled "git maintenance" job is expected to work on all
repositories it knows about, but it stopped at the first one that
errored out. Now it keeps going.
* js/for-each-repo-keep-going:
maintenance: running maintenance should not stop on errors
for-each-repo: optionally keep going on an error
Junio C Hamano [Tue, 30 Apr 2024 21:49:44 +0000 (14:49 -0700)]
Merge branch 'js/build-fuzz-more-often'
In addition to building the objects needed, try to link the objects
that are used in fuzzer tests, to make sure at least they build
without bitrot, in Linux CI runs.
* js/build-fuzz-more-often:
fuzz: link fuzz programs with `make all` on Linux
Junio C Hamano [Tue, 30 Apr 2024 21:49:42 +0000 (14:49 -0700)]
Merge branch 'jc/format-patch-rfc-more'
The "--rfc" option of "git format-patch" learned to take an
optional string value to be used in place of "RFC" to tweak the
"[PATCH]" on the subject header.
* jc/format-patch-rfc-more:
format-patch: "--rfc=-(WIP)" appends to produce [PATCH (WIP)]
format-patch: allow --rfc to optionally take a value, like --rfc=WIP
Junio C Hamano [Tue, 30 Apr 2024 21:49:42 +0000 (14:49 -0700)]
Merge branch 'ds/format-patch-rfc-and-k'
The "-k" and "--rfc" options of "format-patch" will now error out
when used together, as one tells us not to add anything to the
title of the commit, and the other one tells us to add "RFC" in
addition to "PATCH".
* ds/format-patch-rfc-and-k:
format-patch: ensure that --rfc and -k are mutually exclusive
Junio C Hamano [Tue, 30 Apr 2024 21:49:41 +0000 (14:49 -0700)]
Merge branch 'pw/rebase-m-signoff-fix'
"git rebase --signoff" used to forget that it needs to add a
sign-off to the resulting commit when told to continue after a
conflict stops its operation.
* pw/rebase-m-signoff-fix:
rebase -m: fix --signoff with conflicts
sequencer: store commit message in private context
sequencer: move current fixups to private context
sequencer: start removing private fields from public API
sequencer: always free "struct replay_opts"
When the user gives an unknown command to the "add -p" prompt, the list
of accepted commands with their explanation is given. This is the same
output they get when they say '?'.
However, the unknown command may be due to a user input error rather
than the user not knowing the valid command.
To reduce the likelihood of user confusion and error repetition, instead
of displaying the list of accepted commands, display a short error
message with the unknown command received, as feedback to the user.
Include a reminder about the current command '?' in the new message, to
guide the user if they want help.
Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no need to show some UI messages on stderr, and yet doing so
may produce some undesirable results, such as messages appearing in an
unexpected order.
Let's use stdout for all UI messages, and adjusts the tests accordingly.
Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* maint-2.44: (41 commits)
Git 2.44.1
Git 2.43.4
Git 2.42.2
Git 2.41.1
Git 2.40.2
Git 2.39.4
fsck: warn about symlink pointing inside a gitdir
core.hooksPath: add some protection while cloning
init.templateDir: consider this config setting protected
clone: prevent hooks from running during a clone
Add a helper function to compare file contents
init: refactor the template directory discovery into its own function
find_hook(): refactor the `STRIP_EXTENSION` logic
clone: when symbolic links collide with directories, keep the latter
entry: report more colliding paths
t5510: verify that D/F confusion cannot lead to an RCE
submodule: require the submodule path to contain directories only
clone_submodule: avoid using `access()` on directories
submodules: submodule paths must not contain symlinks
clone: prevent clashing git dirs when cloning submodule in parallel
...
projects / thirdparty / git.git / log
