Hints that suggest what to do after resolving conflicts can now be
squelched by disabling advice.mergeConflict.
Acked-by: Phillip Wood <phillip.wood123@gmail.com>
cf. <e040c631-42d9-4501-a7b8-046f8dac6309@gmail.com>
* pb/advice-merge-conflict:
builtin/am: allow disabling conflict advice
sequencer: allow disabling conflict advice
Code clean-up in the "git log" machinery that implements custom log
message formatting.
* jk/pretty-subject-cleanup:
format-patch: fix leak of empty header string
format-patch: simplify after-subject MIME header handling
format-patch: return an allocated string from log_write_email_headers()
log: do not set up extra_headers for non-email formats
pretty: drop print_email_subject flag
pretty: split oneline and email subject printing
shortlog: stop setting pp.print_email_subject
"git checkout --conflict=bad" reported a bad conflictStyle as if it
were given to a configuration variable; it has been corrected to
report that the command line option is bad.
* pw/checkout-conflict-errorfix:
checkout: fix interaction between --conflict and --merge
checkout: cleanup --conflict=<style> parsing
merge options: add a conflict style member
merge-ll: introduce LL_MERGE_OPTIONS_INIT
xdiff-interface: refactor parsing of merge.conflictstyle
The status.showUntrackedFiles configuration variable had a name
that tempts users to set a Boolean value expressed in our usual
"false", "off", and "0", but it only took "no". This has been
corrected so "true" and its synonyms are taken as "normal", while
"false" and its synonyms are taken as "no".
* jc/show-untracked-false:
status: allow --untracked=false and friends
status: unify parsing of --untracked= and status.showUntrackedFiles
Work to support a repository that work with both SHA-1 and SHA-256
hash algorithms has started.
* eb/hash-transition: (30 commits)
t1016-compatObjectFormat: add tests to verify the conversion between objects
t1006: test oid compatibility with cat-file
t1006: rename sha1 to oid
test-lib: compute the compatibility hash so tests may use it
builtin/ls-tree: let the oid determine the output algorithm
object-file: handle compat objects in check_object_signature
tree-walk: init_tree_desc take an oid to get the hash algorithm
builtin/cat-file: let the oid determine the output algorithm
rev-parse: add an --output-object-format parameter
repository: implement extensions.compatObjectFormat
object-file: update object_info_extended to reencode objects
object-file-convert: convert commits that embed signed tags
object-file-convert: convert commit objects when writing
object-file-convert: don't leak when converting tag objects
object-file-convert: convert tag objects when writing
object-file-convert: add a function to convert trees between algorithms
object: factor out parse_mode out of fast-import and tree-walk into in object.h
cache: add a function to read an OID of a specific algorithm
tag: sign both hashes
commit: export add_header_signature to support handling signatures on tags
...
"git bugreport --no-suffix" was not supported and instead
segfaulted, which has been corrected.
* js/bugreport-no-suffix-fix:
bugreport.c: fix a crash in `git bugreport` with `--no-suffix` option
In the run_am() function, we set up a child_process struct to run
"git-am", allocating memory for its args and env strvecs. These are
normally cleaned up when we call run_command(). But if we encounter
certain errors, we exit the function early and try to clean up ourselves
by clearing the am.args field. This leaks the "env" strvec.
We should use child_process_clear() instead, which covers both. And more
importantly, it future proofs us against the struct ever growing more
allocated fields.
These are unlikely errors to happen in practice, so they don't actually
trigger the leak sanitizer in the tests. But we can add a new test which
does exercise one of the paths (and fails SANITIZE=leak without this
patch).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When pretty-printing a commit in the email format, we have to fill in
the "after subject" field of the pretty_print_context with any extra
headers the user provided (e.g., from "--to" or "--cc" options) plus any
special MIME headers.
We return an out-pointer that sometimes points to a newly heap-allocated
string and sometimes not. To avoid leaking, we store the allocated
version in a buffer with static lifetime, which is ugly. Worse, as we
extend the header feature, we'll end up having to repeat this ugly
pattern.
Instead, let's have our out-pointer pass ownership back to the caller,
and duplicate the string when necessary. This does mean one extra
allocation per commit when you use extra headers, but in the context of
format-patch which is showing diffs, I don't think that's even
measurable.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With one exception, the print_email_subject flag is set if and only if
the commit format is email based:
- in make_cover_letter() we set it along with CMIT_FMT_EMAIL
explicitly
- in show_log(), we set it if cmit_fmt_is_mail() is true. That covers
format-patch as well as "git log --format=email" (or mboxrd).
The one exception is "rev-list --format=email", which somewhat
nonsensically prints the author and date as email headers, but no
subject, like:
$ git rev-list --format=email HEAD
commit 64fc4c2cdd4db2645eaabb47aa4bac820b03cdba
From: Jeff King <peff@peff.net>
Date: Tue, 19 Mar 2024 19:39:26 -0400
this is the subject
this is the body
It's doubtful that this is a useful format at all (the "commit" lines
replace the "From" lines that would make it work as an actual mbox).
But I think that printing the subject as a header (like this patch does)
is the least surprising thing to do.
So let's drop this field, making the code a little simpler and easier to
reason about. Note that we do need to set the "rev" field of the
pretty_print_context in rev-list, since that is used to check for
subject_prefix, etc. It's not possible to set those fields via rev-list,
so we'll always just print "Subject: ". But unless we pass in our
rev_info, fmt_output_email_subject() would segfault trying to figure it
out.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The pp_title_line() function is used for two formats: the oneline format
and the subject line of the email format. But most of the logic in the
function does not make any sense for oneline; it is about special
formatting of email headers.
Lumping the two formats together made sense long ago in 4234a76167
(Extend --pretty=oneline to cover the first paragraph, 2007-06-11), when
there was a lot of manual logic to paste lines together. But later,
88c44735ab (pretty: factor out format_subject(), 2008-12-27) pulled that
logic into its own function.
We can implement the oneline format by just calling that one function.
This makes the intention of the code much more clear, as we know we only
need to worry about those extra email options when dealing with actual
email.
While the intent here is cleanup, it is possible to trigger these cases
in practice by running format-patch with an explicit --oneline option.
But if you did, the results are basically nonsense. For example, with
the preserve_subject flag:
$ printf "%s\n" one two three | git commit --allow-empty -F -
$ git format-patch -1 --stdout -k | grep ^Subject
Subject: =?UTF-8?q?one=0Atwo=0Athree?=
$ git format-patch -1 --stdout -k --oneline --no-signature
2af7fbe one
two
three
Or with extra headers:
$ git format-patch -1 --stdout --cc=me --oneline --no-signature
2af7fbe one two three
Cc: me
So I'd actually consider this to be an improvement, though you are
probably crazy to use other formats with format-patch in the first place
(arguably it should forbid non-email formats entirely, but that's a
bigger change).
As a bonus, it eliminates some pointless extra allocations for the
oneline output. The email code, since it has to deal with wrapping,
formats into an extra auxiliary buffer. The speedup is tiny, though like
"rev-list --no-abbrev --format=oneline" seems to improve by a consistent
1-2% for me.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When shortlog processes a commit using its internal traversal, it may
pretty-print the subject line for the summary view. When we do so, we
set the "print_email_subject" flag in the pretty-print context. But this
flag does nothing! Since we are using CMIT_FMT_USERFORMAT, we skip most
of the usual formatting code entirely.
This flag is there due to commit 6d167fd7cc (pretty: use
fmt_output_email_subject(), 2017-03-01). But that just switched us away
from setting an empty "subject" header field, which was similarly
useless. That was added by dd2e794a21 (Refactor pretty_print_commit
arguments into a struct, 2009-10-19). Before using the struct, we had to
pass _something_ as the argument, so we passed the empty string (a NULL
would have worked equally well).
So this setting has never done anything, and we can drop the line. That
shortens the code, but more importantly, makes it easier to reason about
and refactor the other users of this flag.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code simplification by getting rid of code that sets an environment
variable that is no longer used.
* pw/rebase-i-ignore-cherry-pick-help-environment:
rebase -i: stop setting GIT_CHERRY_PICK_HELP
When 'git am' or 'git rebase --apply' encounter a conflict, they show a
message instructing the user how to continue the operation. This message
can't be disabled.
Use ADVICE_MERGE_CONFLICT introduced in the previous commit to allow
disabling it. Update the tests accordingly, as the advice output is now
on stderr instead of stdout. In t4150, redirect stdout to 'out' and
stderr to 'err', since this is less confusing. In t4254, as we are
testing a specific failure mode of 'git am', simply disable the advice.
Note that we are not testing that this advice is shown in 'git rebase'
for the apply backend since 2ac0d6273f (rebase: change the default
backend from "am" to "merge", 2020-02-15).
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
`git bugreport` does not complain when `--no-suffix` is given, but
it leads to a segmentation fault as the it is not prepared to see a
NULL assigned to the option_suffix variable.
Signed-off-by: Jiamu Sun <barroit@linux.com>
Acked-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Uses of xwrite() helper have been audited and updated for better
error checking and simpler code.
* jc/xwrite-cleanup:
repack: check error writing to pack-objects subprocess
sideband: avoid short write(2)
unpack: replace xwrite() loop with write_in_full()
When git refuses to create a branch because the proposed branch
name is not a valid refname, an advice message is given to refer
the user to exact naming rules.
* kh/branch-ref-syntax-advice:
branch: advise about ref syntax rules
advice: use double quotes for regular quoting
advice: use backticks for verbatim
advice: make all entries stylistically consistent
t3200: improve test style
Trailer API updates.
Acked-by: Christian Couder <christian.couder@gmail.com>
cf. <CAP8UFD1Zd+9q0z1JmfOf60S2vn5-sD3SafDvAJUzRFwHJKcb8A@mail.gmail.com>
* la/trailer-api:
format_trailers_from_commit(): indirectly call trailer_info_get()
format_trailer_info(): move "fast path" to caller
format_trailers(): use strbuf instead of FILE
trailer_info_get(): reorder parameters
trailer: move interpret_trailers() to interpret-trailers.c
trailer: reorder format_trailers_from_commit() parameters
trailer: rename functions to use 'trailer'
shortlog: add test for de-duplicating folded trailers
trailer: free trailer_info _after_ all related usage
The implementation in "git clean" that makes "-n" and "-i" ignore
clean.requireForce has been simplified, together with the
documentation.
* so/clean-dry-run-without-force:
clean: further clean-up of implementation around "--force"
clean: improve -n and -f implementation and documentation
In git-restore we need to free the pathspec and pathspec_from_file
values from the struct checkout_opts.
A simple fix could be to free them in cmd_restore, after the call to
checkout_main returns, like we are doing [1][2] in the sibling function
cmd_checkout.
However, we can do even better.
We have git-switch and git-restore, both of them spin-offs[3][4] of
git-checkout. All three are implemented as thin wrappers around
checkout_main. Considering this, it makes a lot of sense to do the
cleanup closer to checkout_main.
Move the cleanups, including the new_branch_info variable, to
checkout_main.
As a consequence, mark: t2070, t2071, t2072 and t6418 as leak-free.
[1] 9081a421a6 (checkout: fix "branch info" memory leaks, 2021-11-16)
[2] 7ce4088ab7 (parse-options: consistently allocate memory in
fix_filename(), 2023-03-04)
[3] d787d311db (checkout: split part of it to new command 'switch',
2019-03-29)
[4] 46e91b663b (checkout: split part of it to new command 'restore',
2019-04-25)
Signed-off-by: Rubén Justo <rjusto@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When using "git checkout" to recreate merge conflicts or merge
uncommitted changes when switching branch "--conflict" sensibly implies
"--merge". Unfortunately the way this is implemented means that "git
checkout --conflict=diff3 --no-merge" implies "--merge" violating the
usual last-one-wins rule. Fix this by only overriding the value of
opts->merge if "--conflicts" comes after "--no-merge" or "-[-no]-merge"
is not given on the command line.
The behavior of "git checkout --merge --no-conflict" is unchanged and
will still merge on the basis that the "-[-no]-conflict" options are
primarily intended to affect the conflict style and so "--no-conflict"
should cancel a previous "--conflict" but not override "--merge".
Of the four new tests the second one tests the behavior change
introduced by this commit, the other three check that this commit does
not regress the existing behavior.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Passing an invalid conflict style name such as "--conflict=bad" gives
the error message
error: unknown style 'bad' given for 'merge.conflictstyle'
which is unfortunate as it talks about a config setting rather than
the option given on the command line. This happens because the
implementation calls git_xmerge_config() to set the conflict style
using the value given on the command line. Use the newly added
parse_conflict_style_name() instead and pass the value down the call
chain to override the config setting. This also means we can avoid
setting up a struct config_context required for calling
git_xmerge_config().
The option is now parsed in a callback to avoid having to store the
option name. This is a change in behavior as now
git checkout --conflict=bad --conflict=diff3
will error out when parsing "--conflict=bad" whereas before this change
it would succeed because it would only try to parse the value of the
last "--conflict" option given on the command line.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Introduce a macro to initialize `struct ll_merge_options` in preparation
for the next commit that will add a new member that needs to be
initialized to a non-zero value.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is natural to expect that the "--untracked" option and the
status.showuntrackedFiles configuration variable to take a Boolean
value ("do you want me to show untracked files?"), but the current
code takes nothing but "no" as "no, please do not show any".
Allow the usual Boolean values to be given, and treat 'true' as
"normal", and 'false' as "no".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are two code paths that take a string and parse it to enum
untracked_status_type. Introduce a helper function and use it.
As these two places handle an error differently, add an additional
invalid value to the enum, and have the caller of the helper handle
the error condition, instead of dying or emitting error message from
the helper.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Various parts of upload-pack has been updated to bound the resource
consumption relative to the size of the repository to protect from
abusive clients.
* jk/upload-pack-bounded-resources:
upload-pack: free tree buffers after parsing
upload-pack: use PARSE_OBJECT_SKIP_HASH_CHECK in more places
upload-pack: always turn off save_commit_buffer
upload-pack: disallow object-info capability by default
upload-pack: accept only a single packfile-uri line
upload-pack: use a strmap for want-ref lines
upload-pack: use oidset for deepen_not list
upload-pack: switch deepen-not list to an oid_array
upload-pack: drop separate v2 "haves" array
A custom remote helper no longer cannot access the newly created
repository during "git clone", which is a regression in Git 2.44.
This has been corrected.
* ps/remote-helper-repo-initialization-fix:
builtin/clone: allow remote helpers to detect repo
"git commit -v --cleanup=scissors" used to add the scissors line
twice in the log message buffer, which has been corrected.
* jt/commit-redundant-scissors-fix:
commit: unify logic to avoid multiple scissors lines when merging
commit: avoid redundant scissor line with --cleanup=scissors -v
"git merge-tree" has learned that the three trees involved in the
3-way merge only need to be trees, not necessarily commits.
* js/merge-tree-3-trees:
fill_tree_descriptor(): mark error message for translation
cache-tree: avoid an unnecessary check
Always check `parse_tree*()`'s return value
t4301: verify that merge-tree fails on missing blob objects
merge-ort: do check `parse_tree()`'s return value
merge-tree: fail with a non-zero exit code on missing tree objects
merge-tree: accept 3 trees as arguments
"git rev-list --missing=print" has learned to optionally take
"--allow-missing-tips", which allows the objects at the starting
points to be missing.
* cc/rev-list-allow-missing-tips:
revision: fix --missing=[print|allow*] for annotated tags
rev-list: allow missing tips with --missing=[print|allow*]
t6022: fix 'test' style and 'even though' typo
oidset: refactor oidset_insert_from_set()
revision: clarify a 'return NULL' in get_reference()
bare in that context is an option, not purely an adjective
Mark it properly
Signed-off-by: Alexander Shopov <ash@kambanaria.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-branch(1) will error out if you give it a bad ref name. But the user
might not understand why or what part of the name is illegal.
The user might know that there are some limitations based on the *loose
ref* format (filenames), but there are also further rules for
easier integration with shell-based tools, pathname expansion, and
playing well with reference name expressions.
The man page for git-check-ref-format(1) contains these rules. Let’s
advise about it since that is not a command that you just happen
upon. Also make this advise configurable since you might not want to be
reminded every time you make a little typo.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git for-each-ref" learned "--include-root-refs" option to show
even the stuff outside the 'refs/' hierarchy.
* kn/for-all-refs:
for-each-ref: add new option to include root refs
ref-filter: rename 'FILTER_REFS_ALL' to 'FILTER_REFS_REGULAR'
refs: introduce `refs_for_each_include_root_refs()`
refs: extract out `loose_fill_ref_dir_regular_file()`
refs: introduce `is_pseudoref()` and `is_headref()`
Many small allocations "git name-rev" makes have been updated to
allocate from a mem-pool.
* rs/name-rev-with-mempool:
name-rev: use mem_pool_strfmt()
mem-pool: add mem_pool_strfmt()
We clarified how "clean.requireForce" interacts with the "--dry-run"
option in the previous commit, both in the implementation and in the
documentation. Even when "git clean" (without other options) is
required to be used with "--force" (i.e. either clean.requireForce
is unset, or explicitly set to true) to protect end-users from
casual invocation of the command by mistake, "--dry-run" does not
require "--force" to be used, because it is already its own
protection mechanism by being a no-op to the working tree files.
The previous commit, however, missed another clean-up opportunity
around the same area. Just like in the "--dry-run" mode, the
command in the "--interactive" mode does not require "--force",
either. This is because by going interactive and giving the end
user one more chance to confirm, the mode itself is serving as its
own protection mechanism.
Let's take things one step further, and unify the code that defines
interaction between "--force" and these two other options. Just
like we added explanation for the reason why "--dry-run" does not
honor "clean.requireForce", give an explanation for the reason why
"--interactive" makes "clean.requireForce" to be ignored.
Finally, add some tests to show the interaction between "--force"
and "--interactive". We already have tests that show interaction
between "--force" and "--dry-run", but didn't test "--interactive".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
What -n actually does in addition to its documented behavior is
ignoring of configuration variable clean.requireForce, that makes
sense provided -n prevents files removal anyway.
So, first, document this in the manual, and then modify implementation
to make this more explicit in the code.
Improved implementation also stops to share single internal variable
'force' between command-line -f option and configuration variable
clean.requireForce, resulting in more clear logic.
Two error messages with slightly different text depending on if
clean.requireForce was explicitly set or not, are merged into a single
one.
The resulting error message now does not mention -n as well, as it
neither matches intended clean.requireForce usage nor reflects
clarified implementation.
Documentation of clean.requireForce is changed accordingly.
Signed-off-by: Sergey Organov <sorganov@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When "git repack" repacks promisor objects, it starts a pack-objects
subprocess and uses xwrite() to send object names over the pipe to
it, but without any error checking. An I/O error or short write
(even though a short write is unlikely for such a small amount of
data) can result in a packfile that lacks certain objects we wanted
to put in there, leading to a silent repository corruption.
Use write_in_full(), instead of xwrite(), to mitigate short write
risks, check errors from it, and abort if we see a failure.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have two packfile stream consumers, index-pack and
unpack-objects, that allow excess payload after the packfile stream
data. Their code to relay excess data hasn't changed significantly
since their original implementation that appeared in 67e5a5ec
(git-unpack-objects: re-write to read from stdin, 2005-06-28) and
9bee2478 (mimic unpack-objects when --stdin is used with index-pack,
2006-10-25).
These code blocks contain hand-rolled loops using xwrite(), written
before our write_in_full() helper existed. This helper now provides
the same functionality.
Replace these loops with write_in_full() for shorter, clearer
code. Update related variables accordingly.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git reflog" learned a "list" subcommand that enumerates known reflogs.
* ps/reflog-list:
builtin/reflog: introduce subcommand to list reflogs
refs: stop resolving ref corresponding to reflogs
refs: drop unused params from the reflog iterator callback
refs: always treat iterators as ordered
refs/files: sort merged worktree and common reflogs
refs/files: sort reflogs returned by the reflog iterator
dir-iterator: support iteration in sorted order
dir-iterator: pass name to `prepare_next_entry_data()` directly
This is another preparatory refactor to unify the trailer formatters.
Make format_trailers() also write to a strbuf, to align with
format_trailers_from_commit() which also does the same. Doing this makes
format_trailers() behave similar to format_trailer_info() (which will
soon help us replace one with the other).
Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The interpret-trailers.c builtin is the only place we need to call
interpret_trailers(), so move its definition there (together with a few
helper functions called only by it) and remove its external declaration
from <trailer.h>.
Several helper functions that are called by interpret_trailers() remain
in trailer.c because other callers in the same file still call them.
Declare them in <trailer.h> so that interpret_trailers() (now in
builtin/interpret-trailers.c) can continue calling them as a trailer API
user.
This enriches <trailer.h> with a more granular API, which can then be
unit-tested in the future (because interpret_trailers() by itself does
too many things to be able to be easily unit-tested).
Take this opportunity to demote some file-handling functions out of the
trailer API implementation, as these have nothing to do with trailers.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
