We provide oid_array_for_each_unique() for iterating over the
de-duplicated items in an array. But it's awkward to use for two
reasons:
1. It uses a callback, which means marshaling arguments into a struct
and passing it to the callback with a void parameter.
2. The callback doesn't know the numeric index of the oid we're
looking at. This is useful for things like progress meters.
Iterating with a for-loop is much more natural for some cases, but the
caller has to do the de-duping itself. However, we can provide a small
helper to make this easier (see the docstring in the header for an
example use).
The caller does have to remember to sort the array first. We could add
an assertion into the helper that array->sorted is set, but I didn't
want to complicate what is otherwise a pretty fast code path.
I also considered adding a full iterator type with init/next/end
functions (similar to what we have for hashmaps). But it ended up making
the callers much harder to read. This version keeps us close to a basic
for-loop.
Yet another option would be adding an option to sort the array and
compact out the duplicates. This would mean iterating over the array an
extra time, though that's probably not a big deal (we did just do an
O(n log n) sort). But we'd still have to write a for-loop to iterate, so
it doesn't really make anything easier for the caller.
No new test, since we'll convert the callback iterator (which is covered
by t0064, among other callers) to use the new code.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We sort the oid-array as a side effect of calling the lookup or
unique-iteration functions. But callers may want to sort it themselves
(especially as we add new iteration options in future patches).
We'll also move the check of the "sorted" flag into the sort function,
so callers don't have to remember to check it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We define git_hash_algo and object_id in hash.h, but most of the utility
functions are declared in the main cache.h. Let's move them to hash.h
along with their struct definitions. This cleans up cache.h a bit, but
also avoids circular dependencies when other headers need to know about
these functions (e.g., if oid-array.h were to have an inline that used
oideq(), it couldn't include cache.h because it is itself included by
cache.h).
No including C files should be affected, because hash.h is always
included in cache.h already.
We do have to mention repository.h at the top of hash.h, though, since
we depend on the_repository in some of our inline functions.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our tests for handling duplicates in oid-array provide only a single
duplicate for each number, so our sorted array looks like:
44 44 55 55 88 88 aa aa
A slightly more interesting test is to have multiple duplicates, which
makes sure that we not only skip the duplicate, but keep skipping until
we are out of the set of matching duplicates.
Unsurprisingly this works just fine, but it's worth beefing up this test
since we're about to change the duplicate-detection code.
Note that we do need to adjust the results on the lookup test, since it
is returning the index of the found item (and now we have more items
before our range, and the range itself is slightly larger, since we'll
accept a match of any element).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The data type is an oid_array these days, and we are using "test-tool
oid-array", so let's name the test script appropriately.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When this file was moved from sha1-array.h, we forgot to update the
preprocessor header guard to match the new name.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An earlier attempt to fix "git fetch --recurse-submodules" broke
another use case; revert it until a better fix is found.
* pk/subsub-fetch-fix:
Revert "submodules: fix of regression on fetching of non-init subsub-repo"
"git fetch" that is killed may leave a pack-objects process behind,
still computing to find a good compression, wasting cycles. This
has been corrected.
* jk/stop-pack-objects-when-fetch-is-killed:
upload-pack: kill pack-objects helper on signal or exit
"git push" that is killed may leave a pack-objects process behind,
still computing to find a good compression, wasting cycles. This
has been corrected.
* jk/stop-pack-objects-when-push-is-killed:
send-pack: kill pack-objects helper on signal or exit
Simplify the logic to deal with a repack operation that ended up
creating the same packfile.
* tb/repack-simplify:
builtin/repack.c: don't move existing packs out of the way
builtin/repack.c: keep track of what pack-objects wrote
repack: make "exts" array available outside cmd_repack()
"git pull --rebase --recurse-submodules" checked for local changes
in a wrong range and failed to run correctly when it should.
* pb/pull-rebase-recurse-submodules:
pull: check for local submodule modifications with the right range
t5572: describe '--rebase' tests a little more
t5572: add notes on a peculiar test
pull --rebase: compute rebase arguments in separate function
"git-parse-remote" shell script library outlived its usefulness.
* ab/retire-parse-remote:
submodule: fix fetch_in_submodule logic
parse-remote: remove this now-unused library
submodule: remove sh function in favor of helper
submodule: use "fetch" logic instead of custom remote discovery
This reverts commit 1b7ac4e6d4d490b224f5206af7418ed74e490608; in
<CAN0XMOLiS_8JZKF_wW70BvRRxkDHyUoa=Z3ODtB_Bd6f5Y=7JQ@mail.gmail.com>,
Ralf Thielow reports that "git fetch" with submodule.recurse set can
result in a bogus and infinitely recursive fetching of the same
submodule.
We spawn an external pack-objects process to actually send objects to
the remote side. If we are killed by a signal during this process, then
pack-objects may continue to run. As soon as it starts producing output
for the pack, it will see a failure writing to upload-pack and exit
itself. But before then, it may do significant work traversing the
object graph, compressing deltas, etc, which will all be pointless. So
let's make sure to kill as soon as we know that the caller will not read
the result.
There's no test here, since it's inherently racy, but here's an easy
reproduction is on a large-ish repo like linux.git:
- make sure you don't have pack bitmaps (since they make the enumerating
phase go quickly). For linux.git it takes ~30s or so to walk the
whole graph on my machine.
- run "git clone --no-local -q . dst"; the "-q" is important because
if pack-objects is writing progress to upload-pack (to get
multiplexed over the sideband to the client), then it will notice
pretty quickly the failure to write to stderr
- kill the client-side clone process in another terminal (don't use
^C, as that will send SIGINT to all of the processes)
- run "ps au | grep git" or similar to observe upload-pack dying
within 5 seconds (it will send a keepalive that will notice the
client has gone away)
- but you'll still see pack-objects consuming 100% CPU (and 1GB+ of
RAM) during the traversal and delta compression phases. It will exit
as soon as it starts to write the pack (when it will notice that
upload-pack went away).
With this patch, pack-objects exits as soon as upload-pack does.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Multiple "credential-store" backends can race to lock the same
file, causing everybody else but one to fail---reattempt locking
with some timeout to reduce the rate of the failure.
* sa/credential-store-timeout:
crendential-store: use timeout when locking file
A test script got cleaned up and then made not to depend on the
value of init.defaultBranch.
* js/t3404-master-to-primary:
t3404: do not depend on any specific default branch name
Config parser fix for "git notes".
* na/notes-displayref-is-not-boolean:
t3301: test proper exit response to no-value notes.displayRef.
notes.c: fix a segfault in notes_display_config()
Expectation for the original contributor after responding to a
review comment to use the explanation in a patch update has been
described.
* jc/do-not-just-explain-but-update-your-patch:
MyFirstContribition: answering questions is not the end of the story
Fix formulation of an error message with two placeholders in "git
worktree add" subcommand.
* mt/worktree-error-message-fix:
worktree: fix order of arguments in error message
Fix an option name in "gc" documentation.
* ab/gc-keep-base-option:
gc: rename keep_base_pack variable for --keep-largest-pack
gc docs: change --keep-base-pack to --keep-largest-pack
A test script got cleaned up not to depend on the value of
init.defaultBranch.
* js/t4015-wo-master:
t4015: let the test pass with any default branch name
A test script got cleaned up and then made not to depend on the
value of init.defaultBranch.
* js/t2106-cleanup:
t2106: ensure that the checkout fails for the expected reason
t2106: make test independent of the current main branch name
t2106: adjust style to the current conventions
A lazily defined test prerequisite can now be defined in terms of
another lazily defined test prerequisite.
* sg/tests-prereq:
tests: fix description of 'test_set_prereq'
tests: make sure nested lazy prereqs work reliably
Since jgit does not yet work with SHA-256 repositories, mark the
tests that uses it not to run unless we are testing with ShA-1
repositories.
* sg/t5310-jgit-wants-sha1:
t5310-pack-bitmaps: skip JGit tests with SHA256
"git fetch" did not work correctly with nested submodules where the
innermost submodule that is not of interest got updated in the
upstream, which has been corrected.
* pk/subsub-fetch-fix:
submodules: fix of regression on fetching of non-init subsub-repo
The code was not prepared to deal with pack .idx file that is
larger than 4GB.
* jk/4gb-idx:
packfile: detect overflow in .idx file size checks
block-sha1: take a size_t length parameter
fsck: correctly compute checksums on idx files larger than 4GB
use size_t to store pack .idx byte offsets
compute pack .idx byte offsets using size_t
The exchange between receive-pack and proc-receive hook did not
carefully check for errors.
* jx/t5411-flake-fix:
receive-pack: use default version 0 for proc-receive
receive-pack: gently write messages to proc-receive
t5411: new helper filter_out_user_friendly_and_stable_output
"git bisect start/next" in a large span of history spends a lot of
time trying to come up with exactly the half-way point; this can be
optimized by stopping when we see a commit that is close enough to
the half-way point.
* sg/bisect-approximately-halfway:
bisect: loosen halfway() check for a large number of commits
The command line completion script (in contrib/) learned to expand
commands that are alias of alias.
* fc/bash-completion-alias-of-alias:
completion: bash: improve alias loop detection
completion: bash: check for alias loop
completion: bash: support recursive aliases
When holding the lock for rewriting the credential file, use a timeout
to avoid race conditions when the credentials file needs to be updated
in parallel.
An example would be doing `fetch --all` on a repository with several
remotes that need credentials, using parallel fetching.
The timeout can be configured using "credentialStore.lockTimeoutMS",
defaulting to 1 second.
Signed-off-by: Simão Afonso <simao.afonso@powertools-tech.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The sleep function is defined in wrapper.c, so it makes more sense to be a in
system compatibility header.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A review exchange may begin with a reviewer asking "what did you
mean by this phrase in your log message (or here in the doc)?", the
author answering what was meant, and then the reviewer saying "ah,
that is what you meant---then the flow of the logic makes sense".
But that is not the happy end of the story. New contributors often
forget that the material that has been reviewed in the above exchange
is still unclear in the same way to the next person who reads it,
until it gets updated.
While we are in the vicinity, rephrase the verb "request" used to
refer to comments by reviewers to "suggest"---this matches the
contrast between "original" and "suggested" that appears later in
the same paragraph, and more importantly makes it clearer that it is
not like authors are to please reviewers' wishes but rather
reviewers are merely helping authors to polish their commits.
Reviewed-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that we can override the default branch name in the tests via
`GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME`, we should avoid expecting a
particular hard-coded name.
So let's rename the initial branch immediately to `primary` and work
with that.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 1c1518071c (submodule: use "fetch" logic instead of custom remote
discovery, 2020-11-14) rewrote the logic in fetch_in_submodule to do:
elif test "$2" -ne ""
But this is nonsense in shell: -ne is for numeric comparisons. This
should be "=" or more idiomatically:
elif test -n "$2"
But once we fix that, many tests start failing. Because that commit
introduced another problem. The caller that passes 3 arguments looks
like this:
fetch_in_submodule "$sm_path" $depth "$sha1"
Note the unquoted $depth parameter. When it isn't set, the function will
see only 2 arguments, and the function has no idea if what it sees in $2
is an option to go on the command line, or a refspec to pass on stdin.
In the old code before that commit:
fetch_in_submodule () (
sanitize_submodule_env &&
cd "$1" &&
- case "$2" in
- '')
- git fetch ;;
- *)
- shift
- git fetch $(get_default_remote) "$@" ;;
- esac
we treated those the same, so it didn't matter. But in the new logic
(with my fix above):
+ if test $# -eq 3
+ then
+ echo "$3" | git fetch --stdin "$2"
+ elif test -n "$n"
+ then
+ git fetch "$2"
+ else
+ git fetch
+ fi
we use the number of parameters to distinguish the two. Let's insist
that the caller pass an empty string for positional parameter two if
they want to have a third parameter after it.
But that still leaves one problem. In the --stdin block, we
unconditionally pass "$2" to git-fetch, even if it's the empty string.
Rather than add another conditional, we can use :+ parameter expansion
to include it only if it's non-empty. In fact, we can do the same for
the elif, too, simplifying it further. Technically this is overkill,
since we know the --depth parameter will not have whitespace (and
indeed, most callers do not bother quoting it), but it doesn't hurt for
the function to be careful.
It's somewhat amazing that no tests were failing. I think what happened
is that:
- the 3-arg form rarely triggered; any call with a non-empty $depth
and a $sha1 would work, but one with an empty $depth would only have
2 arguments
- because of the wrong arguments to "test", the shell would complain
and exit non-zero. So we never ran the middle conditional at all
- that left every call running "git fetch" with no arguments. A
well-written test could have detected the distinction here, but in
practice omitting --depth just means fetching more commits, and
fetching everything (rather than a single sha1) works as long as the
commit in question is reachable
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Restore a space that was lost in 8a0fc8d19d (stash: convert apply to
builtin, 2019-02-25).
Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
