Taylor Blau [Mon, 31 Oct 2022 01:04:41 +0000 (21:04 -0400)]
Merge branch 'jz/patch-id'
A new "--include-whitespace" option is added to "git patch-id", and
existing bugs in the internal patch-id logic that did not match
what "git patch-id" produces have been corrected.
* jz/patch-id:
builtin: patch-id: remove unused diff-tree prefix
builtin: patch-id: add --verbatim as a command mode
patch-id: fix patch-id for mode changes
builtin: patch-id: fix patch-id with binary diffs
patch-id: use stable patch-id for rebases
patch-id: fix stable patch id for binary / header-only
Philip Oakley [Sat, 29 Oct 2022 16:41:10 +0000 (17:41 +0100)]
glossary: add "commit graph" description
Git has an additional "commit graph" capability that supplements the
normal commit object's directed acyclic graph (DAG). The supplemental
commit graph file is designed for speed of access.
Describe the commit graph both from the normative DAG view point and
from the commit graph file perspective.
Also, clarify the link between the branch ref and branch tip
by linking to the `ref` glossary entry, matching this commit graph
entry.
The commit-graph file is also distinguished by its hyphenation.
Subsequent commit catches the few cases where the hyphenation of
commit-graph was missing.
Signed-off-by: Philip Oakley <philipoakley@iee.email> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Philip Oakley [Sat, 29 Oct 2022 16:41:09 +0000 (17:41 +0100)]
doc: use 'object database' not ODB or abbreviation
The abbreviation 'ODB' is used in the technical documentation
sections for commit-graph and parallel-checkout, along with an
'odb' option in `git-pack-redundant`, without expansion.
Use 'object database' in full, in those entries. The text has not
been reflowed to keep the changes minimal.
While in the glossary for `object` terms, add the common`oid`
abbreviation to its entry.
Signed-off-by: Philip Oakley <philipoakley@iee.email> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sat, 29 Oct 2022 10:06:06 +0000 (12:06 +0200)]
archive-tar: report filter start error only once
A missing tar filter is reported by start_command() using error(), but
also by its caller, write_tar_filter_archive(), using die():
$ git -c tar.invalid.command=foo archive --format=invalid HEAD
error: cannot run foo: No such file or directory
fatal: unable to start 'foo' filter: No such file or directory
The second message contains all relevant information and even says that
the failed command was intended to be used as a filter. Silence the
first one because it's redundant.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:55:06 +0000 (12:55 +0100)]
replace and remove run_command_v_opt()
Replace the remaining calls of run_command_v_opt() with run_command()
calls and explict struct child_process variables. This is more verbose,
but not by much overall. The code becomes more flexible, e.g. it's easy
to extend to conditionally add a new argument.
Then remove the now unused function and its own flag names, simplifying
the run-command API.
Suggested-by: Jeff King <peff@peff.net> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:52:40 +0000 (12:52 +0100)]
replace and remove run_command_v_opt_tr2()
The convenience function run_command_v_opt_tr2() is only used by a
single caller. Use struct child_process and run_command() directly
instead and remove the underused function.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:51:55 +0000 (12:51 +0100)]
replace and remove run_command_v_opt_cd_env()
run_command_v_opt_cd_env() is only used in an example in a comment. Use
the struct child_process member "env" and run_command() directly instead
and then remove the unused convenience function.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:51:14 +0000 (12:51 +0100)]
use child_process members "args" and "env" directly
Build argument list and environment of child processes by using
struct child_process and populating its members "args" and "env"
directly instead of maintaining separate strvecs and letting
run_command_v_opt() and friends populate these members. This is
simpler, shorter and slightly more efficient.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:50:27 +0000 (12:50 +0100)]
use child_process member "args" instead of string array variable
Use run_command() with a struct child_process variable and populate its
"args" member directly instead of building a string array and passing it
to run_command_v_opt(). This avoids the use of magic index numbers and
makes simplifies the possible addition of more arguments in the future.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:49:37 +0000 (12:49 +0100)]
sequencer: simplify building argument list in do_exec()
Build child_argv during initialization, taking advantage of the C99
support for initialization expressions that are not compile time
constants. This avoids the use of a magic index constant and is shorter
and simpler.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:48:16 +0000 (12:48 +0100)]
bisect--helper: factor out do_bisect_run()
Deduplicate the code for reporting and starting the bisect run command
by moving it to a short helper function. Use a string array instead of
a strvec to prepare the arguments, for simplicity.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:47:02 +0000 (12:47 +0100)]
bisect: simplify building "checkout" argument list
Reduce the scope of argv_checkout, which allows to fully build it during
initialization. Use oid_to_hex() instead of oid_to_hex_r(), because
that's simpler and using the static buffer of the former is just as safe
as the old static argv_checkout.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:46:11 +0000 (12:46 +0100)]
am: simplify building "show" argument list
Build the string array av during initialization, without any magic
numbers or heap allocations. Not duplicating the result of oid_to_hex()
is safe because run_command_v_opt() duplicates all arguments already.
(It would even be safe if it didn't, but that's a different story.)
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
René Scharfe [Sun, 30 Oct 2022 11:45:13 +0000 (12:45 +0100)]
run-command: fix return value comment
483bbd4e4c (run-command: introduce child_process_init(), 2014-08-19) and 2d71608ec0 (run-command: factor out child_process_clear(), 2015-10-24)
added help texts about child_process_init() and child_process_clear()
without updating the immediately following documentation of return codes
that only applied to the preexisting functions.
4c4066d95d (run-command: move doc to run-command.h, 2019-11-17) started
to list the functions explicitly that this paragraph applies to, but
still wrongly included child_process_init() and child_process_clear().
Remove their names from that list.
Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Simplify the code that builds the arguments for the "read-tree"
invocation in reset_hard() and read_empty() to remove the "verbose"
parameter.
Before 172b6428d06 (do not overwrite untracked during merge from
unborn branch, 2010-11-14) there was a "reset_hard()" function that
would be called in two places, one of those passed a "verbose=1", the
other a "verbose=0".
After 172b6428d06 when read_empty() was split off from reset_hard()
both of these functions only had one caller. The "verbose" in
read_empty() would always be false, and the one in reset_hard() would
always be true.
There was never a good reason for the code to act this way, it
happened because the read_empty() function was a copy/pasted and
adjusted version of reset_hard().
Since we're no longer conditionally adding the "-v" parameter
here (and we'd only add it for "reset_hard()" we'll be able to move to
a simpler and safer run-command API in the subsequent commit.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
Junio C Hamano [Fri, 28 Oct 2022 21:16:09 +0000 (14:16 -0700)]
adjust_shared_perm(): leave g+s alone when the group does not matter
Julien Moutinho reports that in an environment where directory does
not have BSD group semantics and requires the g+s to be set (aka
FORCE_DIR_SET_GID), but the system forbids chmod() to touch the g+s
bit, adjust_shared_perm() fails even when the repository is for
private use with perm = 0600, because we unconditionally try to set
the g+s bit.
When we grant extra access based on group membership (i.e. the
directory has either g+r or g+w bit set), which group the directory
and its contents are owned by matters. But otherwise (e.g. perm is
set to 0600, in Julien's case), flipping g+s bit is not necessary.
Reported-by: Julien Moutinho <julm+git@sourcephile.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 28 Oct 2022 18:26:55 +0000 (11:26 -0700)]
Merge branch 'tb/diffstat-with-utf8-strwidth'
"git diff --stat" etc. were invented back when everything was ASCII
and strlen() was a way to measure the display width of a string;
adjust them to compute the display width assuming UTF-8 pathnames.
* tb/diffstat-with-utf8-strwidth:
diff: leave NEEDWORK notes in show_stats() function
diff.c: use utf8_strwidth() to count display width
Junio C Hamano [Thu, 27 Oct 2022 22:24:11 +0000 (15:24 -0700)]
Merge branch 'tb/midx-repack-ignore-cruft-packs' into maint-2.38
"git multi-pack-index repack/expire" used to repack unreachable
cruft into a new pack, which have been corrected.
cf. <63a1c3d4-eff3-af10-4263-058c88e74594@github.com>
* tb/midx-repack-ignore-cruft-packs:
midx.c: avoid cruft packs with non-zero `repack --batch-size`
midx.c: remove unnecessary loop condition
midx.c: replace `xcalloc()` with `CALLOC_ARRAY()`
midx.c: avoid cruft packs with `repack --batch-size=0`
midx.c: prevent `expire` from removing the cruft pack
Documentation/git-multi-pack-index.txt: clarify expire behavior
Documentation/git-multi-pack-index.txt: fix typo
Junio C Hamano [Thu, 27 Oct 2022 22:24:09 +0000 (15:24 -0700)]
Merge branch 'jc/environ-docs' into maint-2.38
Documentation on various Boolean GIT_* environment variables have
been clarified.
* jc/environ-docs:
environ: GIT_INDEX_VERSION affects not just a new repository
environ: simplify description of GIT_INDEX_FILE
environ: GIT_FLUSH should be made a usual Boolean
environ: explain Boolean environment variables
environ: document GIT_SSL_NO_VERIFY
Jeff King [Tue, 18 Oct 2022 20:15:33 +0000 (16:15 -0400)]
Makefile: force -O0 when compiling with SANITIZE=leak
Cherry pick commit d3775de0 (Makefile: force -O0 when compiling with
SANITIZE=leak, 2022-10-18), as otherwise the leak checker at GitHub
Actions CI seems to fail with a false positive.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 27 Oct 2022 21:51:53 +0000 (14:51 -0700)]
Merge branch 'js/cmake-updates'
Update to build procedure with VS using CMake/CTest.
* js/cmake-updates:
cmake: increase time-out for a long-running test
cmake: avoid editing t/test-lib.sh
add -p: avoid ambiguous signed/unsigned comparison
cmake: copy the merge tools for testing
cmake: make it easier to diagnose regressions in CTest runs
Junio C Hamano [Thu, 27 Oct 2022 21:51:53 +0000 (14:51 -0700)]
Merge branch 'ab/run-hook-api-cleanup'
Move a global variable added as a hack during regression fixes to
its proper place in the API.
* ab/run-hook-api-cleanup:
run-command.c: remove "max_processes", add "const" to signal() handler
run-command.c: pass "opts" further down, and use "opts->processes"
run-command.c: use "opts->processes", not "pp->max_processes"
run-command.c: don't copy "data" to "struct parallel_processes"
run-command.c: don't copy "ungroup" to "struct parallel_processes"
run-command.c: don't copy *_fn to "struct parallel_processes"
run-command.c: make "struct parallel_processes" const if possible
run-command API: move *_tr2() users to "run_processes_parallel()"
run-command API: have run_process_parallel() take an "opts" struct
run-command.c: use designated init for pp_init(), add "const"
run-command API: don't fall back on online_cpus()
run-command API: make "n" parameter a "size_t"
run-command tests: use "return", not "exit"
run-command API: have "run_processes_parallel{,_tr2}()" return void
run-command test helper: use "else if" pattern
Junio C Hamano [Thu, 27 Oct 2022 21:51:52 +0000 (14:51 -0700)]
Merge branch 'jk/unused-anno-more'
More UNUSED annotation to help using -Wunused option with the
compiler.
* jk/unused-anno-more:
ll-merge: mark unused parameters in callbacks
diffcore-pickaxe: mark unused parameters in pickaxe functions
convert: mark unused parameter in null stream filter
apply: mark unused parameters in noop error/warning routine
apply: mark unused parameters in handlers
date: mark unused parameters in handler functions
string-list: mark unused callback parameters
object-file: mark unused parameters in hash_unknown functions
mark unused parameters in trivial compat functions
update-index: drop unused argc from do_reupdate()
submodule--helper: drop unused argc from module_list_compute()
diffstat_consume(): assert non-zero length
Junio C Hamano [Thu, 27 Oct 2022 21:51:52 +0000 (14:51 -0700)]
Merge branch 'tb/midx-bitmap-selection-fix'
A bugfix with tracing support in midx codepath
* tb/midx-bitmap-selection-fix:
pack-bitmap-write.c: instrument number of reused bitmaps
midx.c: instrument MIDX and bitmap generation with trace2 regions
midx.c: consider annotated tags during bitmap selection
midx.c: fix whitespace typo
Emily Shaffer [Wed, 26 Oct 2022 21:32:45 +0000 (17:32 -0400)]
config: let feature.experimental imply gc.cruftPacks=true
We are interested in exploring whether gc.cruftPacks=true should become
the default value.
To determine whether it is safe to do so, let's encourage more users to
try it out.
Users who have set feature.experimental=true have already volunteered to
try new and possibly-breaking config changes, so let's try this new
default with that set of users.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Emily Shaffer [Wed, 26 Oct 2022 21:32:43 +0000 (17:32 -0400)]
gc: add tests for --cruft and friends
In 5b92477f89 (builtin/gc.c: conditionally avoid pruning objects via
loose, 2022-05-20) gc learned to respect '--cruft' and 'gc.cruftPacks'.
'--cruft' is exercised in t5329-pack-objects-cruft.sh, but in a way that
doesn't check whether a lone gc run generates these cruft packs.
'gc.cruftPacks' is never exercised.
Add some tests to exercise these options to gc in the gc test suite.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The Meta/redo-jch.sh script is generated a few lines earlier by running:
$ Meta/Reintegrate master..seen >Meta/redo-jch.sh
But the resulting script is not necessarily executable. Later mentions
of this script invoke it with sh (instead of directly), but this one is
an odd one out.
Update the documentation to invoke the Meta/redo-jch.sh script with sh
in case the maintainer has not made the script executable.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rubén Justo [Tue, 25 Oct 2022 23:01:29 +0000 (01:01 +0200)]
branch: error copying or renaming a detached HEAD
In c847f53712 (Detached HEAD (experimental), 2007-01-01) an error
condition was introduced in rename_branch() to prevent renaming, later
also copying, a detached HEAD.
The condition used was checking for NULL in oldname, the source branch
to rename/copy. That condition cannot be satisfied because if no source
branch is specified, HEAD is going to be used in the call.
The error issued instead is:
fatal: Invalid branch name: 'HEAD'
Let's remove the condition in copy_or_rename_branch() (the current
function name) and check for HEAD before calling it, dying with the
original intended error if we're in a detached HEAD.
Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan [Tue, 25 Oct 2022 23:29:34 +0000 (16:29 -0700)]
negotiator/skipping: avoid stack overflow
mark_common() in negotiator/skipping.c may overflow the stack due to
recursive function calls. Avoid this by instead recursing using a
heap-allocated data structure.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 26 Oct 2022 00:11:43 +0000 (17:11 -0700)]
Merge branch 'rs/diff-caret-bang-with-parents'
"git diff rev^!" did not show combined diff to go to the rev from
its parents.
* rs/diff-caret-bang-with-parents:
diff: support ^! for merges
revisions.txt: unspecify order of resolved parts of ^!
revision: use strtol_i() for exclude_parent
Junio C Hamano [Wed, 26 Oct 2022 00:11:39 +0000 (17:11 -0700)]
Merge branch 'jk/cleanup-callback-parameters' into maint-2.38
Code clean-up.
* jk/cleanup-callback-parameters:
attr: drop DEBUG_ATTR code
commit: avoid writing to global in option callback
multi-pack-index: avoid writing to global in option callback
test-submodule: inline resolve_relative_url() function
Junio C Hamano [Wed, 26 Oct 2022 00:11:34 +0000 (17:11 -0700)]
Merge branch 'js/merge-ort-in-read-only-repo' into maint-2.38
In read-only repositories, "git merge-tree" tried to come up with a
merge result tree object, which it failed (which is not wrong) and
led to a segfault (which is bad), which has been corrected.
* js/merge-ort-in-read-only-repo:
merge-ort: return early when failing to write a blob
merge-ort: fix segmentation fault in read-only repositories
Junio C Hamano [Wed, 26 Oct 2022 00:11:32 +0000 (17:11 -0700)]
Merge branch 'ab/coding-guidelines-c99' into maint-2.38
Update CodingGuidelines to clarify what features to use and avoid
in C99.
* ab/coding-guidelines-c99:
CodingGuidelines: recommend against unportable C99 struct syntax
CodingGuidelines: mention C99 features we can't use
CodingGuidelines: allow declaring variables in for loops
CodingGuidelines: mention dynamic C99 initializer elements
CodingGuidelines: update for C99
Junio C Hamano [Tue, 25 Oct 2022 22:42:24 +0000 (15:42 -0700)]
Documentation: add lint-fsck-msgids
During the initial development of the fsck-msgids.txt feature, it
has become apparent that it is very much error prone to make sure
the description in the documentation file are sorted and correctly
match what is in the fsck.h header file.
Add a quick-and-dirty Perl script and doc-lint target to sanity
check that the fsck-msgids.txt is consistent with the error type
list in the fsck.h header file.
John Cai [Tue, 25 Oct 2022 22:42:23 +0000 (15:42 -0700)]
fsck: document msg-id
The documentation lacks mention of specific <msg-id> that are supported.
While git-help --config will display a list of these options, often
developers' first instinct is to consult the git docs to find valid
config values.
Add a list of fsck error messages, and link to it from the git-fsck
documentation.
Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
John Cai [Tue, 25 Oct 2022 22:42:21 +0000 (15:42 -0700)]
fsck: remove the unused BAD_TAG_OBJECT
2175a0c6 (fsck: stop checking tag->tagged, 2019-10-18) stopped
checking the tagged object referred to by a tag object, which is what the
error message BAD_TAG_OBJECT was for. Since then the BAD_TAG_OBJECT
message is no longer used anywhere.
Remove the BAD_TAG_OBJECT msg-id.
Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 25 Oct 2022 18:24:31 +0000 (14:24 -0400)]
apply: reject patches larger than ~1 GiB
The apply code is not prepared to handle extremely large files. It uses
"int" in some places, and "unsigned long" in others.
This combination leads to unfortunate problems when switching between
the two types. Using "int" prevents us from handling large files, since
large offsets will wrap around and spill into small negative values,
which can result in wrong behavior (like accessing the patch buffer with
a negative offset).
Converting from "unsigned long" to "int" also has truncation problems
even on LLP64 platforms where "long" is the same size as "int", since
the former is unsigned but the latter is not.
To avoid potential overflow and truncation issues in `git apply`, apply
similar treatment as in dcd1742e56 (xdiff: reject files larger than
~1GB, 2015-09-24), where the xdiff code was taught to reject large
files for similar reasons.
The maximum size was chosen somewhat arbitrarily, but picking a value
just shy of a gigabyte allows us to double it without overflowing 2^31-1
(after which point our value would wrap around to a negative number).
To give ourselves a bit of extra margin, the maximum patch size is a MiB
smaller than a full GiB, which gives us some slop in case we allocate
"(records + 1) * sizeof(int)" or similar.
Luckily, the security implications of these conversion issues are
relatively uninteresting, because a victim needs to be convinced to
apply a malicious patch.
Reported-by: 정재우 <thebound7@gmail.com> Suggested-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Julia Ramer [Mon, 24 Oct 2022 22:07:19 +0000 (22:07 +0000)]
embargoed releases: also describe the git-security list and the process
With the recent turnover on the git-security list, questions came up how
things are usually run. Rather than answering questions individually,
extend Git's existing documentation about security vulnerabilities to
describe the git-security mailing list, how things are run on that list,
and what to expect throughout the process from the time a security bug
is reported all the way to the time when a fix is released.
Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Julia Ramer <gitprplr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jerry Zhang [Mon, 24 Oct 2022 20:07:44 +0000 (20:07 +0000)]
builtin: patch-id: remove unused diff-tree prefix
The last git version that had "diff-tree" in the header text
of "git diff-tree" output was v1.3.0 from 2006. The header text
was changed from "diff-tree" to "commit" in 91539833
("Log message printout cleanups").
Given how long ago this change was made, it is highly unlikely that
anyone is still feeding in outputs from that git version.
Remove the handling of the "diff-tree" prefix and document the
source of the other prefixes so that the overall functionality
is more clear.
Signed-off-by: Jerry Zhang <Jerry@skydio.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jerry Zhang [Mon, 24 Oct 2022 20:07:43 +0000 (20:07 +0000)]
builtin: patch-id: add --verbatim as a command mode
There are situations where the user might not want the default
setting where patch-id strips all whitespace. They might be working
in a language where white space is syntactically important, or they
might have CI testing that enforces strict whitespace linting. In
these cases, a whitespace change would result in the patch
fundamentally changing, and thus deserving of a different id.
Add a new mode that is exclusive of --stable and --unstable called
--verbatim. It also corresponds to the config
patchid.verbatim = true. In this mode, the stable algorithm is
used and whitespace is not stripped from the patch text.
Users of --unstable mainly care about compatibility with old git
versions, which unstripping the whitespace would break. Thus there
isn't a usecase for the combination of --verbatim and --unstable,
and we don't expose this so as to not add maintainence burden.
Signed-off-by: Jerry Zhang <jerry@skydio.com>
fixes https://github.com/Skydio/revup/issues/2 Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jerry Zhang [Mon, 24 Oct 2022 20:07:42 +0000 (20:07 +0000)]
patch-id: fix patch-id for mode changes
Currently patch-id as used in rebase and cherry-pick does not account
for file modes if the file is modified. One consequence of this is
that if you have a local patch that changes modes, but upstream
has applied an outdated version of the patch that doesn't include
that mode change, "git rebase" will drop your local version of the
patch along with your mode changes. It also means that internal
patch-id doesn't produce the same output as the builtin, which does
account for mode changes due to them being part of diff output.
Fix by adding mode to the patch-id if it has changed, in the same
format that would be produced by diff, so that it is compatible
with builtin patch-id.
Signed-off-by: Jerry Zhang <Jerry@skydio.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jerry Zhang [Mon, 24 Oct 2022 20:07:41 +0000 (20:07 +0000)]
builtin: patch-id: fix patch-id with binary diffs
"git patch-id" currently doesn't produce correct output if the
incoming diff has any binary files. Add logic to get_one_patchid
to handle the different possible styles of binary diff. This
attempts to keep resulting patch-ids identical to what would be
produced by the counterpart logic in diff.c, that is it produces
the id by hashing the a and b oids in succession.
In general we handle binary diffs by first caching the object ids from
the "index" line and using those if we then find an indication
that the diff is binary.
The input could contain patches generated with "git diff --binary". This
currently breaks the parse logic and results in multiple patch-ids
output for a single commit. Here we have to skip the contents of the
patch itself since those do not go into the patch id. --binary
implies --full-index so the object ids are always available.
When the diff is generated with --full-index there is no patch content
to skip over.
When a diff is generated without --full-index or --binary, it will
contain abbreviated object ids. This will still result in a sufficiently
unique patch-id when hashed, but does not match internal patch id
output. We'll call this ok for now as we already need specialized
arguments to diff in order to match internal patch id (namely -U3).
Signed-off-by: Jerry Zhang <Jerry@skydio.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jerry Zhang [Mon, 24 Oct 2022 20:07:40 +0000 (20:07 +0000)]
patch-id: use stable patch-id for rebases
Git doesn't persist patch-ids during the rebase process, so there is
no need to specifically invoke the unstable variant. Use the stable
logic for all internal patch-id calculations to minimize the number of
code paths and improve test coverage.
Signed-off-by: Jerry Zhang <jerry@skydio.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jerry Zhang [Mon, 24 Oct 2022 20:07:39 +0000 (20:07 +0000)]
patch-id: fix stable patch id for binary / header-only
Patch-ids for binary patches are found by hashing the object
ids of the before and after objects in succession. However in
the --stable case, there is a bug where hunks are not flushed
for binary and header-only patch ids, which would always result
in a patch-id of 0000. The --unstable case is currently correct.
Reorder the logic to branch into 3 cases for populating the
patch body: header-only which populates nothing, binary which
populates the object ids, and normal which populates the text
diff. All branches will end up flushing the hunk.
Don't populate the ---a/ and +++b/ lines for binary diffs, to correspond
to those lines not being present in the "git diff" text output.
This is necessary because we advertise that the patch-id calculated
internally and used in format-patch is the same that what the
builtin "git patch-id" would produce when piped from a diff.
Update the test to run on both binary and normal files.
Signed-off-by: Jerry Zhang <jerry@skydio.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 24 Oct 2022 18:55:44 +0000 (14:55 -0400)]
shortlog: implement `--group=author` in terms of `--group=<format>`
Instead of handling SHORTLOG_GROUP_AUTHOR separately, reimplement it as
a special case of the new `--group=<format>` mode, where the author mode
is a shorthand for `--group='%aN <%aE>'.
Note that we still need to keep the SHORTLOG_GROUP_AUTHOR enum since it
has a different meaning in `read_from_stdin()`, where it is still used
for a different purpose.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 24 Oct 2022 18:55:41 +0000 (14:55 -0400)]
shortlog: extract `shortlog_finish_setup()`
Extract a function which finishes setting up the shortlog struct for
use. The caller in `make_cover_letter()` does not care about trailer
sorting, so it isn't strictly necessary to add a call there in this
patch.
But the next patch will add additional functionality to the new
`shortlog_finish_setup()` function, which the caller in
`make_cover_letter()` will care about.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 24 Oct 2022 18:55:39 +0000 (14:55 -0400)]
shortlog: support arbitrary commit format `--group`s
In addition to generating a shortlog based on committer, author, or the
identity in one or more specified trailers, it can be useful to generate
a shortlog based on an arbitrary commit format.
This can be used, for example, to generate a distribution of commit
activity over time, like so:
Arbitrary commit formats can be used. In fact, `git shortlog`'s default
behavior (to count by commit authors) can be emulated as follows:
$ git shortlog --group='%aN <%aE>' ...
and future patches will make the default behavior (as well as
`--committer`, and `--group=trailer:<trailer>`) special cases of the
more flexible `--group` option.
Note also that the SHORTLOG_GROUP_FORMAT enum value is used only to
designate that `--group:<format>` is in use when in stdin mode to
declare that the combination is invalid.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Mon, 24 Oct 2022 18:55:33 +0000 (14:55 -0400)]
shortlog: make trailer insertion a noop when appropriate
When there are no trailers to insert, it is natural that
insert_records_from_trailers() should return without having done any
work.
But instead we guard this call unnecessarily by first checking whether
`log->groups` has the `SHORTLOG_GROUP_TRAILER` bit set.
Prepare to match a similar pattern in the future where a function which
inserts records of a certain type does no work when no specifiers
matching that type are given.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>