Our parse_blob_buffer() takes a ptr/len combo, just like
parse_tree_buffer(), etc, and returns success or failure. But it doesn't
actually do anything with them; we just set the "parsed" flag in the
object and return success, without even looking at the contents.
There could be some value to keeping these unused parameters:
- it's consistent with the parse functions for other object types. But
we already lost that consistency in 837d395a5c (Replace parse_blob()
with an explanatory comment, 2010-01-18).
- As the comment from 837d395a5c explains, callers are supposed to
make sure they have the object content available. So in theory
asking for these parameters could serve as a signal. But there are
only two callers, and one of them always passes NULL (after doing a
streaming check of the object hash).
This shows that there aren't likely to be a lot of callers (since
everyone either uses the type-generic parse functions, or handles
blobs individually), and that they need to take special care anyway
(because we usually want to avoid loading whole blobs in memory if
we can avoid it).
So let's just drop these unused parameters, and likewise the useless
return value. While we're touching the header file, let's move the
declaration of parse_blob_buffer() right below that explanatory comment,
where it's more likely to be seen by people looking for the function.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ls_refs() function (for the v2 protocol command of the same name)
takes a repository parameter (like all v2 commands), but ignores it. It
should use it to access the refs.
This isn't a bug in practice, since we only call this function when
serving upload-pack from the main repository. But it's an awkward
gotcha, and it causes -Wunused-parameter to complain.
The main reason we don't use the repository parameter is that the ref
iteration interface we call doesn't have a "refs_" variant that takes a
ref_store. However we can easily add one. In fact, since there is only
one other caller (in ref-filter.c), there is no need to maintain the
non-repository wrapper; that caller can just use the_repository. It's
still a long way from consistently using a repository object, but it's
one small step in the right direction.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The server_supports_v2() helper lets a caller find out if the server
supports a feature, and will optionally die if it's not supported. This
makes the return value confusing, as it's only meaningful when the
function is not asked to die.
Coverity flagged a new call like:
/* check that we support "foo" */
server_supports_v2("foo", 1);
complaining that we usually checked the return value, but this time we
didn't. But this call is correct, and other ones that did:
if (server_supports_v2("foo", 1))
do_something_with_foo();
are "wrong", in the sense that we know the conditional will always be
true (but there's no bug; the code is simply misleading).
Let's split the "die" behavior into its own function which returns void,
and modify each caller to use the correct one.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
apply_parse_options() passes the array of argument strings to
parse_options(), which removes recognized options. The removed strings
are not freed, though.
Make a copy of the strvec to pass to the function to retain the pointers
of its strings, so we release them all at the end.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Don't put clean parents on the pending list, as they and their ancestors
don't need any treatment and would be skipped later anyway. This saves
the allocation and release of a commit list item in ca. 20% of the cases
during a run of the test suite.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
reflog_expiry_prepare() calls mark_reachable(), which recurively flags
commits as REACHABLE. The traversal stops beyond a certain age
threshold; the boundary commits also marked as REACHABLE and put back
into mark_list at the end. unreachable() finishes the traversal down to
the roots if necessary -- but if all interesting commits are younger
than the age threshold then only recent commits need to be visited.
When this optimization works then the boundary commits still sit there
in mark_list at the end. Clear their REACHABLE flag and release the
commit list allocations.
While at it remove a duplicate code line from mark_reachable(); the same
flag is already set five lines up.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ee4512ed48 ("trace2: create new combined trace facility", 2019-02-
22) introduced trace2_cmd_name() and taught both the Git built-ins and
some non-built-ins to use it. However, http-fetch was not one of them
(perhaps due to its low usage at the time).
Teach http-fetch to invoke this function. After this patch, this
function will be invoked right after argument parsing, just like in
remote-curl.c.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently, auto correction doesn't work reliably for commands which must
run in a work tree (e.g. `git status`) in Git work trees which are
created from a bare repository.
As far as I'm able to determine, this has been broken since commit
659fef199f (help: use early config when autocorrecting aliases,
2017-06-14), where the call to `git_config()` in `help_unknown_cmd()`
was replaced with a call to `read_early_config()`. From what I can tell,
the actual cause for the unexpected error is that we call
`git_default_config()` in the `git_unknown_cmd_config` callback instead
of simply returning `0` for config entries which we aren't interested
in.
Calling `git_default_config()` in this callback to `read_early_config()`
seems like a bad idea since those calls will initialize a bunch of state
in `environment.c` (among other things `is_bare_repository_cfg`) before
we've properly detected that we're running in a work tree.
All other callbacks provided to `read_early_config()` appear to only
extract their configurations while simply returning `0` for all other
config keys.
This commit changes the `git_unknown_cmd_config` callback to not call
`git_default_config()`. Instead we also simply return `0` for config
keys which we're not interested in.
Additionally the commit adds a new test case covering `help.autocorrect`
in a work tree created from a bare clone.
Signed-off-by: Simon Gerber <gesimu@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When Git's test suite uses `test_cmp`, it is not actually trying to
compare binary files as the name `cmp` would suggest to users familiar
with Unix' tools, but the tests instead verify that actual output
matches the expected text.
On Unix, `cmp` works well enough for Git's purposes because only Line
Feed characters are used as line endings. However, on Windows, while
most tools accept Line Feeds as line endings, many tools produce
Carriage Return + Line Feed line endings, including some of the tools
used by the test suite (which are therefore provided via Git for Windows
SDK). Therefore, `cmp` would frequently fail merely due to different
line endings.
To accommodate for that, the `mingw_test_cmp` function was introduced
into Git's test suite to perform a line-by-line comparison that ignores
line endings. This function is a Bash function that is only used on
Windows, everywhere else `cmp` is used.
This is a double whammy because `cmp` is fast, and `mingw_test_cmp` is
slow, even more so on Windows because it is a Bash script function, and
Bash scripts are known to run particularly slowly on Windows due to
Bash's need for the POSIX emulation layer provided by the MSYS2 runtime.
The commit message of 32ed3314c1 (t5351: avoid using `test_cmp` for
binary data, 2022-07-29) provides an illuminating account of the
consequences: On Windows, the platform on which Git could really use all
the help it can get to improve its performance, the time spent on one
entire test script was reduced from half an hour to less than half a
minute merely by avoiding a single call to `mingw_test_cmp` in but a
single test case.
Learning the lesson to avoid shell scripting wherever possible, the Git
for Windows project implemented a minimal replacement for
`mingw_test_cmp` in the form of a `test-tool` subcommand that parses the
input files line by line, ignoring line endings, and compares them.
Essentially the same thing as `mingw_test_cmp`, but implemented in
C instead of Bash. This solution served the Git for Windows project
well, over years.
However, when this solution was finally upstreamed, the conclusion was
reached that a change to use `git diff --no-index` instead of
`mingw_test_cmp` was more easily reviewed and hence should be used
instead.
The reason why this approach was not even considered in Git for Windows
is that in 2007, there was already a motion on the table to use Git's
own diff machinery to perform comparisons in Git's test suite, but it
was dismissed in https://lore.kernel.org/git/xmqqbkrpo9or.fsf@gitster.g/
as undesirable because tests might potentially succeed due to bugs in
the diff machinery when they should not succeed, and those bugs could
therefore hide regressions that the tests try to prevent.
By the time Git for Windows' `mingw-test-cmp` in C was finally
contributed to the Git mailing list, reviewers agreed that the diff
machinery had matured enough and should be used instead.
When the concern was raised that the diff machinery, due to its
complexity, would perform substantially worse than the test helper
originally implemented in the Git for Windows project, a test
demonstrated that these performance differences are well lost within the
100+ minutes it takes to run Git's test suite on Windows.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The old version we currently use runs in node.js v12.x, which is being
deprecated in GitHub Actions. The new version uses node.js v16.x.
Incidentally, this also avoids the warning about the deprecated
`::set-output::` workflow command because the newer version of the
`github-script` Action uses the recommended new way to specify outputs.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Adjust the GitHub CI to newer ubuntu release.
* jx/ci-ubuntu-fix:
ci: install python on ubuntu
ci: use the same version of p4 on both Linux and macOS
ci: remove the pipe after "p4 -V" to catch errors
github-actions: run gcc-8 on ubuntu-20.04 image
Update GitHub CI to use actions/checkout@v3; use of the older
checkout@v2 gets annoying deprecation notices.
* od/ci-use-checkout-v3-when-applicable:
ci(main): upgrade actions/checkout to v3
Update GitHub CI to use actions/checkout@v3; use of the older
checkout@v2 gets annoying deprecation notices.
* od/ci-use-checkout-v3-when-applicable:
ci(main): upgrade actions/checkout to v3
I haven't been very active in the community lately, but I'm soon going
to lose access to my previous commit email (@usp.br); so add my current
personal address to mailmap for any future message exchanges or patch
contributions.
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In b3b1a21d1a (sequencer: rewrite update-refs as user edits todo list,
2022-07-19), the 'todo_list_filter_update_refs()' step was added to handle
the removal of 'update-ref' lines from a 'rebase-todo'. Specifically, it
removes potential ref updates from the "update refs state" if a ref does not
have a corresponding 'update-ref' line.
However, because 'write_update_refs_state()' will not update the state if
the 'refs_to_oids' list was empty, removing *all* 'update-ref' lines will
result in the state remaining unchanged from how it was initialized (with
all refs' "after" OID being null). Then, when the ref update is applied, all
refs will be updated to null and consequently deleted.
To fix this, delete the 'update-refs' state file when 'refs_to_oids' is
empty. Additionally, add a tests covering "all update-ref lines removed"
cases.
Reported-by: herr.kaste <herr.kaste@gmail.com>
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Helped-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Victoria Dye <vdye@github.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
t1509-root-work-tree.sh, which tests behavior of a Git repository
located at the root `/` directory, refuses to run if it detects the
presence of an existing repository at `/`. This safeguard ensures that
it won't clobber a legitimate repository at that location. However,
because t1509 does a poor job of cleaning up after itself, it runs afoul
of its own safety check on subsequent runs, which makes it painful to
run the script repeatedly since each run requires manual cleanup of
detritus from the previous run.
Address this shortcoming by making t1509 clean up after itself as its
last action. This is safe since the script can only make it to this
cleanup action if it did not find a legitimate repository at `/` in the
first place, so the resources cleaned up here can only have been created
by the script itself.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
One of the t1509 setup tests is very particular about the output it
expects from `git init`, and fails if the output differs even slightly
which can happen easily if the script is run multiple times since it
doesn't do a good job of cleaning up after itself (i.e. it leaves
detritus in the root directory `/`). One bit of cruft in particular
(`/HEAD`) makes the test fail since its presence causes `git init` to
alter its output; rather than reporting "Initialized empty Git
repository", it instead reports "Reinitialized existing Git repository"
when `/HEAD` is present. Address this problem by making the test do a
more careful job of crafting its intended initial state.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
When 8959555cee (setup_git_directory(): add an owner check for the
top-level directory, 2022-03-02) tightened security surrounding
directory ownership, it neglected to adjust t1509-root-work-tree.sh to
take the new restriction into account. As a result, since the root
directory `/` is typically not owned by the user running the test
(indeed, t1509 refuses to run as `root`), the ownership check added
by 8959555cee kicks in and causes the test to fail:
fatal: detected dubious ownership in repository at '/'
To add an exception for this directory, call:
git config --global --add safe.directory /
This problem went unnoticed for so long because t1509 is rarely run
since it requires setting up a `chroot` environment or a sacrificial
virtual machine in which `/` can be made writable and polluted by any
user.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
The deprecated versions of these Actions still use node.js 12 whereas
workflows will need to use node.js 16 to avoid problems going forward.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the "js/ci-github-workflow-markup" topic was originally merged in
[1] it included a change to get rid of the "ci/print-test-failures.sh"
step[2]. This was then brought back in [3] as part of a fix-up patches
on top[4].
The problem was that [3] was not a revert of the relevant parts of
[2], but rather copy/pasted the "ci/print-test-failures.sh" step that
was present for the Windows job to all "ci/print-test-failures.sh"
steps. The Windows steps specified "shell: bash", but the non-Windows
ones did not.
This broke the "ci/print/test-failures.sh" step for the "linux-musl"
job, where we don't have a "bash" shell, just a "/bin/sh" (a
"dash"). This breakage was reported at the time[5], but hadn't been
fixed.
It would be sufficient to change this only for "linux-musl", but let's
change this for both "regular" and "dockerized" to omit the "shell"
line entirely, as we did before [2].
Let's also change undo the "name" change that [3] made while
copy/pasting the "print test failures" step for the Windows job. These
steps are now the same as they were before [2], except that the "if"
includes the "env.FAILED_TEST_ARTIFACTS" test.
1. fc5a070f59 (Merge branch 'js/ci-github-workflow-markup', 2022-06-07)
2. 08dccc8fc1 (ci: make it easier to find failed tests' logs in the
GitHub workflow, 2022-05-21)
3. 5aeb145780 (ci(github): bring back the 'print test failures' step,
2022-06-08)
4. d0d96b8280 (Merge branch 'js/ci-github-workflow-markup', 2022-06-17)
5. https://lore.kernel.org/git/220725.86sfmpneqp.gmgdl@evledraar.gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Per [1] and the warnings our CI is emitting GitHub is phasing in
"macos-12" as their "macos-latest".
As with [2], let's pin our image to a specific version so that we're
not having it swept from under us, and our upgrade cycle can be more
predictable than whenever GitHub changes their images.
1. https://github.com/actions/runner-images/issues/6384
2. 0178420b9c (github-actions: run gcc-8 on ubuntu-20.04 image,
2022-11-25)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
grep(1) converts CRLF line endings to LF on current MinGW:
$ uname -sr
MINGW64_NT-10.0-22621 3.3.6-341.x86_64
$ printf 'a\r\n' | hexdump.exe -C
00000000 61 0d 0a |a..|
00000003
$ printf 'a\r\n' | grep . | hexdump.exe -C
00000000 61 0a |a.|
00000002
Create the intended test file by grepping the original file with LF
line endings and adding CRs explicitly.
The missing CRs went unnoticed because test_cmp on MinGW ignores line
endings since 4d715ac05c (Windows: a test_cmp that is agnostic to random
LF <> CRLF conversions, 2013-10-26). Fix this test anyway to avoid
depending on that special test_cmp behavior, especially since this is
the only test that needs it.
Piping the output of grep(1) through append_cr has the side-effect of
ignoring its return value. That means we no longer need the explicit
"|| true" to support commit messages without a body.
Signed-off-by: René Scharfe <l.s.r@web.de>
Acked-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In Git for Windows, when passing paths from shell scripts to regular
Win32 executables, thanks to the MSYS2 runtime a somewhat magic path
conversion happens that lets the shell script think that there is a file
at `/git/Makefile` and the Win32 process it spawned thinks that the
shell script said `C:/git-sdk-64/git/Makefile` instead.
This conversion is documented in detail over here:
https://www.msys2.org/docs/filesystem-paths/#automatic-unix-windows-path-conversion
As all automatic conversions, there are gaps. For example, to avoid
mistaking command-line options like `/LOG=log.txt` (which are quite
common in the Windows world) from being mistaken for a Unix-style
absolute path, the MSYS2 runtime specifically exempts arguments
containing a `=` character from that conversion.
We are about to change `test_cmp` to use `git diff --no-index`, which
involves spawning precisely such a Win32 process.
In combination, this would cause a failure in `t0021-conversion.sh`
where we pass an absolute path containing an equal character to the
`test_cmp` function.
Seeing as the Unix tools like `cp` and `diff` that are used by Git's
test suite in the Git for Windows SDK (thanks to the MSYS2 project)
understand both Unix-style as well as Windows-style paths, we can stave
off this problem by simply switching to Windows-style paths and
side-stepping the need for any automatic path conversion.
Note: The `PATH` variable is obviously special, as it is colon-separated
in the MSYS2 Bash used by Git for Windows, and therefore _cannot_
contain absolute Windows-style paths, lest the colon after the drive
letter is mistaken for a path separator. Therefore, we need to be
careful to keep the Unix-style when modifying the `PATH` variable.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
