Commit graph

10712 commits

Author SHA1 Message Date
Taylor Blau 8851c4b065 Merge branch 'pw/rebase-reflog-fixes'
Fix some bugs in the reflog messages when rebasing and changes the
reflog messages of "rebase --apply" to match "rebase --merge" with
the aim of making the reflog easier to parse.

* pw/rebase-reflog-fixes:
  rebase: cleanup action handling
  rebase --abort: improve reflog message
  rebase --apply: make reflog messages match rebase --merge
  rebase --apply: respect GIT_REFLOG_ACTION
  rebase --merge: fix reflog message after skipping
  rebase --merge: fix reflog when continuing
  t3406: rework rebase reflog tests
  rebase --apply: remove duplicated code
2022-10-30 21:04:43 -04:00
Taylor Blau 003f815dd9 Merge branch 'pw/rebase-keep-base-fixes'
"git rebase --keep-base" used to discard the commits that are
already cherry-picked to the upstream, even when "keep-base" meant
that the base, on top of which the history is being rebuilt, does
not yet include these cherry-picked commits.  The --keep-base
option now implies --reapply-cherry-picks and --no-fork-point
options.

* pw/rebase-keep-base-fixes:
  rebase --keep-base: imply --no-fork-point
  rebase --keep-base: imply --reapply-cherry-picks
  rebase: factor out branch_base calculation
  rebase: rename merge_base to branch_base
  rebase: store orig_head as a commit
  rebase: be stricter when reading state files containing oids
  t3416: set $EDITOR in subshell
  t3416: tighten two tests
2022-10-30 21:04:42 -04:00
Taylor Blau c112d8d9c2 Merge branch 'tb/shortlog-group'
"git shortlog" learned to group by the "format" string.

* tb/shortlog-group:
  shortlog: implement `--group=committer` in terms of `--group=<format>`
  shortlog: implement `--group=author` in terms of `--group=<format>`
  shortlog: extract `shortlog_finish_setup()`
  shortlog: support arbitrary commit format `--group`s
  shortlog: extract `--group` fragment for translation
  shortlog: make trailer insertion a noop when appropriate
  shortlog: accept `--date`-related options
2022-10-30 21:04:42 -04:00
Taylor Blau c88895e67b Merge branch 'jk/repack-tempfile-cleanup'
The way "git repack" creared temporary files when it received a
signal was prone to deadlocking, which has been corrected.

* jk/repack-tempfile-cleanup:
  t7700: annotate cruft-pack failure with ok=sigpipe
  repack: drop remove_temporary_files()
  repack: use tempfiles for signal cleanup
  repack: expand error message for missing pack files
  repack: populate extension bits incrementally
  repack: convert "names" util bitfield to array
2022-10-30 21:04:42 -04:00
Taylor Blau 160314e625 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
2022-10-30 21:04:41 -04:00
Junio C Hamano c5dd7773e1 Merge branch 'tb/remove-unused-pack-bitmap'
When creating a multi-pack bitmap, remove per-pack bitmap files
unconditionally as they will never be consulted.

* tb/remove-unused-pack-bitmap:
  builtin/repack.c: remove redundant pack-based bitmaps
2022-10-28 11:26:54 -07:00
Junio C Hamano 7b9b634ca5 Merge branch 'ab/doc-synopsis-and-cmd-usage'
The short-help text shown by "git cmd -h" and the synopsis text
shown at the beginning of "git help cmd" have been made more
consistent.

* ab/doc-synopsis-and-cmd-usage: (34 commits)
  tests: assert consistent whitespace in -h output
  tests: start asserting that *.txt SYNOPSIS matches -h output
  doc txt & -h consistency: make "worktree" consistent
  worktree: define subcommand -h in terms of command -h
  reflog doc: list real subcommands up-front
  doc txt & -h consistency: make "commit" consistent
  doc txt & -h consistency: make "diff-tree" consistent
  doc txt & -h consistency: use "[<label>...]" for "zero or more"
  doc txt & -h consistency: make "annotate" consistent
  doc txt & -h consistency: make "stash" consistent
  doc txt & -h consistency: add missing options
  doc txt & -h consistency: use "git foo" form, not "git-foo"
  doc txt & -h consistency: make "bundle" consistent
  doc txt & -h consistency: make "read-tree" consistent
  doc txt & -h consistency: make "rerere" consistent
  doc txt & -h consistency: add missing options and labels
  doc txt & -h consistency: make output order consistent
  doc txt & -h consistency: add or fix optional "--" syntax
  doc txt & -h consistency: fix mismatching labels
  doc SYNOPSIS & -h: use "-" to separate words in labels, not "_"
  ...
2022-10-28 11:26:54 -07:00
Junio C Hamano 6ae1a6eaf2 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
2022-10-27 14:51:53 -07:00
Junio C Hamano f62c546455 Merge branch 'tb/save-keep-pack-during-geometric-repack'
When geometric repacking feature is in use together with the
--pack-kept-objects option, we lost packs marked with .keep files.

* tb/save-keep-pack-during-geometric-repack:
  repack: don't remove .keep packs with `--pack-kept-objects`
2022-10-27 14:51:53 -07:00
Junio C Hamano 220604042c 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
2022-10-27 14:51:52 -07:00
Junio C Hamano b988427918 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
2022-10-25 17:11:43 -07:00
Jerry Zhang 0d32ae8d7f 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>
2022-10-24 15:44:20 -07:00
Jerry Zhang 2871f4d447 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>
2022-10-24 15:44:20 -07:00
Jerry Zhang 0df19eb9d9 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>
2022-10-24 15:44:19 -07:00
Jerry Zhang 51276c1832 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>
2022-10-24 15:44:19 -07:00
Taylor Blau 7b11234e3b shortlog: implement --group=committer in terms of --group=<format>
In the same spirit as the previous commit, reimplement
`--group=committer` as a special case of `--group=<format>`, too.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-24 14:48:05 -07:00
Taylor Blau 9c10d4ff24 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>
2022-10-24 14:48:05 -07:00
Taylor Blau 10538e2a62 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>
2022-10-24 14:48:05 -07:00
Taylor Blau 3dc95e09e1 shortlog: support arbitrary commit format --groups
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:

    $ git shortlog --group='%cd' --date='format:%Y-%m' -s v2.37.0..
       117  2022-06
       274  2022-07
       324  2022-08
       263  2022-09
         7  2022-10

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>
2022-10-24 14:48:05 -07:00
Taylor Blau b017d3dae9 shortlog: extract --group fragment for translation
The subsequent commit will add another unhandled case in
`read_from_stdin()` which will want to use the same message as with
`--group=trailer`.

Extract the "--group=trailer" part from this message so the same
translation key can be used for both cases.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-24 14:48:05 -07:00
Taylor Blau 0b293df964 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>
2022-10-24 14:48:05 -07:00
Jeff King 251554c269 shortlog: accept --date-related options
Prepare for a future patch which will introduce arbitrary pretty formats
via the `--group` argument.

