[pull] master from git:master - #258
Merged
Merged
Conversation
apply-one-time-script.sh is a test helper that executes a "one-time-script" responsible for modifying the response normally returned by git-http-backend. apply-one-time-script.sh should run "one-time-script" once and return a modified response once. However, sometimes a race between multiple concurrent requests causes apply-one-time-script.sh to misbehave and return multiple modified responses or an empty response that results in: fatal: ... The requested URL returned error: 500 fatal: could not fetch <oid> from promisor remote This can be seen in the flaky failure of t5616.47 on the macOS CI runners. Fix the logic that checks if "one-time-script" has returned its modified response by chaining "rm one-time-script" with its execution. This ensures a racing script does not also have the opportunity to execute "one-time-script". Add t/t5567-one-time-script.sh to verify the race is fixed. Implement a stub "git-http-backend" that intentionally invokes a concurrent request, and check that only one modified response is returned without error. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
http-429.sh is a helper for testing retry logic. It uses "test -f" to check for the existence of a state file and later uses "touch" or "rm -f" on that file to determine if it should return a 429. This method of managing state can fail if the helper script is invoked concurrently. However, this failure does not currently manifest itself since the helper is invoked sequentially. As a preventive measure, fix the state management logic so it relies on an atomic mkdir operation to mark that a 429 was returned. When $retry_after is "permanent", always return 429 now that we do not rely on a state file that is "touch"ed and "rm"ed to indicate when to respond with a 429. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update t/lib-httpd.sh to document the fixes applied to apply-one-time-script.sh and http-429.sh for future developers working on helper scripts. Add concrete examples of patterns and anti-patterns that should be considered when handling state management. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The documented purpose of imap-send is to upload draft emails for sending later, but it did not have any way to mark the messages as \Draft, so some email clients presented the result as an un-editable, un-sendable email even if it happened to be in a "Drafts" folder. Signed-off-by: Wolfgang Faust <contrib-git@wolfgangfaust.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git stash apply", "pop" and "branch" exit with status 1 both when applying the stash entry resulted in conflicts and when they fail for other reasons, so callers cannot tell the two apart. Follow the convention of "git merge-tree" and the merge strategies, which exit with status 1 to indicate conflicts and with a different non-zero status for errors: those subcommands now exit with status 1 only when applying the stash entry resulted in conflicts, in which case the stash entry is left in place, and exit with status 128, the status die() uses, when they fail for other reasons. Document the exit statuses. The only subcommand implementations that can return a positive value are "apply", "pop" and "branch", which return the value of do_apply_stash(): "apply" returns it directly, and "pop" and "branch" drop the stash entry, via do_drop_stash(), which always returns 0, only when the application succeeded. do_apply_stash() only returns a positive value when the three-way merge was unclean. cmd_stash() now maps negative values to 128 and passes positive values through as the exit status, so exit status 1 unambiguously indicates conflicts. enum stash_apply_result makes the convention explicit, and the autostash helpers use it to tell users that their stashed changes were saved when applying them fails. Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git checkout -m" stashes the user's local changes when it cannot
perform the checkout, and then applies the stash. When applying the
stash results in conflicts, the advice on how to deal with them is
printed directly on top of the branch-switch message ("Switched to
branch ..."), making the two hard to tell apart. Print a blank line
in between so that the advice and the branch-switch message are
visually distinct.
apply_autostash_ref() reports whether applying the stash resulted in
conflicts via its enum stash_apply_result return value, so only print
the blank line in the conflicted case.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When working with partial clones, callers often need only the missing object IDs. Today that means post-processing --missing=print to drop present objects and strip the leading '?': git rev-list --objects --all --missing=print | perl -ne 'print if s/^[?]//' This is for a one-shot walk, not a fetch loop. Callers already have --missing=print and strip the leading '?'. Gitaly does that when packing a quarantine: '?' lines are objects that must already exist in the main repo. Tests do the same (is this blob still missing). --missing-only is just that list without the prefix. Add --missing-only. Use it with --missing=print or --missing=print-info to print only missing objects. --missing= still picks the format; --missing-only only filters. The leading '?' is omitted. With print-info, path= and type= are still shown. Require --missing=print or --missing=print-info. Reject --count and --disk-usage. Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When garbage collecting rerere entries we need to figure out whether any given entry is stale before pruning it. In a subsequent commit we're about to introduce a second caller that wants to determine staleness, but the logic is not currently reusable. Extract the logic to compute staleness by introducing two new helper functions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "rerere-gc" maintenance task is responsible for pruning rerere
entries older than a certain configurable cutoff point. Whether or not
the task gets run during auto-maintenance can be configured via
"maintenance.rerere-gc.auto":
- A negative value indicates that maintenance should always run.
- A zero value indicates that maintenance should never run.
- Otherwise, a positive value indicates that maintenance should always
run in case we have at least a single rerere entry.
While the first two conditions are sensible, the last one is less so as
it does not account for whether we would even prune old entries in the
first place. Instead, it effectively implies that we unconditionally
spawn "git rerere gc" when rerere is enabled. Chances are high though
that there is nothing to prune, as the default cutoff dates are 60 days
for resolved rerere entries and 15 days for unresolved ones.
Besides being a waste of compute, it also obstructs concurrent processes
that want to write new resolutions as garbage collection takes a central
lock file, as reported in [1]. That race is a longstanding one that
existed even before we introduced fine-grained maintenance tasks, and
the proper fix is to use a locking timeout in the writing processes. But
the race is made worse by us performing garbage collection a lot more
often.
Refine the heuristic to take into account whether any entries can be
pruned in the first place. This ensures that we'll only ever run this
task in situations where it will do anything, and should thus result in
a lot less frequent invocations of "git rerere gc".
Furthermore, tweak the meaning of "maintenance.rerere-gc.auto" so that
positive values allow the user to configure the number of prunable
entries that need to exist before we run it and set the default value to
512. This number is pulled out of thin air, but it ensures that we know
to batch-delete entries instead of pruning every single entry that is
older than the cutoff point.
Note that this now requires us to actually open the rerere-entry
directories and stat the individual files in there, which does add a bit
of overhead when one has lots of rerere entries. To counteract this
overhead, we thus use the same sampling heuristic as we do for loose
objects, where we only consider those entries that start with a "17".
[1]: <pull.2214.git.1788337897490.gitgitgadget@gmail.com>
Reported-by: Thomas Bachem <mail@thomasbachem.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
CGI helper scripts used by HTTP-related test scripts have been updated to use atomic filesystem operations, preventing race conditions when Apache handles concurrent requests. * mm/lib-httpd-cgi-safe: t/lib-httpd: document writing concurrency-safe CGI helpers t/lib-httpd: make http-429 first-request check atomic t/lib-httpd: fix apply-one-time-script race under concurrent requests
The 'git imap-send' command has been taught to take the '--draft' option to mark uploaded messages as drafts, which helps some email clients render them properly for editing and sending. * wf/imap-send-draft: imap-send: add --draft to set IMAP \Draft flag
The autostash fallback in 'git checkout -m' has been refined to only retry when there are local changes. Additionally, a blank line now visually separates autostash conflict advice from the subsequent branch-switch message. * hn/checkout-m-autostash-refine: checkout: separate autostash conflict advice from branch-switch message stash: reserve exit status 1 for conflicts
"git maintenance" triggered "rerere gc" in unappropriate times and interfered with "git rebase" etc. too much. The conditions "rerere gc" gets triggered have been tweaked. * ps/tune-rerere-gc: builtin/maintenance: improve heuristic for "rerere gc" rerere: extract logic to determine whether entries are stale
The git rev-list command has been augmented with a '--missing-only' option that filters the output to only show missing objects, stripping the leading '?' character and suppressing present objects, which is useful when used in combination with '--missing=print' or '--missing=print-info'. * sa/rev-list-missing-only: rev-list: add --missing-only option to filter output
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )