"git for-each-ref" learns '%(ahead-behind:<base>)' that computes the
distances from a single reference point in the history with bunch
of commits in bulk.
* ds/ahead-behind:
commit-reach: add tips_reachable_from_bases()
for-each-ref: add ahead-behind format atom
commit-reach: implement ahead_behind() logic
commit-graph: introduce `ensure_generations_valid()`
commit-graph: return generation from memory
commit-graph: simplify compute_generation_numbers()
commit-graph: refactor compute_topological_levels()
for-each-ref: explicitly test no matches
for-each-ref: add --stdin option
"git pack-redundant" gave a warning when run, as the command has
outlived its usefulness long ago and is nominated for future
removal. Now we escalate to give an error.
* jk/really-deprecate-pack-redundant:
pack-redundant: escalate deprecation warning to an error
Streamline --rebase-merges command line option handling and
introduce rebase.merges configuration variable.
* ah/rebase-merges-config:
rebase: add a config option for --rebase-merges
rebase: deprecate --rebase-merges=""
rebase: add documentation and test for --no-rebase-merges
Code clean-up.
* jk/fast-export-cleanup:
fast-export: drop unused parameter from anonymize_commit_message()
fast-export: drop data parameter from anonymous generators
fast-export: de-obfuscate --anonymize-map handling
fast-export: factor out anonymized_entry creation
fast-export: simplify initialization of anonymized hashmaps
fast-export: drop const when storing anonymized values
Code clean-up to include and/or uninclude parse-options.h file as
needed.
* sg/parse-options-h-users:
treewide: remove unnecessary inclusions of parse-options.h from headers
treewide: include parse-options.h in source files
Transports that do not support protocol v2 did not correctly fall
back to protocol v0 under certain conditions, which has been
corrected.
* jk/fix-proto-downgrade-to-v0:
git_connect(): fix corner cases in downgrading v2 to v0
The purpose of the new option is to accommodate users who would like
--rebase-merges to be on by default and to facilitate turning on
--rebase-merges by default without configuration in a future version of
Git.
Name the new option rebase.rebaseMerges, even though it is a little
redundant, for consistency with the name of the command line option and
to be clear when scrolling through values in the [rebase] section of
.gitconfig.
Support setting rebase.rebaseMerges to the nonspecific value "true" for
users who don't need to or don't want to learn about the difference
between rebase-cousins and no-rebase-cousins.
Make --rebase-merges without an argument on the command line override
any value of rebase.rebaseMerges in the configuration, for consistency
with other command line flags with optional arguments that have an
associated config option.
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The unusual syntax --rebase-merges="" (that is, --rebase-merges with an
empty string argument) has been an undocumented synonym of
--rebase-merges without an argument. Deprecate that syntax to avoid
confusion when a rebase.rebaseMerges config option is introduced, where
rebase.rebaseMerges="" will be equivalent to --no-rebase-merges.
It is not likely that anyone is actually using this syntax, but just in
case, deprecate the empty string argument instead of dropping support
for it immediately.
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In c3b58472be (pack-redundant: gauge the usage before proposing its
removal, 2020-08-25), we added a big, ugly warning when pack-redundant
is run. The plan there indicated that we would ratchet that up to an
error before finally removing it. Since it has been 2.5 years (and 9
releases) since then, let's continue with the plan.
Note that we did get one bite on the warning, which was somebody asking
about alternatives:
https://lore.kernel.org/git/CAKvOHKAFXQwt4D8yUCCkf_TQL79mYaJ=KAKhtpDNTvHJFuX1NA@mail.gmail.com/
but we didn't undo the ugly warning (and the advice continues to be "use
repack -d" instead).
There was also some discussion around the time of the deprecation that
pack-redundant was invoked by the bitbake tool, and it still seems to do
so now:
https://git.openembedded.org/bitbake
That use should probably just go away in favor of an occasional repack
(which probably even happens via auto-gc after fetch these days).
But since neither of those data points caused us to cancel the
deprecation plan by dropping the warning, it seems like we should
proceed with the next step.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As the comment above the function indicates, we do not bother actually
storing commit messages in our anonymization map. But we still take the
message as a parameter, and just ignore it. Let's stop doing that, which
will make -Wunused-parameter happier.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The anonymization code has a specific generator callback for each type
of data (e.g., one for paths, one for oids, and so on). These all take a
"data" parameter, but none of them use it for anything. Which is not
surprising, as the point is to generate a new name independent of any
input, and each function keeps its own static counter.
We added the extra pointer in d5bf91fde4 (fast-export: add a "data"
callback parameter to anonymize_str(), 2020-06-23) to handle
--anonymize-map parsing, but that turned out to be awkward itself, and
was recently dropped.
So let's get rid of this "data" parameter that nobody is using, both
from the generators and from anonymize_str() which plumbed it through.
This simplifies the code, and makes -Wunused-parameter happier.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we handle an --anonymize-map option, we parse the orig/anon pair,
and then feed the "orig" string to anonymize_str(), along with a
generator function that duplicates the "anon" string to be cached in the
map.
This works, because anonymize_str() says "ah, there is no mapping yet
for orig; I'll add one from the generator". But there are some
downsides:
1. It's a bit too clever, as it's not obvious what the code is trying
to do or why it works.
2. It requires allowing generator functions to take an extra void
pointer, which is not something any of the normal callers of
anonymize_str() want.
3. It does the wrong thing if the same token is provided twice.
When there are conflicting options, like:
git fast-export --anonymize \
--anonymize-map=foo:one \
--anonymize-map=foo:two
we usually let the second one override the first. But by using
anonymize_str(), which has first-one-wins logic, we do the
opposite.
So instead of relying on anonymize_str(), let's directly add the entry
ourselves. We can tweak the tests to show that we handle overridden
options correctly now.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When anonymizing output, there's only one spot where we generate new
entries to add to our hashmap: when anonymize_str() doesn't find an
entry, we use the generate() callback to make one and add it. Let's pull
that into its own function in preparation for another caller.
Note that we'll add one extra feature. In anonymize_str(), we know that
we won't find an existing entry in the hashmap (since it will only try
to add after failing to find one). But other callers won't have the same
behavior, so we should catch this case and free the now-dangling entry.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We take pains to avoid doing a lookup on a hashmap which has not been
initialized with hashmap_init(). That was necessary back when this code
was written. But hashmap_get() became safer in b7879b0ba6 (hashmap:
allow re-use after hashmap_free(), 2020-11-02). Since then it's OK to
call functions on a zero-initialized table; it will just correctly
return NULL, since there is no match.
This simplifies the code a little, and also lets us keep the
initialization line closer to when we add an entry (which is when the
hashmap really does need to be totally initialized). That will help
later refactoring.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We store anonymized values as pointers to "const char *", since they are
conceptually const to callers who use them. But they are actually
allocated strings whose memory is owned by the struct.
The ownership mismatch hasn't been a big deal since we never free() them
(they are held until the program ends), but let's switch them to "char *"
in preparation for changing that.
Since most code only accesses them via anonymize_str(), it can continue
to narrow them to "const char *" in its return value.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git receive-pack" that responds to "git push" requests failed to
clean a stale lockfile when killed in the middle, which has been
corrected.
* ps/receive-pack-unlock-before-die:
receive-pack: fix stale packfile locks when dying
Fix for a "ls-files --format="%(path)" that produced nonsense
output, which was a bug in 2.38.
* aj/ls-files-format-fix:
ls-files: fix "--format" output of relative paths
"git format-patch" honors the src/dst prefixes set to nonstandard
values with configuration variables like "diff.noprefix", causing
receiving end of the patch that expects the standard -p1 format to
break. Teach "format-patch" to ignore end-user configuration and
always use the standard prefixes.
This is a backward compatibility breaking change.
* jk/format-patch-ignore-noprefix:
rebase: prefer --default-prefix to --{src,dst}-prefix for format-patch
format-patch: add format.noprefix option
format-patch: do not respect diff.noprefix
diff: add --default-prefix option
t4013: add tests for diff prefix options
diff: factor out src/dst prefix setup
The previous change implemented the ahead_behind() method, including an
algorithm to compute the ahead/behind values for a number of commit tips
relative to a number of commit bases. Now, integrate that algorithm as
part of 'git for-each-ref' hidden behind a new format atom,
ahead-behind. This naturally extends to 'git branch' and 'git tag'
builtins, as well.
This format allows specifying multiple bases, if so desired, and all
matching references are compared against all of those bases. For this
reason, failing to read a reference provided from these atoms results in
an error.
In order to translate the ahead_behind() method information to the
format output code in ref-filter.c, we must populate arrays of
ahead_behind_count structs. In struct ref_array, we store the full array
that will be passed to ahead_behind(). In struct ref_array_item, we
store an array of pointers that point to the relvant items within the
full array. In this way, we can pull all relevant ahead/behind values
directly when formatting output for a specific item. It also ensures the
lifetime of the ahead_behind_count structs matches the time that the
array is being used.
Add specific tests of the ahead/behind counts in t6600-test-reach.sh, as
it has an interesting repository shape. In particular, its merging
strategy and its use of different commit-graphs would demonstrate over-
counting if the ahead_behind() method did not already account for that
possibility.
Also add tests for the specific for-each-ref, branch, and tag builtins.
In the case of 'git tag', there are intersting cases that happen when
some of the selected tips are not commits. This requires careful logic
around commits_nr in the second loop of filter_ahead_behind(). Also, the
test in t7004 is carefully located to avoid being dependent on the GPG
prereq. It also avoids using the test_commit helper, as that will add
ticks to the time and disrupt the expected timestamps in later tag
tests.
Also add performance tests in a new p1300-graph-walks.sh script. This
will be useful for more uses in the future, but for now compare the
ahead-behind counting algorithm in 'git for-each-ref' to the naive
implementation by running 'git rev-list --count' processes for each
input.
For the Git source code repository, the improvement is already obvious:
Test this tree
---------------------------------------------------------------
1500.2: ahead-behind counts: git for-each-ref 0.07(0.07+0.00)
1500.3: ahead-behind counts: git branch 0.07(0.06+0.00)
1500.4: ahead-behind counts: git tag 0.07(0.06+0.00)
1500.5: ahead-behind counts: git rev-list 1.32(1.04+0.27)
But the standard performance benchmark is the Linux kernel repository,
which demosntrates a significant improvement:
Test this tree
---------------------------------------------------------------
1500.2: ahead-behind counts: git for-each-ref 0.27(0.24+0.02)
1500.3: ahead-behind counts: git branch 0.27(0.24+0.03)
1500.4: ahead-behind counts: git tag 0.28(0.27+0.01)
1500.5: ahead-behind counts: git rev-list 4.57(4.03+0.54)
The 'git rev-list' test exists in this change as a demonstration, but it
will be removed in the next change to avoid wasting time on this
comparison.
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a user wishes to input a large list of patterns to 'git
for-each-ref' (likely a long list of exact refs) there are frequently
system limits on the number of command-line arguments.
Add a new --stdin option to instead read the patterns from standard
input. Add tests that check that any unrecognized arguments are
considered an error when --stdin is provided. Also, an empty pattern
list is interpreted as the complete ref set.
When reading from stdin, we populate the filter.name_patterns array
dynamically as opposed to pointing to the 'argv' array directly. This is
simple when using a strvec, as it is NULL-terminated in the same way. We
then free the memory directly from the strvec.
Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The builtins 'ls-remote', 'pack-objects', 'receive-pack', 'reflog' and
'send-pack' use parse_options(), but their source files don't directly
include 'parse-options.h'. Furthermore, the source files
'diagnose.c', 'list-objects-filter-options.c', 'remote.c' and
'send-pack.c' define option parsing callback functions, while
'revision.c' defines an option parsing helper function, and thus need
access to various fields in 'struct option' and 'struct
parse_opt_ctx_t', but they don't directly include 'parse-options.h'
either. They all can still be built, of course, because they include
one of the header files that does include 'parse-options.h' (though
unnecessarily, see the next commit).
Add those missing includes to these files, as our general rule is that
"a C file must directly include the header files that declare the
functions and the types it uses".
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git format-patch" learned to write a log-message only output file
for empty commits.
* jk/format-patch-change-format-for-empty-commits:
format-patch: output header for empty commits
"git bundle" learned that "-" is a common way to say that the input
comes from the standard input and/or the output goes to the
standard output. It used to work only for output and only from the
root level of the working tree.
* jk/bundle-use-dash-for-stdfiles:
parse-options: use prefix_filename_except_for_dash() helper
parse-options: consistently allocate memory in fix_filename()
bundle: don't blindly apply prefix_filename() to "-"
bundle: document handling of "-" as stdin
bundle: let "-" mean stdin for reading operations
Allow "git bisect reset" to check out the original branch when the
branch is already checked out in a different worktree linked to the
same repository.
* rj/bisect-already-used-branch:
bisect: fix "reset" when branch is checked out elsewhere
"git push" has been taught to allow deletion of refs with one-level
names to help repairing a repository who acquired such a ref by
mistake. In general, we don't encourage use of such a ref, and
creation or update to such a ref is rejected as before.
* zh/push-to-delete-onelevel-ref:
push: allow delete single-level ref
receive-pack: fix funny ref error messsage
"git restore" supports options like "--ours" that are only
meaningful during a conflicted merge, but these options are only
meaningful when updating the working tree files. These options are
marked to be incompatible when both "--staged" and "--worktree" are
in effect.
* ak/restore-both-incompatible-with-conflicts:
restore: fault --staged --worktree with merge opts
There's code in git_connect() that checks whether we are doing a push
with protocol_v2, and if so, drops us to protocol_v0 (since we know
how to do v2 only for fetches). But it misses some corner cases:
1. it checks the "prog" variable, which is actually the path to
receive-pack on the remote side. By default this is just
"git-receive-pack", but it could be an arbitrary string (like
"/path/to/git receive-pack", etc). We'd accidentally stay in v2
mode in this case.
2. besides "receive-pack" and "upload-pack", there's one other value
we'd expect: "upload-archive" for handling "git archive --remote".
Like receive-pack, this doesn't understand v2, and should use the
v0 protocol.
In practice, neither of these causes bugs in the real world so far. We
do send a "we understand v2" probe to the server, but since no server
implements v2 for anything but upload-pack, it's simply ignored. But
this would eventually become a problem if we do implement v2 for those
endpoints, as older clients would falsely claim to understand it,
leading to a server response they can't parse.
We can fix (1) by passing in both the program path and the "name" of the
operation. I treat the name as a string here, because that's the pattern
set in transport_connect(), which is one of our callers (we were simply
throwing away the "name" value there before).
We can fix (2) by allowing only known-v2 protocols ("upload-pack"),
rather than blocking unknown ones ("receive-pack" and "upload-archive").
That will mean whoever eventually implements v2 push will have to adjust
this list, but that's reasonable. We'll do the safe, conservative thing
(sticking to v0) by default, and anybody working on v2 will quickly
realize this spot needs to be updated.
The new tests cover the receive-pack and upload-archive cases above, and
re-confirm that we allow v2 with an arbitrary "--upload-pack" path (that
already worked before this patch, of course, but it would be an easy
thing to break if we flipped the allow/block logic without also handling
"name" separately).
Here are a few miscellaneous implementation notes, since I had to do a
little head-scratching to understand who calls what:
- transport_connect() is called only for git-upload-archive. For
non-http git remotes, that resolves to the virtual connect_git()
function (which then calls git_connect(); confused yet?). So
plumbing through "name" in connect_git() covers that.
- for regular fetches and pushes, callers use higher-level functions
like transport_fetch_refs(). For non-http git remotes, that means
calling git_connect() under the hood via connect_setup(). And that
uses the "for_push" flag to decide which name to use.
- likewise, plumbing like fetch-pack and send-pack may call
git_connect() directly; they each know which name to use.
- for remote helpers (including http), we already have separate
parameters for "name" and "exec" (another name for "prog"). In
process_connect_service(), we feed the "name" to the helper via
"connect" or "stateless-connect" directives.
There's also a "servpath" option, which can be used to tell the
helper about the "exec" path. But no helpers we implement support
it! For http it would be useless anyway (no reasonable server
implementation will allow you to send a shell command to run the
server). In theory it would be useful for more obscure helpers like
remote-ext, but even there it is not implemented.
It's tempting to get rid of it simply to reduce confusion, but we
have publicly documented it since it was added in fa8c097cc9
(Support remote helpers implementing smart transports, 2009-12-09),
so it's possible some helper in the wild is using it.
- So for v2, helpers (again, including http) are mainly used via
stateless-connect, driven by the main program. But they do still
need to decide whether to do a v2 probe. And so there's similar
logic in remote-curl.c's discover_refs() that looks for
"git-receive-pack". But it's not buggy in the same way. Since it
doesn't support servpath, it is always dealing with a "service"
string like "git-receive-pack". And since it doesn't support
straight "connect", it can't be used for "upload-archive".
So we could leave that spot alone. But I've updated it here to match
the logic we're changing in connect_git(). That seems like the least
confusing thing for somebody who has to touch both of these spots
later (say, to add v2 push support). I didn't add a new test to make
sure this doesn't break anything; we already have several tests (in
t5551 and elsewhere) that make sure we are using v2 over http.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A new "fetch.hideRefs" option can be used to exclude specified refs
from "rev-list --objects --stdin --not --all" traversal for
checking object connectivity, most useful when there are many
unrelated histories in a single repository.
* ew/fetch-hiderefs:
fetch: support hideRefs to speed up connectivity checks
Instead of forcing each command to choose to honor GPG related
configuration variables, make the subsystem lazily initialize
itself.
* jc/gpg-lazy-init:
drop pure pass-through config callbacks
gpg-interface: lazily initialize and read the configuration
More work towards -Wunused.
* jk/unused-post-2.39-part2: (21 commits)
help: mark unused parameter in git_unknown_cmd_config()
run_processes_parallel: mark unused callback parameters
userformat_want_item(): mark unused parameter
for_each_commit_graft(): mark unused callback parameter
rewrite_parents(): mark unused callback parameter
fetch-pack: mark unused parameter in callback function
notes: mark unused callback parameters
prio-queue: mark unused parameters in comparison functions
for_each_object: mark unused callback parameters
list-objects: mark unused callback parameters
mark unused parameters in signal handlers
run-command: mark error routine parameters as unused
mark "pointless" data pointers in callbacks
ref-filter: mark unused callback parameters
http-backend: mark unused parameters in virtual functions
http-backend: mark argc/argv unused
object-name: mark unused parameters in disambiguate callbacks
serve: mark unused parameters in virtual functions
serve: use repository pointer to get config
ls-refs: drop config caching
...
Code clean-up to clarify the rule that "git-compat-util.h" must be
the first to be included.
* en/header-cleanup:
diff.h: remove unnecessary include of object.h
Remove unnecessary includes of builtin.h
treewide: replace cache.h with more direct headers, where possible
replace-object.h: move read_replace_refs declaration from cache.h to here
object-store.h: move struct object_info from cache.h
dir.h: refactor to no longer need to include cache.h
object.h: stop depending on cache.h; make cache.h depend on object.h
ident.h: move ident-related declarations out of cache.h
pretty.h: move has_non_ascii() declaration from commit.h
cache.h: remove dependence on hex.h; make other files include it explicitly
hex.h: move some hex-related declarations from cache.h
hash.h: move some oid-related declarations from cache.h
alloc.h: move ALLOC_GROW() functions from cache.h
treewide: remove unnecessary cache.h includes in source files
treewide: remove unnecessary cache.h includes
treewide: remove unnecessary git-compat-util.h includes in headers
treewide: ensure one of the appropriate headers is sourced first
Code clean-up to clarify directory traversal API.
* en/dir-api-cleanup:
unpack-trees: add usage notices around df_conflict_entry
unpack-trees: special case read-tree debugging as internal usage
unpack-trees: rewrap a few overlong lines from previous patch
unpack-trees: mark fields only used internally as internal
unpack_trees: start splitting internal fields from public API
sparse-checkout: avoid using internal API of unpack-trees, take 2
sparse-checkout: avoid using internal API of unpack-trees
unpack-trees: clean up some flow control
dir: mark output only fields of dir_struct as such
dir: add a usage note to exclude_per_dir
dir: separate public from internal portion of dir_struct
unpack-trees: heed requests to overwrite ignored files
t2021: fix platform-specific leftover cruft
"git fsck" learned to check the index files in other worktrees,
just like "git gc" honors them as anchoring points.
* jk/fsck-indices-in-worktrees:
fsck: check even zero-entry index files
fsck: mention file path for index errors
fsck: check index files in all worktrees
fsck: factor out index fsck
When git-rebase invokes format-patch, it wants to make sure we use the
normal prefixes, and are not confused by diff.noprefix or similar. When
this was added in 5b220a6876 (Add --src/dst-prefix to git-formt-patch
in git-rebase.sh, 2010-09-09), we only had --src-prefix and --dst-prefix
to do so, which requires re-specifying the prefixes we expect to see.
These days we can say what we want more directly: just use the defaults.
This is a minor cleanup that should have no behavior change, but
hopefully the result expresses more clearly what the code is trying to
accomplish.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix a bug introduced with the "--format" option in
ce74de93 (ls-files: introduce "--format" option, 2022-07-23),
where relative paths were computed using the output buffer,
which could lead to random garbage data in the output.
Signed-off-by: Adam Johnson <me@adamj.eu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When accepting a packfile in git-receive-pack(1), we feed that packfile
into git-index-pack(1) to generate the packfile index. As the packfile
would often only contain unreachable objects until the references have
been updated, concurrently running garbage collection might be tempted
to delete the packfile right away and thus cause corruption. To fix
this, we ask git-index-pack(1) to create a `.keep` file before moving
the packfile into place, which is getting deleted again once all of the
reference updates have been processed.
Now in production systems we have observed that those `.keep` files are
sometimes not getting deleted as expected, where the result is that
repositories tend to grow packfiles that are never deleted over time.
This seems to be caused by a race when git-receive-pack(1) is killed
after we have migrated the kept packfile from the quarantine directory
into the main object database. While this race window is typically small
it can be extended for example by installing a `proc-receive` hook.
Fix this race by registering the lockfile as a tempfile so that it will
automatically be removed at exit or when receiving a signal.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It seems a user would expect this option would work regardless
of whether it's fetching from a single remote, many remotes,
or recursing into submodules.
Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous commit dropped support for diff.noprefix in format-patch.
While this will do the right thing in most cases (where sending patches
without a prefix was an accidental side effect of the sender preferring
to see their local patches without prefixes), it left no good option for
a project or workflow where you really do want to send patches without
prefixes. You'd be stuck using "--no-prefix" for every invocation.
So let's add a config option specific to format-patch that enables this
behavior. That gives people who have such a workflow a way to get what
they want, but makes it hard to accidentally trigger it.
A more backwards-compatible way of doing the transition would be to have
format.noprefix default to diff.noprefix when it's not set. But that
doesn't really help the "accidental" problem; people would have to
manually set format.noprefix=false. And it's unlikely that anybody
really wants format.noprefix=true in the first place. I'm adding it here
mostly as an escape hatch, not because anybody has expressed any
interest in it.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The output of format-patch respects diff.noprefix, but this usually ends
up being a hassle for people receiving the patch, as they have to
manually specify "-p0" in order to apply it.
I don't think there was any specific intention for it to behave this
way. The noprefix option is handled by git_diff_ui_config(), and
format-patch exists in a gray area between plumbing and porcelain.
People do look at the output, and we'd expect it to colorize things,
respect their choice of algorithm, and so on. But this particular option
creates problems for the receiver (in theory so does diff.mnemonicprefix,
but since we are always formatting commits, the mnemonic prefixes will
always be "a/" and "b/").
So let's disable it. The slight downsides are:
- people who have set diff.noprefix presumably like to see their
patches without prefixes. If they use format-patch to review their
series, they'll see prefixes. On the other hand, it is probably a
good idea for them to look at what will actually get sent out.
We could try to play games here with "is stdout a tty", as we do for
color. But that's not a completely reliable signal, and it's
probably not worth the trouble. If you want to see the patch with
the usual bells and whistles, then you are better off using "git
log" or "git show".
- if a project really does have a workflow that likes prefix-less
patches, and the receiver is prepared to use "-p0", then the sender
now has to manually say "--no-prefix" for each format-patch
invocation. That doesn't seem _too_ terrible given that the receiver
has to manually say "-p0" for each git-am invocation.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When handling OPT_FILENAME(), we have to stick the "prefix" (if any) in
front of the filename to make up for the fact that Git has chdir()'d to
the top of the repository. We can do this with prefix_filename(), but
there are a few special cases we handle ourselves.
Unfortunately the memory allocation is inconsistent here; if we do make
it to prefix_filename(), we'll allocate a string which the caller must
free to avoid a leak. But if we hit our special cases, we'll return the
string as-is, and a caller which tries to free it will crash. So there's
no way to win.
Let's consistently allocate, so that callers can do the right thing.
There are now three cases to care about in the function (and hence a
three-armed if/else):
1. we got a NULL input (and should leave it as NULL, though arguably
this is the sign of a bug; let's keep the status quo for now and we
can pick at that scab later)
2. we hit a special case that means we leave the name intact; we
should duplicate the string. This includes our special "-"
matching. Prior to this patch, it also included empty prefixes and
absolute filenames. But we can observe that prefix_filename()
already handles these, so we don't need to detect them.
3. everything else goes to prefix_filename()
I've dropped the "const" from the "char **file" parameter to indicate
that we're allocating, though in practice it's not really important.
This is all being shuffled through a void pointer via opt->value before
it hits code which ever looks at the string. And it's even a bit weird,
because we are really taking _in_ a const string and using the same
out-parameter for a non-const string. A better function signature would
be:
static char *fix_filename(const char *prefix, const char *file);
but that would mean the caller dereferences the double-pointer (and the
NULL check is currently handled inside this function). So I took the
path of least-change here.
Note that we have to fix several callers in this commit, too, or we'll
break the leak-checking tests. These are "new" leaks in the sense that
they are now triggered by the test suite, but these spots have always
been leaky when Git is run in a subdirectory of the repository. I fixed
all of the cases that trigger with GIT_TEST_PASSING_SANITIZE_LEAK. There
may be others in scripts that have other leaks, but we can fix them
later along with those other leaks (and again, you _couldn't_ fix them
before this patch, so this is the necessary first step).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A user can specify a filename to a command from the command line,
either as the value given to a command line option, or a command
line argument. When it is given as a relative filename, in the
user's mind, it is relative to the directory "git" was started from,
but by the time the filename is used, "git" would almost always have
chdir()'ed up to the root level of the working tree.
The given filename, if it is relative, needs to be prefixed with the
path to the current directory, and it typically is done by calling
prefix_filename() helper function. For commands that can also take
"-" to use the standard input or the standard output, however, this
needs to be done with care.
"git bundle create" uses the next word on the command line as the
output filename, and can take "-" to mean "write to the standard
output". It blindly called prefix_filename(), so running it in a
subdirectory did not quite work as expected.
Introduce a new helper, prefix_filename_except_for_dash(), and use
it to help "git bundle create" codepath.
Reported-by: Michael Henry
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For writing, "bundle create -" indicates that the bundle should be
written to stdout. But there's no matching handling of "-" for reading
operations. This is inconsistent, and a little inflexible (though one
can always use "/dev/stdin" on systems that support it).
However, it's easy to change. Once upon a time, the bundle-reading code
required a seekable descriptor, but that was fixed long ago in
e9ee84cf28 (bundle: allowing to read from an unseekable fd,
2011-10-13). So we just need to handle "-" explicitly when opening the
file.
We _could_ do this by handling "-" in read_bundle_header(), which the
reading functions all call already. But that is probably a bad idea.
It's also used by low-level code like the transport functions, and we
may want to be more careful there. We do not know that stdin is even
available to us, and certainly we would not want to get confused by a
configured URL that happens to point to "-".
So instead, let's add a helper to builtin/bundle.c. Since both the
bundle code and some of the callers refer to the bundle by name for
error messages, let's use the string "<stdin>" to make the output a bit
nicer to read.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 79862b6b77 (bundle-create: progress output control, 2019-11-10),
"bundle create" learned about the --all-progress and
--all-progress-implied options, which were copied from pack-objects.
I think these were a mistake.
In pack-objects, "all-progress-implied" is about switching the behavior
between a regular on-disk "git repack" and the use of pack-objects for
push/fetch (where a fetch does not want progress from the server during
the write stage; the client will print progress as it receives the
data). But there's no such distinction for bundles. Prior to
79862b6b77, we always printed the write stage. Afterwards, a vanilla:
git bundle create foo.bundle
omits the write progress, appearing to hang (especially if your
repository is large or your disk is slow). That seems like a regression.
It's possible that the flexibility to disable the write-phase progress
_could_ be useful for bundle. E.g., if you did something like:
ssh some-host git bundle create foo.bundle |
git bundle unbundle
But if you are running both in real-time, why are you using bundles in
the first place? You're better off doing a real fetch.
But even if we did want to support that, it should be the exception, and
vanilla "bundle create" should display the full progress. So we'd want
to name the option "--no-write-progress" or something.
The "--all-progress" option itself is even worse. It exists in
pack-objects only for historical reasons. It's a mistake because it
implies "--progress", and we added "--all-progress-implied" to fix that.
There is no reason to propagate that mistake to new commands.
Likewise, the documentation for these options was pulled from
pack-objects. But it doesn't make any sense in this context. It talks
about "--stdout", but that is not even an option that git-bundle
supports.
This patch flips the default for "--all-progress-implied" back to
"true", fixing the regression in 79862b6b77. This turns that option
into a noop, and means that "--all-progress" is really the same as
"--progress". We _could_ drop them completely, but since they've been
shipped with Git since v2.25.0, it's polite to continue accepting them.
I didn't implement any sort of "--no-write-progress" here. I'm not at
all convinced it's necessary, and the discussion from the original
thread:
https://lore.kernel.org/git/20191110204126.30553-2-robbat2@gentoo.org/
shows that that the main focus was on getting --progress and --quiet
support, and not any kind of clever "real-time bundle over the network"
feature. But technically this patch is making it impossible to do
something that you _could_ do post-79862b6b77c.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When formatting an empty commit, it is surprising that a totally empty
file is generated. Set the flag to always print the header, matching
the behaviour of git-log.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We discourage the creation/update of single-level refs
because some upper-layer applications only work in specified
reference namespaces, such as "refs/heads/*" or "refs/tags/*",
these single-level refnames may not be recognized. However,
we still hope users can delete them which have been created
by mistake.
Therefore, when updating branches on the server with
"git receive-pack", by checking whether it is a branch deletion
operation, it will determine whether to allow the update of
a single-level refs. This avoids creating/updating such
single-level refs, but allows them to be deleted.
On the client side, "git push" also does not properly fill in
the old-oid of single-level refs, which causes the server-side
"git receive-pack" to think that the ref's old-oid has changed
when deleting single-level refs, this causes the push to be
rejected. So the solution is to fix the client to be able to
delete single-level refs by properly filling old-oid.
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the user deletes the remote one level branch through
"git push origin -d refs/foo", remote will return an error:
"refusing to create funny ref 'refs/foo' remotely", here we
are not creating "refs/foo" instead wants to delete it, so a
better error description here would be: "refusing to update
funny ref 'refs/foo' remotely".
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The config option (as documented) for rebase.autoSquash has a capital S,
whereas the command line option has a small case s.
Cf. <20220617100309.3224-1-worldhello.net@gmail.com>
Signed-off-by: Fangyi Zhou <me@fangyi.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