To allow additional customizability (for example, to support something
like `git shortlog -s --group='%aD' --date='format:%Y-%m' ...` (which
groups commits by the datestring 'YYYY-mm' according to author date), we
must store off the `--date` parsed from calling `parse_revision_opt()`.

Note that this also affects custom output `--format` strings in `git
shortlog`. Though this is a behavior change, this is arguably fixing a
long-standing bug (ie., that `--format` strings are not affected by
`--date` specifiers as they should be).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-24 14:48:05 -07:00
Jeff King 193430717a repack: drop remove_temporary_files()
After we've successfully finished the repack, we call
remove_temporary_files(), which looks for and removes any files matching
".tmp-$$-pack-*", where $$ is the pid of the current process. But this
is pointless. If we make it this far in the process, we've already
renamed these tempfiles into place, and there is nothing left to delete.

Nor is there a point in trying to call it to clean up when we _aren't_
successful. It's not safe for using in a signal handler, and the
previous commit already handed that job over to the tempfile API.

It might seem like it would be useful to clean up stray .tmp files left
by other invocations of git-repack. But it won't clean those files; it
only matches ones with its pid, and leaves the rest. Fortunately, those
are cleaned up naturally by successive calls to git-repack; we'll
consider .tmp-*.pack the same as normal packfiles, so "repack -ad", etc,
will roll up their contents and eventually delete them.

The one case that could matter is if pack-objects generates an extension
we don't know about, like ".tmp-pack-$$-$hash.some-new-ext". The current
code will quietly delete such a file, while after this patch we'd leave
it in place. In practice this doesn't happen, and would be indicative of
a bug. Leaving the file as cruft is arguably a better behavior, as it
means somebody is more likely to eventually notice and fix the bug.  If
we really wanted to be paranoid, we could scan for and warn about such
files, but that seems like overkill.

There's nothing to test with regard to the removal of this function. It
was doing nothing, so the behavior should be the same.  However, we can
verify (and protect) our assumption that "repack -ad" will eventually
remove stray files by adding a test for that.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-21 18:03:52 -07:00
Jeff King 9cf10d8786 repack: use tempfiles for signal cleanup
When git-repack exits due to a signal, it tries to clean up by calling
its remove_temporary_files() function, which walks through the packs dir
looking for ".tmp-$$-pack-*" files to delete (where "$$" is the pid of
the current process).

The biggest problem here is that remove_temporary_files() is not safe to
call in a signal handler. It uses opendir(), which isn't on the POSIX
async-signal-safe list. The details will be platform-specific, but a
likely issue is that it needs to allocate memory; if we receive a signal
while inside malloc(), etc, we'll conflict on the allocator lock and
deadlock with ourselves.

We can fix this by just cleaning up the files directly, without walking
the directory. We already know the complete list of .tmp-* files that
were generated, because we recorded them via populate_pack_exts(). When
we find files there, we can use register_tempfile() to record the
filenames. If we receive a signal, then the tempfile API will clean them
up for us, and it's async-safe and pretty battle-tested.

Note that this is slightly racier than the existing scheme. We don't
record the filenames until pack-objects tells us the hash over stdout.
So during the period between it generating the file and reporting the
hash, we'd fail to clean up. However, that period is very small. During
most of the pack generation process pack-objects is using its own
internal tempfiles. It's only at the very end that it moves them into
the names git-repack expects, and then it immediately reports the name
to us. Given that cleanup like this is best effort (after all, we may
get SIGKILL), this level of race is acceptable.

When we register the tempfiles, we'll record them locally and use the
result to call rename_tempfile(), rather than renaming by hand.  This
isn't strictly necessary, as once we've renamed the files they're gone,
and the tempfile API's cleanup unlink() would simply become a pointless
noop. But managing the lifetimes of the tempfile objects is the cleanest
thing to do, and the tempfile pointers naturally fill the same role as
the old booleans.

This patch also fixes another small problem. We only hook signals, and
don't set up an atexit handler. So if we see an error that causes us to
die(), we'll leave the .tmp-* files in place. But since the tempfile API
handles this for us, this is now fixed for free. The new test covers
this by stimulating a failure of pack-objects when generating a cruft
pack. Before this patch, the .tmp-* file for the main pack would have
been left, but now we correctly clean it up.

Two small subtleties on the implementation:

  - in the renaming loop, we can stop re-constructing fname_old; we only
    use it when we have a tempfile to rename, so we can just ask the
    tempfile for its path (which, barring bugs, should be identical)

  - when renaming fails, our error message mentions fname_old. But since
    a failed rename_tempfile() invalidates the tempfile struct, we'll
    lose access to that string. Instead, let's mention the destination
    filename, which is what most other callers do.

Reported-by: Jan Pokorný <poki@fnusa.cz>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-21 18:03:52 -07:00
Jeff King a4880b20cc repack: expand error message for missing pack files
If pack-objects tells us it generated pack $hash, we expect to find
.tmp-$$-pack-$hash.pack, .idx, .rev, and so on. Some of these files are
optional, but others are not. For the required ones, we'll bail with an
error if any of them is missing.

The error message is just "missing required file", which is a bit vague.
We should be more clear that it is not the user's fault, but rather that
the sub-pgoram we called is not operating as expected. In practice,
nobody should ever see this message, as it would generally only be
caused by a bug in Git.

It probably doesn't make sense to convert this to a BUG(), though, as
there are other (unlikely) possibilities, such as somebody else racily
deleting the files, filesystem errors causing stat() to fail, and so on.

A nice side effect here is that we stop relying on fname_old in this
code path, which will let us deal with it only in the first part of the
conditional.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-21 18:03:52 -07:00
Jeff King b639606fd0 repack: populate extension bits incrementally
After generating the main pack and then any additional cruft packs, we
iterate over the "names" list (which contains hashes of packs generated
by pack-objects), and call populate_pack_exts() for each.

There's one small problem with this. In repack_promisor_objects(), we
may add entries to "names" and call populate_pack_exts() for them.
Calling it again is mostly just wasteful, as we'll stat() the filename
with each possible extension, get the same result, and just overwrite
our bits.

So we could drop the call there, and leave the final loop to populate
all of the bits. But instead, this patch does the reverse: drops the
final loop, and teaches the other two sites to populate the bits as they
add entries.

This makes the code easier to reason about, as you never have to worry
about when the util field is valid; it is always valid for each entry.

It also serves my ulterior purpose: recording the generated filenames as
soon as possible will make it easier for a future patch to use them for
cleaning up from a failed operation.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-21 18:03:52 -07:00
Jeff King d3d9c51973 repack: convert "names" util bitfield to array
We keep a string_list "names" containing the hashes of packs generated
on our behalf by pack-objects. The util field of each item is treated as
a bitfield that tells us which extensions (.pack, .idx, .rev, etc) are
present for each name.

Let's switch this to allocating a real array. That will give us room in
a future patch to store more data than just a single bit per extension.
And it makes the code a little easier to read, as we avoid casting back
and forth between uintptr_t and a void pointer.

Since the only thing we're storing is an array, we could just allocate
it directly. But instead I've put it into a named struct here. That
further increases readability around the casts, and in particular helps
differentiate us from other string_lists in the same file which use
their util field differently. E.g., the existing_*_packs lists still do
bit-twiddling, but their bits have different meaning than the ones in
"names". This makes it hard to grep around the code to see how the util
fields are used; now you can look for "generated_pack_data".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-21 18:03:52 -07:00
Junio C Hamano c2058ea237 Merge branch 'rj/branch-edit-description-with-nth-checkout'
"git branch --edit-description @{-1}" is now a way to edit branch
description of the branch you were on before switching to the
current branch.

* rj/branch-edit-description-with-nth-checkout:
  branch: support for shortcuts like @{-1}, completed
2022-10-21 11:37:29 -07:00
Junio C Hamano 4a48c7d25f Merge branch 'jc/symbolic-ref-no-recurse'
After checking out a "branch" that is a symbolic-ref that points at
another branch, "git symbolic-ref HEAD" reports the underlying
branch, not the symbolic-ref the user gave checkout as argument.
The command learned the "--no-recurse" option to stop after
dereferencing a symbolic-ref only once.

* jc/symbolic-ref-no-recurse:
  symbolic-ref: teach "--[no-]recurse" option
2022-10-21 11:37:28 -07:00
Taylor Blau 197443e80a repack: don't remove .keep packs with --pack-kept-objects
`git repack` supports a `--pack-kept-objects` flag which more or less
translates to whether or not we pass `--honor-pack-keep` down to `git
pack-objects` when assembling a new pack.

This behavior has existed since ee34a2bead (repack: add
`repack.packKeptObjects` config var, 2014-03-03). In that commit, the
documentation was extended to say:

    [...] Note that we still do not delete `.keep` packs after
    `pack-objects` finishes.

Unfortunately, this is not the case when `--pack-kept-objects` is
combined with a `--geometric` repack. When doing a geometric repack, we
include `.keep` packs when enumerating available packs only when
`pack_kept_objects` is set.

So this all works fine when `--no-pack-kept-objects` (or similar) is
given. Kept packs are excluded from the geometric roll-up, so when we go
to delete redundant packs (with `-d`), no `.keep` packs appear "below
the split" in our geometric progression.

But when `--pack-kept-objects` is given, things can go awry. Namely,
when a kept pack is included in the list of packs tracked by the
`pack_geometry` struct *and* part of the pack roll-up, we will delete
the `.keep` pack when we shouldn't.

Note that this *doesn't* result in object corruption, since the `.keep`
pack's objects are still present in the new pack. But the `.keep` pack
itself is removed, which violates our promise from back in ee34a2bead.

But there's more. Because `repack` computes the geometric roll-up
independently from selecting which packs belong in a MIDX (with
`--write-midx`), this can lead to odd behavior. Consider when a `.keep`
pack appears below the geometric split (ie., its objects will be part of
the new pack we generate).

We'll write a MIDX containing the new pack along with the existing
`.keep` pack. But because the `.keep` pack appears below the geometric
split line, we'll (incorrectly) try to remove it. While this doesn't
corrupt the repository, it does cause us to remove the MIDX we just
wrote, since removing that pack would invalidate the new MIDX.

Funny enough, this behavior became far less noticeable after e4d0c11c04
(repack: respect kept objects with '--write-midx -b', 2021-12-20), which
made `pack_kept_objects` be enabled by default only when we were writing
a non-MIDX bitmap.

But e4d0c11c04 didn't resolve this bug, it just made it harder to notice
unless callers explicitly passed `--pack-kept-objects`.

The solution is to avoid trying to remove `.keep` packs during
`--geometric` repacks, even when they appear below the geometric split
line, which is the approach this patch implements.

Co-authored-by: Victoria Dye <vdye@github.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 21:29:23 -07:00
Taylor Blau 55d902cd61 builtin/repack.c: remove redundant pack-based bitmaps
When we write a MIDX bitmap after repacking, it is possible that the
repository would be left in a state with both pack- and multi-pack
reachability bitmaps.

This can occur, for instance, if a pack that was kept (either by having
a .keep file, or during a geometric repack in which it is not rolled up)
has a bitmap file, and the repack wrote a multi-pack index and bitmap.

When loading a reachability bitmap for the repository, the multi-pack
one is always preferred, so the pack-based one is redundant. Let's
remove it unconditionally, even if '-d' isn't passed, since there is no
practical reason to keep both around. The patch below does just that.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 21:26:16 -07:00
Jeff King 1ee3471045 string-list: mark unused callback parameters
String-lists may be used with callbacks for clearing or iteration. These
callbacks need to conform to a particular interface, even though not
every callback needs all of its parameters. Mark the unused ones to make
-Wunused-parameter happy.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 21:24:04 -07:00
Jeff King 827f8305c4 update-index: drop unused argc from do_reupdate()
The parse-options callback for --again soaks up all remaining options by
manipulating the parse_opt_ctx's argc and argv fields. Even though it
has to look at both, the actual parsing happens via the do_reupdate()
helper, which only looks at the argv half (by passing it along to
parse_pathspec). So that helper doesn't need to see argc at all.

Note that the helper does look at "argv + 1" without confirming that
argc is greater than 0. We know this is correct because it is skipping
past the actual "--again" string, which will always be present. However,
to make what's going on more obvious, let's move that "+1" into the
caller, which has the matching "-1" when fixing up the ctx's argc/argv.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 21:24:03 -07:00
Jeff King 70aa1d7576 submodule--helper: drop unused argc from module_list_compute()
The module_list_compute() function takes an argc/argv pair, but never
looks at argc. This is OK, as the NULL terminator in argv is sufficient
for our purposes (we feed it to parse_pathspec(), which takes only the
array, not a count).

Note that one of the callers _looks_ like it would be buggy, but isn't:
we pass 0/NULL for argc/argv from module_foreach(), so finding the
terminating NULL in that argv naively would segfault. However,
parse_pathspec() is smart enough to interpret a bare NULL as an empty
argv.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 21:24:03 -07:00
Junio C Hamano 9c32cfb49c Git 2.38.1
-----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEE4fA2sf7nIh/HeOzvsLXohpav5ssFAmM/rwcACgkQsLXohpav
 5stHpQ/9Eqd0dVwVA6FijqRr6Nsdt8ufGh4OPZUWlNoQeJbp6N1IDGydAxfzNRNC
 fQTqGyL0ZdvLkWZUQ5ACL+157ArJGINE1f+EjOy+MDcyClPfJpk3r4O/qftmowQk
 l3vnAKBqYRn5ta2+fg6a0R6Q3cH5qZsucXwvspEU+TcqMV6QAQYsbINxnO+VNCSV
 tmqeVO8bvNR+zsZ6p8J1EduWpgvh6XsBpr56UxnOim+XEp+nAzPOILJTbYnMx0Am
 HD6WO7Ws3Wp9hj6cKYjcXyNmXT0T4EOhXtIBCKaXxAjXvvX77a9dpUQNI5n91DAi
 HQ/viM4hhrqBfs3jtr6qnDB/c1wcCLH+1QiOlB/2TE9l4zjR25lAtv901uey4yg6
 A8he9nr1eEiPN0k3vrhYE01rUi9I1arAZ9lVF28NF+JMM25F8dZc2YZbc3UHoBMZ
 7ilpydBqXe43ll4/J8XRcMPQeR7++ss0ROqVN/xXnVB0UWvCYhMFleJ1KA7LHjQd
 XaRi9Xsiki9OTXFrr7u8QZ94RinpHPUkuGxODO7Jqo8uL5+9JIdVuNbJbzQDK8s4
 aU6nfSM7clNebrjaTOeiQB8hv0/uZt6QpUQzT4Q7OBOJzO4uLbkDxChIw/sflQWB
 rWRb63/KOtap78DVvMJMw5OQC4hXi7lJIchgZ8hfBKKs83p5Smk=
 =bTdb
 -----END PGP SIGNATURE-----

Sync with v2.38.1
2022-10-17 15:46:09 -07:00
Junio C Hamano 4050354b14 Merge branch 'rj/branch-edit-desc-unborn'
"git branch --edit-description" on an unborh branch misleadingly
said that no such branch exists, which has been corrected.

* rj/branch-edit-desc-unborn:
  branch: description for non-existent branch errors
2022-10-17 14:56:35 -07:00
Junio C Hamano 272be0db8b Merge branch 'jc/branch-description-unset'
"GIT_EDITOR=: git branch --edit-description" resulted in failure,
which has been corrected.

* jc/branch-description-unset:
  branch: do not fail a no-op --edit-desc
2022-10-17 14:56:33 -07:00
Junio C Hamano 86cc5ee3b7 Merge branch 'jk/cleanup-callback-parameters'
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
2022-10-17 14:56:32 -07:00
Junio C Hamano 8646100e05 Merge branch 'rs/bisect-start-leakfix'
Code clean-up that results in plugging a leak.

* rs/bisect-start-leakfix:
  bisect--helper: plug strvec leak
2022-10-17 14:56:32 -07:00
Junio C Hamano 7b8cfe34d9 Merge branch 'ed/fsmonitor-on-networked-macos'
By default, use of fsmonitor on a repository on networked
filesystem is disabled. Add knobs to make it workable on macOS.

* ed/fsmonitor-on-networked-macos:
  fsmonitor: fix leak of warning message
  fsmonitor: add documentation for allowRemote and socketDir options
  fsmonitor: check for compatability before communicating with fsmonitor
  fsmonitor: deal with synthetic firmlinks on macOS
  fsmonitor: avoid socket location check if using hook
  fsmonitor: relocate socket file if .git directory is remote
  fsmonitor: refactor filesystem checks to common interface
2022-10-17 14:56:31 -07:00
Phillip Wood 9a1925b08f rebase: cleanup action handling
Treating the action as a string is a hang over from the scripted
rebase. The last commit removed the only remaining use of the action
that required a string so lets convert the other action users to use
the existing action enum instead. If we ever need the action name as a
string in the future the action_names array exists exactly for that
purpose.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 12:55:03 -07:00
Phillip Wood 6159e7add4 rebase --abort: improve reflog message
When aborting a rebase the reflog message looks like

	rebase (abort): updating HEAD

which is not very informative. Improve the message by mentioning the
branch that we are returning to as we do at the end of a successful
rebase so it looks like.

	rebase (abort): returning to refs/heads/topic

If GIT_REFLOG_ACTION is set in the environment we no longer omit
"(abort)" from the reflog message. We don't omit "(start)" and
"(finish)" when starting and finishing a rebase in that case so we
shouldn't omit "(abort)".

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 12:55:03 -07:00
Phillip Wood be0d29d301 rebase --apply: make reflog messages match rebase --merge
The apply backend creates slightly different reflog messages to the
merge backend when starting or finishing a rebase and when picking
commits. These differences make it harder than it needs to be to parse
the reflog (I have a script that reads the finishing messages from
rebase and it is a pain to have to accommodate two different message
formats). While it is possible to determine the backend used for a
rebase from the reflog messages, the differences are not designed for
that purpose. c2417d3af7 (rebase: drop '-i' from the reflog for
interactive-based rebases, 2020-02-15) removed the clear distinction
between the reflog messages of the two backends without complaint.

As the merge backend is the default it is likely to be the format most
common in existing reflogs. For that reason the apply backend is changed
to format its reflog messages to match the merge backend as closely as
possible. Note that there is still a difference as when committing a
conflict resolution the apply backend will use "(pick)" rather than
"(continue)" because it is not currently possible to change the message
for a single commit.

In addition to c2417d3af7 we also changed the reflog messages in
68aa495b59 (rebase: implement --merge via the interactive machinery,
2018-12-11) and 2ac0d6273f (rebase: change the default backend from "am"
to "merge", 2020-02-15). This commit makes the same change to "git
rebase --apply" that 2ac0d6273f made to "git rebase" without any backend
specific options. As the messages are changed to use an existing format
any scripts that can parse the reflog messages of the default rebase
backend should be unaffected by this change.

There are existing tests for the messages from both backends which are
adjusted to ensure that they do not get out of sync in the future.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 12:55:03 -07:00
Phillip Wood 33f2b61ff9 rebase --apply: respect GIT_REFLOG_ACTION
The reflog messages when finishing a rebase hard code "rebase" rather
than using GIT_REFLOG_ACTION.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 12:55:03 -07:00
Phillip Wood 1f2d5dc4d2 rebase --merge: fix reflog message after skipping
The reflog message for every pick after running "rebase --skip" looks
like

	rebase (skip) (pick): commit subject line

Fix this by not appending " (skip)" to the reflog action.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 12:55:03 -07:00
Phillip Wood da1d63363f rebase --merge: fix reflog when continuing
The reflog message for a conflict resolution committed by "rebase
--continue" looks like

	rebase (continue): commit subject line

Unfortunately the reflog message each subsequent pick look like

	rebase (continue) (pick): commit subject line

Fix this by setting the reflog message for "rebase --continue" in
sequencer_continue() so it does not affect subsequent commits. This
introduces a memory leak similar to the one leaking GIT_REFLOG_ACTION
in pick_commits(). Both of these will be fixed in a future series that
stops the sequencer calling setenv().

If we fail to commit the staged changes then we error out so
GIT_REFLOG_ACTION does not need to be reset in that case.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 12:55:03 -07:00
Phillip Wood 57a1498592 rebase --apply: remove duplicated code
Use move_to_original_branch() when reattaching HEAD after a fast-forward
rather than open coding a copy of that code. move_to_original_branch()
does not call reset_head() if head_name is NULL but there should be no
user visible changes even though we currently call reset_head() in that
case. The reason for this is that the reset_head() call does not add a
message to the reflog because we're not changing the commit that HEAD
points to and so lock_ref_for_update() elides the update. When head_name
is not NULL then reset_head() behaves like "git symbolic-ref" and so the
reflog is updated.

Note that the removal of "strbuf_release(&msg)" is safe as there is an
identical call just above this hunk which can be seen by viewing the
diff with -U6.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 12:55:03 -07:00
Junio C Hamano a524c627a4 Merge branch 'pw/rebase-keep-base-fixes' into pw/rebase-reflog-fixes
* pw/rebase-keep-base-fixes:
  rebase --keep-base: imply --no-fork-point
  rebase --keep-base: imply --reapply-cherry-picks
  rebase: factor out branch_base calculation
  rebase: rename merge_base to branch_base
  rebase: store orig_head as a commit
  rebase: be stricter when reading state files containing oids
  t3416: set $EDITOR in subshell
  t3416: tighten two tests
2022-10-17 12:54:27 -07:00
Phillip Wood aa1df8146d rebase --keep-base: imply --no-fork-point
Given the name of the option it is confusing if --keep-base actually
changes the base of the branch without --fork-point being explicitly
given on the command line.

The combination of --keep-base with an explicit --fork-point is still
supported even though --fork-point means we do not keep the same base
if the upstream branch has been rewound.  We do this in case anyone is
relying on this behavior which is tested in t3431[1]

[1] https://lore.kernel.org/git/20200715032014.GA10818@generichostname/

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 11:53:03 -07:00
Phillip Wood ce5238a690 rebase --keep-base: imply --reapply-cherry-picks
As --keep-base does not rebase the branch it is confusing if it
removes commits that have been cherry-picked to the upstream branch.
As --reapply-cherry-picks is not supported by the "apply" backend this
commit ensures that cherry-picks are reapplied by forcing the upstream
commit to match the onto commit unless --no-reapply-cherry-picks is
given.

Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 11:53:03 -07:00