Jacob Stopak [Tue, 20 Sep 2022 02:45:57 +0000 (19:45 -0700)]
Documentation: clean up various typos in technical docs
Used GNU "aspell check <filename>" to review various technical
documentation files with the default aspell dictionary. Ignored
false-positives between american and british english.
Signed-off-by: Jacob Stopak <jacob@initialcommit.io> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Stopak [Tue, 20 Sep 2022 02:45:56 +0000 (19:45 -0700)]
Documentation: clean up a few misspelled word typos
Used GNU "aspell check <filename>" to review various documentation
files with the default aspell dictionary. Ignored false-positives
between american and british english.
Signed-off-by: Jacob Stopak <jacob@initialcommit.io> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Stopak [Sun, 11 Sep 2022 10:23:20 +0000 (03:23 -0700)]
Documentation: fix various repeat word typos
Inspired by 24966cd982 ("doc: fix repeated words", 08-09-2019),
I ran "egrep -R "\<([a-zA-Z]+)\> \<\1\>" ./Documentation/*" to
find current cases of repeated words such as "the the" that were
quite clearly typos.
There were many false positives reported, such as "really really"
or valid uses of "that that" which I left alone.
Signed-off-by: Jacob Stopak <jacob@initialcommit.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 9 Sep 2022 19:02:25 +0000 (12:02 -0700)]
Merge branch 'jc/format-patch-force-in-body-from'
"git format-patch --from=<ident>" can be told to add an in-body
"From:" line even for commits that are authored by the given
<ident> with "--force-in-body-from"option.
* jc/format-patch-force-in-body-from:
format-patch: learn format.forceInBodyFrom configuration variable
format-patch: allow forcing the use of in-body From: header
pretty: separate out the logic to decide the use of in-body from
Junio C Hamano [Fri, 9 Sep 2022 19:02:24 +0000 (12:02 -0700)]
Merge branch 'js/add-p-diff-parsing-fix'
Those who use diff-so-fancy as the diff-filter noticed a regression
or two in the code that parses the diff output in the built-in
version of "add -p", which has been corrected.
* js/add-p-diff-parsing-fix:
add -p: ignore dirty submodules
add -p: gracefully handle unparseable hunk headers in colored diffs
add -p: detect more mismatches between plain vs colored diffs
Junio C Hamano [Tue, 6 Sep 2022 01:33:40 +0000 (18:33 -0700)]
Merge branch 'es/t4301-sed-portability-fix'
Test clean-up.
* es/t4301-sed-portability-fix:
t4301: emit blank line in more idiomatic fashion
t4301: fix broken &&-chains and add missing loop termination
t4301: account for behavior differences between sed implementations
Junio C Hamano [Tue, 6 Sep 2022 01:33:40 +0000 (18:33 -0700)]
Merge branch 'rs/tempfile-cleanup-race-fix'
The clean-up of temporary files created via mks_tempfile_dt() was
racy and attempted to unlink() the leading directory when signals
are involved, which has been corrected.
Ensure 'is_sparse_directory_entry()' receives a valid 'name_entry *' if one
exists in the list of tree(s) being unpacked in 'unpack_callback()'.
Currently, 'is_sparse_directory_entry()' is called with the first
'name_entry' in the 'names' list of entries on 'unpack_callback()'. However,
this entry may be empty even when other elements of 'names' are not (such as
when switching from an orphan branch back to a "normal" branch). As a
result, 'is_sparse_directory_entry()' could incorrectly indicate that a
sparse directory is *not* actually sparse because the name of the index
entry does not match the (empty) 'name_entry' path.
Fix the issue by using the existing 'name_entry *p' value in
'unpack_callback()', which points to the first non-empty entry in 'names'.
Because 'p' is 'const', also update 'is_sparse_directory_entry()'s
'name_entry *' argument to be 'const'.
Finally, add a regression test case.
Reported-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The auto-stashed local changes created by "git merge --autostash"
was mixed into a conflicted state left in the working tree, which
has been corrected.
* en/merge-unstash-only-on-clean-merge:
merge: only apply autostash when appropriate
Junio C Hamano [Thu, 1 Sep 2022 20:40:18 +0000 (13:40 -0700)]
Merge branch 'sg/parse-options-subcommand'
Introduce the "subcommand" mode to parse-options API and update the
command line parser of Git commands with subcommands.
* sg/parse-options-subcommand: (23 commits)
remote: run "remote rm" argv through parse_options()
maintenance: add parse-options boilerplate for subcommands
pass subcommand "prefix" arguments to parse_options()
builtin/worktree.c: let parse-options parse subcommands
builtin/stash.c: let parse-options parse subcommands
builtin/sparse-checkout.c: let parse-options parse subcommands
builtin/remote.c: let parse-options parse subcommands
builtin/reflog.c: let parse-options parse subcommands
builtin/notes.c: let parse-options parse subcommands
builtin/multi-pack-index.c: let parse-options parse subcommands
builtin/hook.c: let parse-options parse subcommands
builtin/gc.c: let parse-options parse 'git maintenance's subcommands
builtin/commit-graph.c: let parse-options parse subcommands
builtin/bundle.c: let parse-options parse subcommands
parse-options: add support for parsing subcommands
parse-options: drop leading space from '--git-completion-helper' output
parse-options: clarify the limitations of PARSE_OPT_NODASH
parse-options: PARSE_OPT_KEEP_UNKNOWN only applies to --options
api-parse-options.txt: fix description of OPT_CMDMODE
t0040-parse-options: test parse_options() with various 'parse_opt_flags'
...
Junio C Hamano [Thu, 1 Sep 2022 20:40:17 +0000 (13:40 -0700)]
Merge branch 'ds/bundle-uri-clone'
Implement "git clone --bundle-uri".
* ds/bundle-uri-clone:
clone: warn on failure to repo_init()
clone: --bundle-uri cannot be combined with --depth
bundle-uri: add support for http(s):// and file://
clone: add --bundle-uri option
bundle-uri: create basic file-copy logic
remote-curl: add 'get' capability
Thanks to always running `diff-index` and `diff-files` with the
`--numstat` option (the latter with `--ignore-submodules=dirty`) before
even generating any real diff to parse, the Perl version of `git add -p`
simply ignored dirty submodules and does not even offer them up for
staging.
However, the built-in variant did not use that flag because it tries to
run only one `diff` command, skipping the unneeded
`diff-index`/`diff-files` invocation of the Perl variant and therefore
only faithfully recapitulates what the Perl code does once it _does_
generate and parse the real diff.
This causes a problem when running the built-in `add -p` with
`diff-so-fancy` because that diff colorizer always inserts an empty line
before the diff header to ensure that it produces 4 lines as expected by
`git add -p` (the equivalent of the non-colorized `diff`, `index`, `---`
and `+++` lines). But `git diff-files` does not produce any `index` line
for dirty submodules.
The underlying problem is not even the discrepancy in lines, but that
`git add -p` presents diffs for dirty submodules: there is nothing that
_can_ be staged for those.
Let's fix that bug, and teach the built-in `add -p` to ignore dirty
submodules, too. This _incidentally_ also fixes the `diff-so-fancy`
problem ;-)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
add -p: gracefully handle unparseable hunk headers in colored diffs
In
https://lore.kernel.org/git/ecf6f5be-22ca-299f-a8f1-bda38e5ca246@gmail.com,
Phillipe Blain reported that the built-in `git add -p` command fails
when asked to use [`diff-so-fancy`][diff-so-fancy] to colorize the diff.
The reason is that this tool produces colored diffs with a hunk header
that does not contain any parseable `@@ ... @@` line range information,
and therefore we cannot detect any part in that header that comes after
the line range.
As proposed by Phillip Wood, let's take that for a clear indicator that
we should show the hunk headers verbatim. This is what the Perl version
of the interactive `add` command did, too.
Reported-by: Philippe Blain <levraiphilippeblain@gmail.com> Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
add -p: detect more mismatches between plain vs colored diffs
When parsing the colored version of a diff, the interactive `add`
command really relies on the colored version having the same number of
lines as the plain (uncolored) version. That is an invariant.
We already have code to verify correctly when the colored diff has less
lines than the plain diff. Modulo an off-by-one bug: If the last diff
line has no matching colored one, the code pretends to succeed, still.
To make matters worse, when we adjusted the test in 1e4ffc765db (t3701:
adjust difffilter test, 2020-01-14), we did not catch this because `add
-p` fails for a _different_ reason: it does not find any colored hunk
header that contains a parseable line range.
If we change the test case so that the line range _can_ be parsed, the
bug is exposed.
Let's address all of the above by
- fixing the off-by-one,
- adjusting the test case to allow `add -p` to parse the line range
- making the test case more stringent by verifying that the expected
error message is shown
Also adjust a misleading code comment about the now-fixed code.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the preceding commit $(C_OBJ) added in c373991375a (Makefile: list
generated object files in OBJECTS, 2010-01-26) became synonymous with
$(OBJECTS). Let's avoid the indirection and use the $(OBJECTS)
variable directly instead.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove the PPC_SHA1 implementation added in a6ef3518f9a ([PATCH] PPC
assembly implementation of SHA1, 2005-04-22). When this was added
Apple consumer hardware used the PPC architecture, and the
implementation was intended to improve SHA-1 speed there.
Since it was added we've moved to using sha1collisiondetection by
default, and anyone wanting hard-rolled non-DC SHA-1 implementation
can use OpenSSL's via the OPENSSL_SHA1 knob.
The PPC_SHA1 originally originally targeted 32 bit PPC, and later the
64 bit PPC 970 (a.k.a. Apple PowerPC G5). See 926172c5e48 (block-sha1:
improve code on large-register-set machines, 2009-08-10) for a
reference about the performance on G5 (a comment in block-sha1/sha1.c
being removed here).
I can't get it to do anything but segfault on both the BE and LE POWER
machines in the GCC compile farm[1]. Anyone who's concerned about
performance on PPC these days is likely to be using the IBM POWER
processors.
There have been proposals to entirely remove non-sha1collisiondetection
implementations from the tree[2]. I think per [3] that would be a bit
overzealous. I.e. there are various set-ups git's speed is going to be
more important than the relatively implausible SHA-1 collision attack,
or where such attacks are entirely mitigated by other means (e.g. by
incoming objects being checked with DC_SHA1).
But that really doesn't apply to PPC_SHA1 in particular, which seems
to have outlived its usefulness.
As this gets rid of the only in-tree *.S assembly file we can remove
the small bits of logic from the Makefile needed to build objects
from *.S (as opposed to *.c)
The code being removed here was also throwing warnings with the
"-pedantic" flag, it could have been fixed as 544d93bc3b4 (block-sha1:
remove use of obsolete x86 assembly, 2022-03-10) did for block-sha1/*,
but as noted above let's remove it instead.
1. https://cfarm.tetaneutral.net/machines/list/
Tested on gcc{110,112,135,203}, a mixture of POWER [789] ppc64 and
ppc64le. All segfault in anything needing object
hashing (e.g. t/t1007-hash-object.sh) when compiled with
PPC_SHA1=Y.
2. https://lore.kernel.org/git/20200223223758.120941-1-mh@glandium.org/
3. https://lore.kernel.org/git/20200224044732.GK1018190@coredump.intra.peff.net/
Acked-by: brian m. carlson" <sandals@crustytoothpaste.net> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 30 Aug 2022 20:40:56 +0000 (16:40 -0400)]
test-crontab: minor memory and error handling fixes
Since ee69e7884e (gc: use temporary file for editing crontab,
2022-08-28), we now insist that "argc == 3" (and otherwise return an
error). Coverity notes that this causes some dead code:
if (argc == 3)
fclose(from);
else
fclose(to);
as we will never trigger the else. This also causes a memory leak, since
we'll never close "to".
Now that all paths require 2 arguments, we can just reorganize the
function to check argc up front, and tweak the cleanup to do the right
thing for all cases.
While we're here, we can also notice some minor problems:
- we return a negative int via error() from what is essentially a
main() function; we should return a positive non-zero value for
error. Or better yet, we can just use usage(), which gives a better
message.
- while writing the usage message, we can note the one in the comment
was made out of date by ee69e7884e. But it also had a typo already,
calling the subcommand "cron" and not "crontab"
- we didn't check for an error from fopen(), meaning we would segfault
if the to-be-read file was missing. We can use xfopen() to catch
this.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 30 Aug 2022 19:46:01 +0000 (15:46 -0400)]
tempfile: update comment describing state transitions
Back when 1a9d15db25 (tempfile: a new module for handling temporary
files, 2015-08-10) added this comment, tempfile structs were held in
memory for the life of a process, and there were various guarantees
about which fields were valid in which states.
Since 422a21c6a0 (tempfile: remove deactivated list entries, 2017-09-05)
and 076aa2cbda (tempfile: auto-allocate tempfiles on heap, 2017-09-05),
the flow is quite different: objects come and go from the list, and
inactive ones are deallocated. And the previous commit removed the
"active" flag from the struct entirely.
Let's bring the comment up to date with the current code.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 30 Aug 2022 19:45:06 +0000 (15:45 -0400)]
tempfile: drop active flag
Our tempfile struct contains an "active" flag. Long ago, this flag was
important: tempfile structs were always allocated for the lifetime of
the program and added to a global linked list, and the active flag was
what told us whether a struct's tempfile needed to be cleaned up on
exit.
But since 422a21c6a0 (tempfile: remove deactivated list entries,
2017-09-05) and 076aa2cbda (tempfile: auto-allocate tempfiles on heap,
2017-09-05), we actually remove items from the list, and the active flag
is generally always set to true for any allocated struct. We set it to
true in all of the creation functions, and in the normal code flow it
becomes false only in deactivate_tempfile(), which then immediately
frees the struct.
So the flag isn't performing that role anymore, and in fact makes things
more confusing. Dscho noted that delete_tempfile() is a noop for an
inactive struct. Since 076aa2cbda taught it to free the struct when
deactivating, we'd leak any struct whose active flag is unset. But in
practice it's not a leak, because again, we'll free when we unset the
flag, and never see the allocated-but-inactive state.
Can we just get rid of the flag? The answer is yes, but it requires
looking at a few other spots:
1. I said above that the flag only becomes false before we deallocate,
but there's one exception: when we call remove_tempfiles() from a
signal or atexit handler, we unset the active flag as we remove
each file. This isn't important for delete_tempfile(), as nobody
would call it anymore, since we're exiting.
It does in theory provide us some protection against racily
double-removing a tempfile. If we receive a second signal while we
are already in the cleanup routines, we'll start the cleanup loop
again, and may visit the same tempfile. But this race already
exists, because calling unlink() and unsetting the active flag
aren't atomic! And it's OK in practice, because unlink() is
idempotent (barring the unlikely event that some other process
chooses our exact temp filename in that instant).
So dropping the active flag widens the race a bit, but it was
already there, and is fairly harmless in practice. If we really
care about addressing it, the right thing is probably to block
further signals while we're doing our cleanup (which we could
actually do atomically).
2. The active flag is declared as "volatile sig_atomic_t". The idea is
that it's the final bit that gets set to tell the cleanup routines
that the tempfile is ready to be used (or not used), and it's safe
to receive a signal racing with regular code which adds or removes
a tempfile from the list.
In practice, I don't think this is buying us anything. The presence
on the linked list is really what tells the cleanup routines to
look at the struct. That is already marked as "volatile". It's not
a sig_atomic_t, so it's possible that we could see a sheared write
there as an entry is added or removed. But that is true of the
current code, too! Before we can even look at the "active" flag,
we'd have to follow a link to the struct itself. If we see a
sheared write in the pointer to the struct, then we'll look at
garbage memory anyway, and there's not much we can do.
This patch removes the active flag entirely, using presence on the
global linked list as an indicator that a tempfile ought to be cleaned
up. We are already careful to add to the list as the final step in
activating. On deactivation, we'll make sure to remove from the list as
the first step, before freeing any fields. The use of the volatile
keyword should mean that those things happen in the expected order.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Christian Couder [Tue, 30 Aug 2022 10:50:46 +0000 (12:50 +0200)]
Documentation: clarify whitespace rules for trailers
Commit e4319562bc (trailer: be stricter in parsing separators, 2016-11-02)
restricted whitespaces allowed by `git interpret-trailers` in the "token"
part of the trailers it reads. This commit didn't update the related
documentation in Documentation/git-interpret-trailers.txt though.
Also commit 60ef86a162 (trailer: support values folded to multiple lines,
2016-10-21) updated the documentation, but didn't make it clear how many
whitespace characters are allowed at the beginning of new lines in folded
values.
Let's fix both of these issues by rewriting the paragraph describing
what whitespaces are allowed by `git interpret-trailers` in the trailers
it reads.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 29 Aug 2022 21:55:15 +0000 (14:55 -0700)]
Merge branch 'es/fix-chained-tests'
Fix broken "&&-" chains and failures in early iterations of a loop.
* es/fix-chained-tests:
t5329: notice a failure within a loop
t: detect and signal failure within loop
t1092: fix buggy sparse "blame" test
t2407: fix broken &&-chains in compound statement
Junio C Hamano [Mon, 29 Aug 2022 21:55:14 +0000 (14:55 -0700)]
Merge branch 'sg/xcalloc-cocci-fix'
xcalloc(), imitating calloc(), takes "number of elements of the
array", and "size of a single element", in this order. A call that
does not follow this ordering has been corrected.
* sg/xcalloc-cocci-fix:
promisor-remote: fix xcalloc() argument order
Junio C Hamano [Mon, 29 Aug 2022 21:55:13 +0000 (14:55 -0700)]
Merge branch 'mg/sequencer-untranslate-reflog'
The sequencer machinery translated messages left in the reflog by
mistake, which has been corrected.
* mg/sequencer-untranslate-reflog:
sequencer: do not translate command names
sequencer: do not translate parameters to error_resolve_conflict()
sequencer: do not translate reflog messages
Junio C Hamano [Mon, 29 Aug 2022 21:55:12 +0000 (14:55 -0700)]
Merge branch 'jk/unused-fixes'
Code clean-up to remove unused function parameters.
* jk/unused-fixes:
xdiff: drop unused mmfile parameters from xdl_do_patience_diff()
reflog: assert PARSE_OPT_NONEG in parse-options callbacks
reftable: drop unused parameter from reader_seek_linear()
verify_one_sparse(): drop unused parameters
match_pathname(): drop unused "flags" parameter
log-tree: drop unused commit param in remerge_diff()
xdiff: drop unused mmfile parameters from xdl_do_histogram_diff()
Junio C Hamano [Mon, 29 Aug 2022 21:55:12 +0000 (14:55 -0700)]
Merge branch 'vd/scalar-enables-fsmonitor'
"scalar" now enables built-in fsmonitor on enlisted repositories,
when able.
* vd/scalar-enables-fsmonitor:
scalar: update technical doc roadmap with FSMonitor support
scalar unregister: stop FSMonitor daemon
scalar: enable built-in FSMonitor on `register`
scalar: move config setting logic into its own function
scalar-delete: do not 'die()' in 'delete_enlistment()'
scalar-[un]register: clearly indicate source of error
scalar-unregister: handle error codes greater than 0
scalar: constrain enlistment search
Junio C Hamano [Mon, 29 Aug 2022 21:55:11 +0000 (14:55 -0700)]
Merge branch 'en/ancestry-path-in-a-range'
"git rev-list --ancestry-path=C A..B" is a natural extension of
"git rev-list A..B"; instead of choosing a subset of A..B to those
that have ancestry relationship with A, it lets a subset with
ancestry relationship with C.
* en/ancestry-path-in-a-range:
revision: allow --ancestry-path to take an argument
t6019: modernize tests with helper
rev-list-options.txt: fix simple typo
Junio C Hamano [Mon, 29 Aug 2022 21:55:11 +0000 (14:55 -0700)]
Merge branch 'mt/rot13-in-c'
Test portability improvements.
* mt/rot13-in-c:
tests: use the new C rot13-filter helper to avoid PERL prereq
t0021: implementation the rot13-filter.pl script in C
t0021: avoid grepping for a Perl-specific string at filter output
Junio C Hamano [Mon, 29 Aug 2022 21:55:10 +0000 (14:55 -0700)]
Merge branch 'ds/decorate-filter-tweak'
The namespaces used by "log --decorate" from "refs/" hierarchy by
default has been tightened.
* ds/decorate-filter-tweak:
fetch: use ref_namespaces during prefetch
maintenance: stop writing log.excludeDecoration
log: create log.initialDecorationSet=all
log: add --clear-decorations option
log: add default decoration filter
log-tree: use ref_namespaces instead of if/else-if
refs: use ref_namespaces for replace refs base
refs: add array of ref namespaces
t4207: test coloring of grafted decorations
t4207: modernize test
refs: allow "HEAD" as decoration filter
As the need to use the "--force-in-body-from" option primarily is
tied to which mailing list the mails go to (and get their From:
address mangled), it is likely that a user who needs to use this
option once to interact with their upstream project needs to use it
for all patches they send out.
Add a configuration variable, suitable for setting in the local
configuration file per repository, for this.
Junio C Hamano [Mon, 29 Aug 2022 21:38:36 +0000 (14:38 -0700)]
format-patch: allow forcing the use of in-body From: header
Users may be authoring and committing their commits under the same
e-mail address they use to send their patches from, in which case
they shouldn't need to use the in-body From: line in their outgoing
e-mails. At the receiving end, "git am" will use the address on the
"From:" header of the incoming e-mail and all should be well.
Some mailing lists, however, mangle the From: address from what the
original sender had; in such a situation, the user may want to add
the in-body "From:" header even for their own patches.
"git format-patch --[no-]force-in-body-from" was invented for such
users.
Junio C Hamano [Mon, 29 Aug 2022 21:38:35 +0000 (14:38 -0700)]
pretty: separate out the logic to decide the use of in-body from
When pretty-printing the log message for a given commit in the
e-mail format (e.g. "git format-patch"), we add an in-body "From:"
header when the author identity of the commit is different from the
identity of the person whose identity appears in the header of the
e-mail (the latter is passed with them "--from" option).
Split out the logic into a helper function, as we would want to
extend the condition further.
Eric Sunshine [Sun, 28 Aug 2022 05:17:58 +0000 (05:17 +0000)]
t4301: fix broken &&-chains and add missing loop termination
Fix &&-chain breaks in a couple tests which went unnoticed due to blind
spots in the &&-chain linters. In particular, the "magic exit code 117"
&&-chain checker built into test-lib.sh only recognizes broken &&-chains
at the top-level; it does not work within `{...}` groups, `(...)`
subshells, `$(...)` substitutions, or within bodies of compound
statements, such as `if`, `for`, `while`, `case`, etc. Furthermore,
`chainlint.sed`, which detects broken &&-chains only in `(...)`
subshells, missed these cases (which are in subshells) because it
(surprisingly) neglects to check for intact &&-chain on single-line
`for` loops.
While at it, explicitly signal failure of commands within the `for`
loops (which might arise due to the filesystem being full or "inode"
exhaustion). This is important since failures within `for` and `while`
loops can go unnoticed if not detected and signaled manually since the
loop itself does not abort when a contained command fails, nor will a
failure necessarily be detected when the loop finishes since the loop
returns the exit code of the last command it ran on the final iteration,
which may not be the command which failed.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson [Sun, 28 Aug 2022 21:41:43 +0000 (21:41 +0000)]
gc: use temporary file for editing crontab
While cron is specified by POSIX, there are a wide variety of
implementations in use. "git maintenance" assumes that the
"crontab" command can be fed from its standard input the new
contents and the syntax to do so is not to have any filename
argument, as POSIX describes. However, on FreeBSD, the cron
implementation requires a file name argument: if the user wants to
edit standard input, they must specify "-".
Unfortunately, POSIX systems do not have to interpret "-" on the
command line of crontab as a request to read from the standard
input. Blindly adding "-" on the command line would not work as a
general solution.
Since POSIX tells us that cron must accept a file name argument, let's
solve this problem by specifying a temporary file instead. This will
ensure that we work with the vast majority of implementations.
Note that because delete_tempfile closes the file for us, we should not
call fclose here on the handle, since doing so will introduce a double
free.
Reported-by: Renato Botelho <garga@FreeBSD.org> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sun, 28 Aug 2022 12:50:50 +0000 (08:50 -0400)]
pack-bitmap-write: drop unused pack_idx_entry parameters
Our write_selected_commits_v1() function takes an array of
pack_idx_entry structs. We used to need them for computing commit
positions, but since aa30162559 (bitmap: move `get commit positions`
code to `bitmap_writer_finish`, 2022-08-14), the caller passes in a
separate array of positions for us. We can drop the unused array (and
its matching length parameter).
Likewise, when we added write_lookup_table() in 93eb41e240
(pack-bitmap-write.c: write lookup table extension, 2022-08-14), it
receives the same array of positions. So its "index" parameter was never
used at all.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sun, 28 Aug 2022 10:34:47 +0000 (12:34 +0200)]
test-mergesort: use mem_pool for sort input
The previous patch almost halved the number of heap allocations for the
sort subcommand. Reduce it further by using a mem_pool for the line
objects.
Note that t/perf/run can't be used directly to compare two versions of
test-mergesort because it always runs the helpers from the checked-out
version. So I hand-merged the results of separate runs before and with
this patch:
René Scharfe [Sun, 28 Aug 2022 10:34:05 +0000 (12:34 +0200)]
test-mergesort: read sort input all at once
The sort subcommand of test-mergesort is used to test the performance of
sorting linked lists. It reads lines from stdin, sorts them and prints
the result to stdout. Two heap allocations are done per line: One for
the linked list item and one for the actual line string. That imposes a
significant amount of allocation overhead.
Reduce it by doing the same as the sort subcommand of test-string-list,
namely to read the whole input file into a single buffer and then split
it in-place.
Note that t/perf/run can't be used directly to compare two versions of
test-mergesort because it always runs the helpers from the checked-out
version. So I hand-merged the results of separate runs before and with
this patch:
Eric Sunshine [Sun, 28 Aug 2022 05:17:57 +0000 (05:17 +0000)]
t4301: account for behavior differences between sed implementations
It is a common pattern in this script to write the result of
`merge-tree -z` (NUL-termination mode) to an "actual" file and then
manually append a newline to that file so that it can be diff'd easily
with a hand-crafted "expect" file which itself ends with a newline since
it has been created by standard Unix tools which terminate lines by
default. For instance:
which means that, unlike all other cases, when anonymize_hash() is
called, the file being anonymized does not end with a newline. As a
result, this test fails on some platforms.
anonymize_hash() is implemented like this:
anonymize_hash() {
sed -e "s/[0-9a-f]\{40,\}/HASH/g" "$@"
}
The problem arises due to differences in behavior of various `sed`
implementations when fed an incomplete line (lacking a newline).
Although most modern `sed` implementations output such a line
unmolested (i.e. without a newline), some older `sed` implementations
forcibly add a newline to the incomplete line (giving the output an
extra unexpected newline), while other very old implementations simply
swallow an incomplete line and don't emit it at all (making the output
shorter than expected).
Fix this test by manually adding the newline before passing it through
`sed`, thus ensuring identical behavior with all `sed` implementation,
and bringing the test in line with other tests in this script.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Fri, 26 Aug 2022 22:46:29 +0000 (00:46 +0200)]
tempfile: avoid directory cleanup race
The temporary directory created by mks_tempfile_dt() is deleted by first
deleting the file within, then truncating the filename strbuf and
passing the resulting string to rmdir(2). When the cleanup routine is
invoked concurrently by a signal handler we can end up passing the now
truncated string to unlink(2), however, which could cause problems on
some systems.
Avoid that issue by remembering the directory name separately. This way
the paths stay unchanged. A signal handler can still race with normal
cleanup, but deleting the same files and directories twice is harmless.
Reported-by: Jeff King <peff@peff.net> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 26 Aug 2022 18:13:12 +0000 (11:13 -0700)]
Merge branch 'jk/fsck-tree-mode-bits-fix' into maint
"git fsck" reads mode from tree objects but canonicalizes the mode
before passing it to the logic to check object sanity, which has
hid broken tree objects from the checking logic. This has been
corrected, but to help exiting projects with broken tree objects
that they cannot fix retroactively, the severity of anomalies this
code detects has been demoted to "info" for now.
source: <YvQcNpizy9uOZiAz@coredump.intra.peff.net>
* jk/fsck-tree-mode-bits-fix:
fsck: downgrade tree badFilemode to "info"
fsck: actually detect bad file modes in trees
tree-walk: add a mechanism for getting non-canonicalized modes
Junio C Hamano [Fri, 26 Aug 2022 18:13:12 +0000 (11:13 -0700)]
Merge branch 'js/safe-directory-plus' into maint
Platform-specific code that determines if a directory is OK to use
as a repository has been taught to report more details, especially
on Windows.
source: <pull.1286.v2.git.1659965270.gitgitgadget@gmail.com>
* js/safe-directory-plus:
mingw: handle a file owned by the Administrators group correctly
mingw: be more informative when ownership check fails on FAT32
mingw: provide details about unsafe directories' ownership
setup: prepare for more detailed "dubious ownership" messages
setup: fix some formatting
Junio C Hamano [Fri, 26 Aug 2022 18:13:12 +0000 (11:13 -0700)]
Merge branch 'pw/use-glibc-tunable-for-malloc-optim' into maint
Avoid repeatedly running getconf to ask libc version in the test
suite, and instead just as it once per script.
source: <pull.1311.git.1659620305757.gitgitgadget@gmail.com>
* pw/use-glibc-tunable-for-malloc-optim:
tests: cache glibc version check
Junio C Hamano [Fri, 26 Aug 2022 18:13:10 +0000 (11:13 -0700)]
Merge branch 'jk/struct-zero-init-with-older-gcc' into maint
Older gcc with -Wall complains about the universal zero initializer
"struct s = { 0 };" idiom, which makes developers' lives
inconvenient (as -Werror is enabled by DEVELOPER=YesPlease). The
build procedure has been tweaked to help these compilers.
source: <YuQ60ZUPBHAVETD7@coredump.intra.peff.net>
* jk/struct-zero-init-with-older-gcc:
config.mak.dev: squelch -Wno-missing-braces for older gcc
Junio C Hamano [Fri, 26 Aug 2022 18:13:10 +0000 (11:13 -0700)]
Merge branch 'js/mingw-with-python' into maint
Conditionally allow building Python interpreter on Windows
source: <pull.1306.v2.git.1659109272.gitgitgadget@gmail.com>
* js/mingw-with-python:
mingw: remove unneeded `NO_CURL` directive
mingw: remove unneeded `NO_GETTEXT` directive
windows: include the Python bits when building Git for Windows
Junio C Hamano [Fri, 26 Aug 2022 18:13:10 +0000 (11:13 -0700)]
Merge branch 'ca/unignore-local-installation-on-windows' into maint
Fix build procedure for Windows that uses CMake so that it can pick
up the shell interpreter from local installation location.
source: <pull.1304.git.1658912756815.gitgitgadget@gmail.com>
* ca/unignore-local-installation-on-windows:
cmake: support local installations of git
bitmap-lookup-table: add performance tests for lookup table
Add performance tests to verify the performance of lookup table.
`p5310-pack-bitmaps.sh` contain tests with and without lookup table.
`p5312-pack-bitmaps-revs.sh` contain same tests with and without
lookup table but with `pack.writeReverseIndex` enabled.
Lookup table makes Git run faster in most of the cases. Below is the
result of `t/perf/p5310-pack-bitmaps.sh`.`perf/p5326-multi-pack-bitmaps.sh`
gives similar result. The repository used in the test is linux kernel.
Test this tree
-----------------------------------------------------------------------
5310.4: enable lookup table: false 0.01(0.00+0.00)
5310.5: repack to disk 320.89(230.20+23.45)
5310.6: simulated clone 14.04(5.78+1.79)
5310.7: simulated fetch 1.95(3.05+0.20)
5310.8: pack to file (bitmap) 44.73(20.55+7.45)
5310.9: rev-list (commits) 0.78(0.46+0.10)
5310.10: rev-list (objects) 4.07(3.97+0.08)
5310.11: rev-list with tag negated via --not 0.06(0.02+0.03)
--all (objects)
5310.12: rev-list with negative tag (objects) 0.21(0.15+0.05)
5310.13: rev-list count with blob:none 0.24(0.17+0.06)
5310.14: rev-list count with blob:limit=1k 7.07(5.92+0.48)
5310.15: rev-list count with tree:0 0.25(0.17+0.07)
5310.16: simulated partial clone 5.67(3.28+0.64)
5310.18: clone (partial bitmap) 16.05(8.34+1.86)
5310.19: pack to file (partial bitmap) 59.76(27.22+7.43)
5310.20: rev-list with tree filter (partial bitmap) 0.90(0.18+0.16)
5310.24: enable lookup table: true 0.01(0.00+0.00)
5310.25: repack to disk 319.73(229.30+23.01)
5310.26: simulated clone 13.69(5.72+1.78)
5310.27: simulated fetch 1.84(3.02+0.16)
5310.28: pack to file (bitmap) 45.63(20.67+7.50)
5310.29: rev-list (commits) 0.56(0.39+0.8)
5310.30: rev-list (objects) 3.77(3.74+0.08)
5310.31: rev-list with tag negated via --not 0.05(0.02+0.03)
--all (objects)
5310.32: rev-list with negative tag (objects) 0.21(0.15+0.05)
5310.33: rev-list count with blob:none 0.23(0.17+0.05)
5310.34: rev-list count with blob:limit=1k 6.65(5.72+0.40)
5310.35: rev-list count with tree:0 0.23(0.16+0.06)
5310.36: simulated partial clone 5.57(3.26+0.59)
5310.38: clone (partial bitmap) 15.89(8.39+1.84)
5310.39: pack to file (partial bitmap) 58.32(27.55+7.47)
5310.40: rev-list with tree filter (partial bitmap) 0.73(0.18+0.15)
Test 4-15 are tested without using lookup table. Same tests are
repeated in 16-30 (using lookup table).
Mentored-by: Taylor Blau <me@ttaylorr.com> Co-Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-bitmap-write: learn pack.writeBitmapLookupTable and add tests
Teach Git to provide a way for users to enable/disable bitmap lookup
table extension by providing a config option named 'writeBitmapLookupTable'.
Default is false.
Also add test to verify writting of lookup table.
Mentored-by: Taylor Blau <me@ttaylorr.com> Co-Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Co-Authored-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The bitmap lookup table extension was documented by an earlier
change, but Git does not yet know how to write that extension.
Teach Git to write bitmap lookup table extension. The table contains
the list of `N` <commit_pos, offset, xor_row>` triplets. These
triplets are sorted according to their commit pos (ascending order).
The meaning of each data in the i'th triplet is given below:
- commit_pos stores commit position (in the pack-index or midx).
It is a 4 byte network byte order unsigned integer.
- offset is the position (in the bitmap file) from which that
commit's bitmap can be read.
- xor_row is the position of the triplet in the lookup table
whose bitmap is used to compress this bitmap, or `0xffffffff`
if no such bitmap exists.
Mentored-by: Taylor Blau <me@ttaylorr.com> Co-mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Co-authored-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
bitmap: move `get commit positions` code to `bitmap_writer_finish`
The `write_selected_commits_v1` function takes care of writing commit
positions along with their corresponding bitmaps in the disk. It is
OK because this `search commit position of a given commit` algorithm
is needed only once here. But in later changes of the `lookup table
extension series`, we need same commit positions which means we have
to run the above mentioned algorithm one more time.
Move the `search commit position of a given commit` algorithm to
`bitmap_writer_finish()` and use the `commit_positions` array
to get commit positions of their corresponding bitmaps.
Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When reading bitmap file, Git loads each and every bitmap one by one
even if all the bitmaps are not required. A "bitmap lookup table"
extension to the bitmap format can reduce the overhead of loading
bitmaps which stores a list of bitmapped commit id pos (in the midx
or pack, along with their offset and xor offset. This way Git can
load only the necessary bitmaps without loading the previous bitmaps.
Older versions of Git ignore the lookup table extension and don't
throw any kind of warning or error while parsing the bitmap file.
Add some information for the new "bitmap lookup table" extension in the
bitmap-format documentation.
Mentored-by: Taylor Blau <me@ttaylorr.com> Co-Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Co-Authored-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `git range-diff` command can be quite expensive, which is not a
surprise given that the underlying algorithm to match up pairs of
commits between the provided two commit ranges has a cubic runtime.
Therefore it makes sense to restrict the commit ranges as much as
possible, to reduce the amount of input to that O(N^3) algorithm.
In chatty repositories with wide trees, this is not necessarily
possible merely by choosing commit ranges wisely.
Let's give users another option to restrict the commit ranges: by
providing a pathspec. That helps in repositories with wide trees because
it is likely that the user has a good idea which subset of the tree they
are actually interested in.
Example:
git range-diff upstream/main upstream/seen HEAD -- range-diff.c
This shows commits that are either in the local branch or in `seen`, but
not in `main`, skipping all commits that do not touch `range-diff.c`.
Note: Since we piggy-back the pathspecs onto the `other_arg` mechanism
that was introduced to be able to pass through the `--notes` option to
the revision machinery, we must now ensure that the `other_arg` array is
appended at the end (the revision range must come before the pathspecs,
if any).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch lets `range-diff` validate the arguments not only when
invoked with one or two arguments, but also in the code path where three
arguments are handled.
While at it, we now use `usage_msg_opt*()` consistently.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In d9c66f0b5bf (range-diff: first rudimentary implementation,
2018-08-13), we introduced the argument handling of the `range-diff`
command, special-casing three different stanzas based on the argument
count.
The somewhat unorthodox order (first handling the case of 2 arguments,
then 3, then 1) was chosen for clarity: the natural argument number is 2
because that is how many revision ranges are used internally. The code
to handle three arguments is relatively trivial, so it was added next.
And finally, the code to ungarble a single symmetric range into two
separate ones was added, because it was the most complicated (the most
inelegant part being about interpreting empty sides of the symmetric
range as `HEAD`).
In preparation for allowing pathspecs in `git range-diff` invocations,
where we no longer have the luxury of using the number of arguments to
disambiguate between these three different ways to specify the commit
ranges, we need to order these cases by argument count, in descending
order.
This patch is best viewed with `--color-moved`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 26 Aug 2022 03:49:19 +0000 (03:49 +0000)]
t64xx: convert 'test_create_repo' to 'git init'
Convert the merge-specific tests (those in the t64xx range) over to
using 'git init' instead of 'test_create_repo'.
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 25 Aug 2022 21:42:31 +0000 (14:42 -0700)]
Merge branch 'jk/pipe-command-nonblock'
Fix deadlocks between main Git process and subprocess spawned via
the pipe_command() API, that can kill "git add -p" that was
reimplemented in C recently.
* jk/pipe-command-nonblock:
pipe_command(): mark stdin descriptor as non-blocking
pipe_command(): handle ENOSPC when writing to a pipe
pipe_command(): avoid xwrite() for writing to pipe
git-compat-util: make MAX_IO_SIZE define globally available
nonblock: support Windows
compat: add function to enable nonblocking pipes
Junio C Hamano [Thu, 25 Aug 2022 21:42:29 +0000 (14:42 -0700)]
Merge branch 'en/submodule-merge-messages-fixes'
Further update the help messages given while merging submodules.
* en/submodule-merge-messages-fixes:
merge-ort: provide helpful submodule update message when possible
merge-ort: avoid surprise with new sub_flag variable
merge-ort: remove translator lego in new "submodule conflict suggestion"
submodule merge: update conflict error message
Jeff King [Thu, 25 Aug 2022 10:51:40 +0000 (06:51 -0400)]
remote: run "remote rm" argv through parse_options()
The "git remote rm" command's option parsing is fairly primitive: it
insists on a single argument, which it treats as the remote name, and
displays a usage message otherwise.
This is OK, and maybe even convenient, as you could run:
git remote rm --foo
to drop a remote named "--foo". But it's also weirdly unlike most of the
rest of Git, which would complain that there is no option "--foo". The
right way to spell it by our conventions is:
git remote rm -- --foo
but this doesn't currently work.
So let's bring the command in line with the rest of Git (including its
sibling subcommands!) by feeding argv to parse_options(). We already
have an empty options array for the usage helper.
Note that we have to adjust the argc index down by one, as
parse_options() eats the program name from the start of the array.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Aug 2022 10:51:06 +0000 (06:51 -0400)]
maintenance: add parse-options boilerplate for subcommands
Several of the git-maintenance subcommands don't take any options, so
they don't bother looking at argv at all. This means they'll silently
accept garbage, like:
$ git maintenance register --foo
[no output]
$ git maintenance stop bar
[no output]
Let's give them the basic boilerplate to detect and handle these cases:
$ git maintenance stop bar
usage: git maintenance stop
We could reduce the number of lines of code here a bit with a shared
helper function. But it's worth building out the boilerplate, as it may
serve as the base for adding options later.
Note one complication: maintenance_start() calls directly into
maintenance_register(), so it now needs to pass a plausible argv (we
don't care, but parse_options() is expecting there to at least be an
argv[0] program name). This is an extra line of code, but it eliminates
the need for an explanatory comment.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 25 Aug 2022 10:47:00 +0000 (06:47 -0400)]
pass subcommand "prefix" arguments to parse_options()
Recent commits such as bf0a6b65fc (builtin/multi-pack-index.c: let
parse-options parse subcommands, 2022-08-19) converted a few functions
to match our usual argc/argv/prefix conventions, but the prefix argument
remains unused.
However, there is a good use for it: they should pass it to their own
parse_options() functions, where it may be used to adjust the value of
any filename options. In all but one of these functions, there's no
behavior change, since they don't use OPT_FILENAME. But this is an
actual fix for one option, which you can see by modifying the test suite
like so:
diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh
index 4fe57414c1..d0974d4371 100755
--- a/t/t5326-multi-pack-bitmaps.sh
+++ b/t/t5326-multi-pack-bitmaps.sh
@@ -186,7 +186,11 @@ test_expect_success 'writing a bitmap with --refs-snapshot' '
# Then again, but with a refs snapshot which only sees
# refs/tags/one.
- git multi-pack-index write --bitmap --refs-snapshot=snapshot &&
+ (
+ mkdir subdir &&
+ cd subdir &&
+ git multi-pack-index write --bitmap --refs-snapshot=../snapshot
+ ) &&
I'd emphasize that this wasn't broken by bf0a6b65fc; it has been broken
all along, because the sub-function never got to see the prefix. It is
that commit which is actually enabling us to fix it (and which also
brought attention to the problem because it triggers -Wunused-parameter!)
The other functions changed here don't use OPT_FILENAME at all. In their
cases this isn't fixing anything visible, but it's following the usual
pattern and future-proofing them against somebody adding new options and
being surprised.
I didn't include a test for the one visible case above. We don't
generally test routine parse-options behavior for individual options.
The challenge here was finding the problem, and now that this has been
done, it's not likely to regress. Likewise, we could apply the patch
above to cover it "for free" but it makes reading the rest of the test
unnecessarily complicated.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
