Junio C Hamano [Thu, 22 Jun 2023 23:29:06 +0000 (16:29 -0700)]
Merge branch 'ps/cat-file-null-output'
"git cat-file --batch" and friends learned "-Z" that uses NUL
delimiter for both input and output.
* ps/cat-file-null-output:
cat-file: add option '-Z' that delimits input and output with NUL
cat-file: simplify reading from standard input
strbuf: provide CRLF-aware helper to read until a specified delimiter
t1006: modernize test style to use `test_cmp`
t1006: don't strip timestamps from expected results
The object traversal using reachability bitmap done by
"pack-object" has been tweaked to take advantage of the fact that
using "boundary" commits as representative of all the uninteresting
ones can save quite a lot of object enumeration.
* tb/pack-bitmap-traversal-with-boundary:
pack-bitmap.c: use commit boundary during bitmap traversal
pack-bitmap.c: extract `fill_in_bitmap()`
object: add object_array initializer helper function
Junio C Hamano [Thu, 22 Jun 2023 23:29:05 +0000 (16:29 -0700)]
Merge branch 'ja/worktree-orphan'
'git worktree add' learned how to create a worktree based on an
orphaned branch with `--orphan`.
* ja/worktree-orphan:
worktree add: emit warn when there is a bad HEAD
worktree add: extend DWIM to infer --orphan
worktree add: introduce "try --orphan" hint
worktree add: add --orphan flag
t2400: add tests to verify --quiet
t2400: refactor "worktree add" opt exclusion tests
t2400: cleanup created worktree in test
worktree add: include -B in usage docs
Elijah Newren [Tue, 16 May 2023 06:34:08 +0000 (06:34 +0000)]
fsmonitor-ll.h: split this header out of fsmonitor.h
This creates a new fsmonitor-ll.h with most of the functions from
fsmonitor.h, though it leaves three inline functions where they were.
Two-thirds of the files that previously included fsmonitor.h did not
need those three inline functions or the six extra includes those inline
functions required, so this allows them to only include the lower level
header.
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:34:07 +0000 (06:34 +0000)]
hash-ll, hashmap: move oidhash() to hash-ll
oidhash() was used by both hashmap and khash, which makes sense.
However, the location of this function in hashmap.[ch] meant that
khash.h had to depend upon hashmap.h, making people unfamiliar with
khash think that it was built upon hashmap. (Or at least, I personally
was confused for a while about this in the past.)
Move this function to hash-ll, so that khash.h can stop depending upon
hashmap.h.
This has another benefit as well: it allows us to remove hashmap.h's
dependency on hash-ll.h. While some callers of hashmap.h were making
use of oidhash, most were not, so this change provides another way to
reduce the number of includes.
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:34:06 +0000 (06:34 +0000)]
object-store-ll.h: split this header out of object-store.h
The vast majority of files including object-store.h did not need dir.h
nor khash.h. Split the header into two files, and let most just depend
upon object-store-ll.h, while letting the two callers that need it
depend on the full object-store.h.
After this patch:
$ git grep -h include..object-store | sort | uniq -c
2 #include "object-store.h"
129 #include "object-store-ll.h"
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:34:05 +0000 (06:34 +0000)]
khash: name the structs that khash declares
khash.h lets you instantiate custom hash types that map between two
types. These are defined as a struct, as you might expect, and khash
typedef's that to kh_foo_t. But it declares the struct anonymously,
which doesn't give a name to the struct type itself; there is no
"struct kh_foo". This has two small downsides:
- when using khash, we declare "kh_foo_t *the_foo". This is
unlike our usual naming style, which is "struct kh_foo *the_foo".
- you can't forward-declare a typedef of an unnamed struct type in
C. So we might do something like this in a header file:
struct kh_foo;
struct bar {
struct kh_foo *the_foo;
};
to avoid having to include the header that defines the real
kh_foo. But that doesn't work with the typedef'd name. Without the
"struct" keyword, the compiler doesn't know we mean that kh_foo is
a type.
So let's always give khash structs the name that matches our
conventions ("struct kh_foo" to match "kh_foo_t"). We'll keep doing
the typedef to retain compatibility with existing callers.
Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:34:04 +0000 (06:34 +0000)]
merge-ll: rename from ll-merge
A long term (but rather minor) pet-peeve of mine was the name
ll-merge.[ch]. I thought it made it harder to realize what stuff was
related to merging when I was working on the merge machinery and trying
to improve it.
Further, back in d1cbe1e6d8a ("hash-ll.h: split out of hash.h to remove
dependency on repository.h", 2023-04-22), we have split the portions of
hash.h that do not depend upon repository.h into a "hash-ll.h" (due to
the recommendation to use "ll" for "low-level" in its name[1], but which
I used as a suffix precisely because of my distaste for "ll-merge").
When we discussed adding additional "*-ll.h" files, a request was made
that we use "ll" consistently as either a prefix or a suffix. Since it
is already in use as both a prefix and a suffix, the only way to do so
is to rename some files.
Besides my distaste for the ll-merge.[ch] name, let me also note that
the files
ll-fsmonitor.h, ll-hash.h, ll-merge.h, ll-object-store.h, ll-read-cache.h
would have essentially nothing to do with each other and make no sense
to group. But giving them the common "ll-" prefix would group them. Using
"-ll" as a suffix thus seems just much more logical to me. Rename
ll-merge.[ch] to merge-ll.[ch] to achieve this consistency, and to
ensure we get a more logical grouping of files.
Elijah Newren [Tue, 16 May 2023 06:34:03 +0000 (06:34 +0000)]
git-compat-util.h: remove unneccessary include of wildmatch.h
The include of wildmatch.h in git-compat-util.h was added in cebcab189aa
(Makefile: add USE_WILDMATCH to use wildmatch as fnmatch, 2013-01-01) as
a way to be able to compile-time force any calls to fnmatch() to instead
invoke wildmatch(). The defines and inline function were removed in 70a8fc999d9 (stop using fnmatch (either native or compat), 2014-02-15),
and this include in git-compat-util.h has been unnecessary ever since.
Remove the include from git-compat-util.h, but add it to the .c files
that had omitted the direct #include they needed.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:34:02 +0000 (06:34 +0000)]
builtin.h: remove unneccessary includes
This also made it clear that a few .c files under builtin/ were
depending upon some headers but had forgotten to #include them. Add the
missing direct includes while at it.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:34:00 +0000 (06:34 +0000)]
diff.h: remove unnecessary include of oidset.h
This also made it clear that several .c files depended upon various
things that oidset included, but had omitted the direct #include for
those headers. Add those now.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:57 +0000 (06:33 +0000)]
cache.h: remove this no-longer-used header
Since this header showed up in some places besides just #include
statements, update/clean-up/remove those other places as well.
Note that compat/fsmonitor/fsm-path-utils-darwin.c previously got
away with violating the rule that all files must start with an include
of git-compat-util.h (or a short-list of alternate headers that happen
to include it first). This change exposed the violation and caused it
to stop building correctly; fix it by having it include
git-compat-util.h first, as per policy.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:56 +0000 (06:33 +0000)]
read-cache*.h: move declarations for read-cache.c functions from cache.h
For the functions defined in read-cache.c, move their declarations from
cache.h to a new header, read-cache-ll.h. Also move some related inline
functions from cache.h to read-cache.h. The purpose of the
read-cache-ll.h/read-cache.h split is that about 70% of the sites don't
need the inline functions and the extra headers they include.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:55 +0000 (06:33 +0000)]
repository.h: move declaration of the_index from cache.h
the_index is a global variable defined in repository.c; as such, its
declaration feels better suited living in repository.h rather than
cache.h. Move it.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:52 +0000 (06:33 +0000)]
preload-index.h: move declarations for preload-index.c from elsewhere
We already have a preload-index.c file; move the declarations for the
functions in that file into a new preload-index.h. These were
previously split between cache.h and repository.h.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:48 +0000 (06:33 +0000)]
statinfo: move stat_{data,validity} functions from cache/read-cache
These functions do not depend upon struct cache_entry or struct
index_state in any way, and it seems more logical to break them out into
this file, especially since statinfo.h already has the struct stat_data
declaration.
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:47 +0000 (06:33 +0000)]
read-cache: move shared add/checkout/commit code
The function add_files_to_cache(), plus associated helper functions,
were defined in builtin/add.c, but also shared with builtin/checkout.c
and builtin/commit.c. Move these shared functions to read-cache.c.
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:46 +0000 (06:33 +0000)]
add: modify add_files_to_cache() to avoid globals
The function add_files_to_cache() is used by all three of builtin/{add,
checkout, commit}.c. That suggests this is common library code, and
should be moved somewhere else, like read-cache.c. However, the
function and its helpers made use of two global variables that made
straight code movement difficult:
* the_index
* include_sparse
The latter was perhaps more problematic since it was only accessible in
builtin/add.c but was still affecting builtin/checkout.c and
builtin/commit.c without this fact being very clear from the code. I'm
not sure if the other two callers would want to add a `--sparse` flag
similar to add.c to get non-default behavior, but exposing this
dependence will help if we ever decide we do want to add such a flag.
Modify add_files_to_cache() and its helpers to accept the necessary
arguments instead of relying on globals.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:45 +0000 (06:33 +0000)]
read-cache: move shared commit and ls-files code
The function overlay_tree_on_index(), plus associated helper functions,
were defined in builtin/ls-files.c, but also shared with
builtin/commit.c. Move these shared functions to read-cache.c.
Diff best viewed with `--color-moved`.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:43 +0000 (06:33 +0000)]
init-db, clone: change unnecessary global into passed parameter
Much like the parent commit, this commit was prompted by a desire to
move the functions which builtin/init-db.c and builtin/clone.c share out
of the former file and into setup.c. A secondary issue that made it
difficult was the init_shared_repository global variable; replace it
with a simple parameter that is passed to the relevant functions.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:42 +0000 (06:33 +0000)]
init-db: remove unnecessary global variable
This commit was prompted by a desire to move the functions which
builtin/init-db.c and builtin/clone.c share out of the former file and
into setup.c. One issue that made it difficult was the
init_is_bare_repository global variable.
init_is_bare_repository's sole use in life it to cache a value in
init_db(), and then be used in create_default_files(). This is a bit
odd since init_db() directly calls create_default_files(), and is the
only caller of that function. Convert the global to a simple function
parameter instead.
(Of course, this doesn't fix the fact that this value is then ignored by
create_default_files(), as noted in a big TODO comment in that function,
but it at least includes no behavioral change other than getting rid of
a very questionable global variable.)
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 16 May 2023 06:33:41 +0000 (06:33 +0000)]
init-db: document existing bug with core.bare in template config
The comments in create_default_files() talks about reading config from
the config file in the specified `--templates` directory, which leads to
the question of whether core.bare could be set in such a config file and
thus whether the code is doing the right thing. It turns out, that it
doesn't; it unconditionally ignores core.bare in the config file in any
--templates directory. It is not clear to me that fixing it can be done
within this function; it seems to occur too late:
* create_default_files() is called by init_db()
* init_db() is called by both builtin/{clone.c,init-db.c}
* both callers of init_db() call set_git_work_tree() before init_db()
and in order to actual affect whether a repository is bear, we'd need to
somewhere reset these values, not just the is_bare_repository_cfg
setting.
I do not want to open this can of worms at this time; I'm trying to
clean up some headers, for which I need to move some functions, for
which I need to clean up some globals, and that's far enough down the
rabbit hole. So, simply document the issue with a careful TODO comment
and a few testcases.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teng Long [Sat, 27 May 2023 07:57:53 +0000 (15:57 +0800)]
notes.c: introduce "--[no-]stripspace" option
This commit introduces a new option "--[no-]stripspace" to git notes
append, git notes edit, and git notes add. This option allows users to
control whether the note message need to stripped out.
For the consideration of backward compatibility, let's look at the
behavior about "stripspace" in "git notes" command:
1. "Edit Message" case: using the default editor to edit the note
message.
In "edit" case, the edited message will always be stripped out, the
implementation which can be found in the "prepare_note_data()". In
addition, the "-c" option supports to reuse an existing blob as a
note message, then open the editor to make a further edition on it,
the edited message will be stripped.
This commit doesn't change the default behavior of "edit" case by
using an enum "notes_stripspace", only when "--no-stripspace" option
is specified, the note message will not be stripped out. If you do
not specify the option or you specify "--stripspace", clearly, the
note message will be stripped out.
2. "Assign Message" case: using the "-m"/"-F"/"-C" option to specify the
note message.
In "assign" case, when specify message by "-m" or "-F", the message
will be stripped out by default, but when specify message by "-C",
the message will be copied verbatim, in other word, the message will
not be stripped out. One more thing need to note is "the order of
the options matter", that is, if you specify "-C" before "-m" or
"-F", the reused message by "-C" will be stripped out together,
because everytime concat "-m" or "-F" message, the concated message
will be stripped together. Oppositely, if you specify "-m" or "-F"
before "-C", the reused message by "-C" will not be stripped out.
This commit doesn't change the default behavior of "assign" case by
extending the "stripspace" field in "struct note_msg", so we can
distinguish the different behavior of "-m"/"-F" and "-C" options
when we need to parse and concat the message.
Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teng Long [Sat, 27 May 2023 07:57:52 +0000 (15:57 +0800)]
notes.c: append separator instead of insert by pos
Rename "insert_separator" to "append_separator" and also remove the
"postion" argument, this serves two purpose:
The first is that when specifying more than one "-m" ( like "-F", etc)
to "git notes add" or "git notes append", the order of them matters,
which means we need to append the each separator and message in turn,
so we don't have to make the caller specify the position, the "append"
operation is enough and clear.
The second is that when we execute the "git notes append" subcommand,
we need to combine the "prev_note" and "current_note" to get the
final result. Before, we inserted a newline character at the beginning
of "current_note". Now, we will append a newline to the end of
"prev_note" instead, this will give the consisitent results.
Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When adding new notes or appending to an existing notes, we will
insert a blank line between the paragraphs, like:
$ git notes add -m foo -m bar
$ git notes show HEAD
foo
bar
The default behavour sometimes is not enough, the user may want
to use a custom delimiter between paragraphs, like when
specifying '-m', '-F', '-C', '-c' options. So this commit
introduce a new '--separator' option for 'git notes add' and
'git notes append', for example when executing:
$ git notes add -m foo -m bar --separator="-"
$ git notes show HEAD
foo
-
bar
a newline is added to the value given to --separator if it
does not end with one already. So when executing:
$ git notes add -m foo -m bar --separator="-"
and
$ export LF="
"
$ git notes add -m foo -m bar --separator="-$LF"
Both the two exections produce the same result.
The reason we use a "strbuf" array to concat but not "string_list", is
that the binary file content may contain '\0' in the middle, this will
cause the corrupt result if using a string to save.
Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teng Long [Sat, 27 May 2023 07:57:48 +0000 (15:57 +0800)]
notes.c: cleanup 'strbuf_grow' call in 'append_edit'
Let's cleanup the unnecessary 'strbuf_grow' call in 'append_edit'. This
"strbuf_grow(&d.buf, size + 1);" is prepared for insert a blank line if
needed, but actually when inserting, "strbuf_insertstr(&d.buf, 0,
"\n");" will do the "grow" for us.
348f199b (builtin-notes: Refactor handling of -F option to allow
combining -m and -F, 2010-02-13) added these to mimic the code
introduced by 2347fae5 (builtin-notes: Add "append" subcommand for
appending to note objects, 2010-02-13) that reads in previous note
before the message. And the resulting code with explicit sizing is
carried to this day.
In the context of reading an existing note in, exact sizing may have
made sense, but because the resulting note needs cleansing with
stripspace() when appending with this option, such an exact sizing
does not buy us all that much in practice.
It may help avoiding overallocation due to ALLOC_GROW() slop, but
nobody can feed so many long messages for it to matter from the
command line.
Signed-off-by: Teng Long <dyroneteng@gmail.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git [-c log.follow=true] log [--follow] ':(glob)f**'" used to barf.
* jk/log-follow-with-non-literal-pathspec:
diff: detect pathspec magic not supported by --follow
diff: factor out --follow pathspec check
pathspec: factor out magic-to-name function
The value of config.worktree is per-repository, but has been kept
in a singleton global variable per process. This has been OK as
most Git operations interacted with a single repository at a time,
but not right for operations like recursive "grep" that want to
access multiple repositories from a single process without forking.
The global variable has been eliminated and made into a member in
the per-repository data structure.
* vd/worktree-config-is-per-repository:
repository: move 'repository_format_worktree_config' to repo scope
config: pass 'repo' directly to 'config_with_options()'
config: use gitdir to get worktree config
Junio C Hamano [Tue, 20 Jun 2023 22:53:12 +0000 (15:53 -0700)]
Merge branch 'tb/submodule-null-deref-fix'
"git submodule" code trusted the data coming from the config (and
the in-tree .gitmodules file) too much without validating, leading
to NULL dereference if the user mucks with a repository (e.g.
submodule.<name>.url is removed). This has been corrected.
Junio C Hamano [Tue, 20 Jun 2023 22:53:12 +0000 (15:53 -0700)]
Merge branch 'jc/test-modernization-2'
Test style updates.
* jc/test-modernization-2:
t9400-git-cvsserver-server: modernize test format
t9200-git-cvsexportcommit: modernize test format
t9104-git-svn-follow-parent: modernize test format
t9100-git-svn-basic: modernize test format
t7700-repack: modernize test format
t7600-merge: modernize test format
t7508-status: modernize test format
t7201-co: modernize test format
t7111-reset-table: modernize test format
t7110-reset-merge: modernize test format
Junio C Hamano [Tue, 20 Jun 2023 22:53:12 +0000 (15:53 -0700)]
Merge branch 'jc/test-modernization'
* jc/test-modernization:
t7101-reset-empty-subdirs: modernize test format
t6050-replace: modernize test format
t5306-pack-nobase: modernize test format
t5303-pack-corruption-resilience: modernize test format
t5301-sliding-window: modernize test format
t5300-pack-object: modernize test format
t4206-log-follow-harder-copies: modernize test format
t4202-log: modernize test format
t4004-diff-rename-symlink: modernize test format
t4003-diff-rename-1: modernize test format
t4002-diff-basic: modernize test format
t3903-stash: modernize test format
t3700-add: modernize test format
t3500-cherry: modernize test format
t1006-cat-file: modernize test format
t1002-read-tree-m-u-2way: modernize test format
t1001-read-tree-m-2way: modernize test format
t3210-pack-refs: modernize test format
t0030-stripspace: modernize test format
t0000-basic: modernize test format
Document more pseudo-refs and teach the command line completion
machinery to complete AUTO_MERGE.
* pb/complete-and-document-auto-merge-and-friends:
completion: complete AUTO_MERGE
Documentation: document AUTO_MERGE
git-merge.txt: modernize word choice in "True merge" section
completion: complete REVERT_HEAD and BISECT_HEAD
revisions.txt: document more special refs
revisions.txt: use description list for special refs
Junio C Hamano [Tue, 20 Jun 2023 22:53:10 +0000 (15:53 -0700)]
Merge branch 'ps/fetch-cleanups'
Code clean-up.
* ps/fetch-cleanups:
fetch: use `fetch_config` to store "submodule.fetchJobs" value
fetch: use `fetch_config` to store "fetch.parallel" value
fetch: use `fetch_config` to store "fetch.recurseSubmodules" value
fetch: use `fetch_config` to store "fetch.showForcedUpdates" value
fetch: use `fetch_config` to store "fetch.pruneTags" value
fetch: use `fetch_config` to store "fetch.prune" value
fetch: pass through `fetch_config` directly
fetch: drop unneeded NULL-check for `remote_ref`
fetch: drop unused DISPLAY_FORMAT_UNKNOWN enum value
Derrick Stolee [Tue, 20 Jun 2023 19:01:15 +0000 (19:01 +0000)]
packfile: delete .idx files before .pack files
When installing a packfile, we place the .pack file before the .idx
file. The intention is that Git scans for .idx files in the pack
directory and then loads the .pack files from that list.
However, when we delete packfiles, we do not do this in the reverse
order as we should. The unlink_pack_path() method deletes the .pack
followed by the .idx.
This creates a window where the process could be interrupted between
the .pack deletion and the .idx deletion, leaving the repository in a
state that looks strange, but isn't actually too problematic if we
assume the pack was safe to delete. The .idx without a .pack will cause
some overhead, but will not interrupt other Git processes.
This ordering was introduced into the 'git repack' builtin by a1bbc6c0176 (repack: rewrite the shell script in C, 2013-09-15), though
we must be careful to track history through the code move in 8434e85d5f9
(repack: refactor pack deletion for future use, 2019-06-10) to see that.
This became more important after 73320e49add (builtin/repack.c: only
collect fully-formed packs, 2023-06-07) changed how 'git repack' scanned
for packfiles for use in the cruft pack process. It previously looked
for .pack files, but that was problematic due to the order that packs
are installed: repacks between the creation of a .pack and the creation
of its .idx would result in hard failures.
There is an independent proposal about what to do in the case of a .idx
without a .pack during this 'git repack' scenario, but this change is
focused on deleting .pack files more safely.
Modify the order to delete the .idx before the .pack. The rest of the
modifiers on the .pack should still come after the .pack so we know all
of the presumed properties of the packfile as long as it exists in the
filesystem, in case we wish to reinstate it by re-indexing the .pack
file.
Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sat, 17 Jun 2023 20:43:17 +0000 (22:43 +0200)]
replace strbuf_expand() with strbuf_expand_step()
Avoid the overhead of passing context to a callback function of
strbuf_expand() by using strbuf_expand_step() in a loop instead. It
requires explicit handling of %% and unrecognized placeholders, but is
simpler, more direct and avoids void pointers.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sat, 17 Jun 2023 20:42:26 +0000 (22:42 +0200)]
replace strbuf_expand_dict_cb() with strbuf_expand_step()
Avoid the overhead of setting up a dictionary and passing it via
strbuf_expand() to strbuf_expand_dict_cb() by using strbuf_expand_step()
in a loop instead. It requires explicit handling of %% and unrecognized
placeholders, but is more direct and simpler overall, and expands only
on demand.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sat, 17 Jun 2023 20:41:44 +0000 (22:41 +0200)]
strbuf: factor out strbuf_expand_step()
Extract the part of strbuf_expand that finds the next placeholder into a
new function. It allows to build parsers without callback functions and
the overhead imposed by them.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sat, 17 Jun 2023 05:15:59 +0000 (01:15 -0400)]
http: handle both "h2" and "h2h3" in curl info lines
When redacting auth tokens in trace output from curl, we look for http/2
headers of the form "h2h3 [header: value]". This comes from b637a41ebe
(http: redact curl h2h3 headers in info, 2022-11-11).
But the "h2h3" prefix changed to just "h2" in curl's fc2f1e547 (http2:
support HTTP/2 to forward proxies, non-tunneling, 2023-04-14). That's in
released version curl 8.1.0; linking against that version means we'll
fail to correctly redact the trace. Our t5559.17 notices and fails.
We can fix this by matching either prefix, which should handle both old
and new versions.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Let's mark them with "TEST_PASSES_SANITIZE_LEAK=true" to notice and fix
promptly any new leak that may be introduced and triggered by them in
the future.
Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rubén Justo [Sat, 17 Jun 2023 06:41:40 +0000 (08:41 +0200)]
config: fix a leak in git_config_copy_or_rename_section_in_file
A branch can have its configuration spread over several configuration
sections. This situation was already foreseen in 52d59cc645 (branch:
add a --copy (-c) option to go with --move (-m), 2017-06-18) when
"branch -c" was introduced.
Unfortunately, a leak was also introduced:
$ git branch foo
$ cat >> .git/config <<EOF
[branch "foo"]
some-key-a = a value
[branch "foo"]
some-key-b = b value
[branch "foo"]
some-key-c = c value
EOF
$ git branch -c foo bar
Direct leak of 130 byte(s) in 2 object(s) allocated from:
... in xrealloc wrapper.c
... in strbuf_grow strbuf.c
... in strbuf_vaddf strbuf.c
... in strbuf_addf strbuf.c
... in store_create_section config.c
... in git_config_copy_or_rename_section_in_file config.c
... in git_config_copy_section_in_file config.c
... in git_config_copy_section config.c
... in copy_or_rename_branch builtin/branch.c
... in cmd_branch builtin/branch.c
... in run_builtin git.c
Let's fix it.
Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rubén Justo [Sat, 17 Jun 2023 06:41:22 +0000 (08:41 +0200)]
branch: fix a leak in cmd_branch
In 98e7ab6d42 (for-each-ref: delay parsing of --sort=<atom> options,
2021-10-20) a new string_list was introduced to accumulate any
"branch.sort" setting.
That string_list is cleared in ref_sorting_options(), which is only
called when processing the "--list" sub-command. Therefore, with other
sub-command, while having any sort option set, a leak is produced, e.g.:
Direct leak of 384 byte(s) in 1 object(s) allocated from:
... in xrealloc wrapper.c
... in string_list_append_nodup string-list.c
... in string_list_append string-list.c
... in git_branch_config builtin/branch.c
... in configset_iter config.c
... in repo_config config.c
... in git_config config.c
... in cmd_branch builtin/branch.c
... in run_builtin git.c
Indirect leak of 20 byte(s) in 1 object(s) allocated from:
... in xstrdup wrapper.c
... in string_list_append string-list.c
... in git_branch_config builtin/branch.c
... in configset_iter config.c
... in repo_config config.c
... in git_config config.c
... in cmd_branch builtin/branch.c
... in run_builtin git.c
We don't have a common clean-up section in cmd_branch(). To avoid
refactoring, keep the fix simple, and while we find a better solution
which hopefuly will avoid entirely that string_list, when no sort
options are needed; let's squelch the leak sanitizer using UNLEAK().
Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rubén Justo [Sat, 17 Jun 2023 06:41:08 +0000 (08:41 +0200)]
branch: fix a leak in setup_tracking
In bdaf1dfae7 (branch: new autosetupmerge option "simple" for matching
branches, 2022-04-29) a new exit for setup_tracking() missed the
clean-up, producing a leak.
$ git config branch.autoSetupMerge simple
$ git remote add local .
$ git update-ref refs/remotes/local/foo HEAD
$ git branch bar local/foo
Direct leak of 384 byte(s) in 1 object(s) allocated from:
... in xrealloc wrapper.c
... in string_list_append_nodup string-list.c
... in find_tracked_branch branch.c
... in for_each_remote remote.c
... in setup_tracking branch.c
... in create_branch branch.c
... in cmd_branch builtinbranch.c
... in run_builtin git.c
Indirect leak of 24 byte(s) in 1 object(s) allocated from:
... in xrealloc wrapper.c
... in strbuf_grow strbuf.c
... in strbuf_add strbuf.c
... in match_name_with_pattern remote.c
... in query_refspecs remote.c
... in remote_find_tracking remote.c
... in find_tracked_branch branch.c
... in for_each_remote remote.c
... in setup_tracking branch.c
... in create_branch branch.c
... in cmd_branch builtinbranch.c
... in run_builtin git.c
The return introduced in bdaf1dfae7 was to avoid setting up the
tracking, but even in that case it is still necessary to do the
clean-up. Let's do it.
Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rubén Justo [Sat, 17 Jun 2023 06:40:43 +0000 (08:40 +0200)]
rev-parse: fix a leak with --abbrev-ref
To handle "--abbrev-ref" we use shorten_unambiguous_ref(). This
function takes a refname and returns a shortened refname, which is a
newly allocated string that needs to be freed.
Unfortunately, the refname variable is reused to receive the shortened
one. Therefore, we lose the original refname, which needs to be freed
as well, producing a leak.
This leak can be reviewed with:
$ for a in {1..10}; do git branch foo_${a}; done
$ git rev-parse --abbrev-ref refs/heads/foo_{1..10}
Direct leak of 171 byte(s) in 10 object(s) allocated from:
... in xstrdup wrapper.c
... in expand_ref refs.c
... in repo_dwim_ref refs.c
... in show_rev builtin/rev-parse.c
... in cmd_rev_parse builtin/rev-parse.c
... in run_builtin git.c
We have this leak since a45d34691e (rev-parse: --abbrev-ref option to
shorten ref name, 2009-04-13) when "--abbrev-ref" was introduced.
Let's fix it.
Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sat, 17 Jun 2023 04:26:24 +0000 (00:26 -0400)]
commit: pass --no-divider to interpret-trailers
When git-commit sees any "--trailer" options, it passes the
COMMIT_EDITMSG file through git-interpret-trailers. But it does so
without passing --no-divider, which means that interpret-trailers will
look for a "---" divider to signal the end of the commit message.
That behavior doesn't make any sense in this context; we know we have a
complete and solitary commit message, not something we have to further
parse. And as a result, we'll do the wrong thing if the commit message
contains a "---" marker (which otherwise is not syntactically
significant), inserting any new trailers at the wrong spot.
We can fix this by passing --no-divider. This is the exact situation for
which it was added in 1688c9a489 (interpret-trailers: allow suppressing
"---" divider, 2018-08-22). As noted in the message for that commit, it
just adds the mechanism, and further patches were needed to trigger it
from various callers. We did that back then in a few spots, like ffce7f590f (sequencer: ignore "---" divider when parsing trailers,
2018-08-22), but obviously missed this one.
Reported-by: <eric.frederich@siemens.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
M Hickford [Fri, 16 Jun 2023 19:55:06 +0000 (19:55 +0000)]
credential/libsecret: store new attributes
d208bfd (credential: new attribute password_expiry_utc, 2023-02-18)
and a5c76569e7 (credential: new attribute oauth_refresh_token)
introduced new credential attributes.
libsecret assumes attribute values are non-confidential and
unchanging, so we encode the new attributes in the secret, separated by
newline:
This is extensible and backwards compatible. The credential protocol
already assumes that attribute values do not contain newlines.
Alternatives considered: store password_expiry_utc in a libsecret
attribute. This has the problem that libsecret creates new items
rather than overwrites when attribute values change.
Signed-off-by: M Hickford <mirth.hickford@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Glen Choo [Wed, 14 Jun 2023 19:35:58 +0000 (19:35 +0000)]
setup.c: don't setup in discover_git_directory()
discover_git_directory() started modifying the_repository in ebaf3bcf1ae
(repository: move global r_f_p_c to repo struct, 2021-06-17), when, in
the repository setup process, we started copying members from the
"struct repository_format" we're inspecting to the appropriate "struct
repository". However, discover_git_directory() isn't actually used in
the setup process (its only caller in the Git binary is
read_early_config()), so it shouldn't be doing this setup at all!
As explained by 16ac8b8db6 (setup: introduce the
discover_git_directory() function, 2017-03-13) and the comment on its
declaration, discover_git_directory() is intended to be an entrypoint
into setup.c machinery that allows the Git directory to be discovered
without side effects, e.g. so that read_early_config() can read
".git/config" before the_repository has been set up.
Fortunately, we didn't start to rely on this unintended behavior between
then and now, so we let's just remove it. It isn't harming anyone, but
it's confusing.
Signed-off-by: Glen Choo <chooglen@google.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
M Hickford [Thu, 15 Jun 2023 19:19:33 +0000 (19:19 +0000)]
credential: erase all matching credentials
`credential reject` sends the erase action to each helper, but the
exact behaviour of erase isn't specified in documentation or tests.
Some helpers (such as credential-store and credential-libsecret) delete
all matching credentials, others (such as credential-cache) delete at
most one matching credential.
Test that helpers erase all matching credentials. This behaviour is
easiest to reason about. Users expect that `echo
"url=https://example.com" | git credential reject` or `echo
"url=https://example.com\nusername=tim" | git credential reject` erase
all matching credentials.
Fix credential-cache.
Signed-off-by: M Hickford <mirth.hickford@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
While both git-rev-list(1) and git-log(1) support `--stdin`, it only
accepts commits and files. Most notably, it is impossible to pass any of
the pseudo-opts like `--all`, `--glob=` or others via stdin.
This makes it hard to use this function in certain scripted scenarios,
like when one wants to support queries against specific revisions, but
also against reference patterns. While this is theoretically possible by
using arguments, this may run into issues once we hit platform limits
with sufficiently large queries. And because `--stdin` cannot handle
pseudo-opts, the only alternative would be to use a mixture of arguments
and standard input, which is cumbersome.
Implement support for handling pseudo-opts in both commands to support
this usecase better. One notable restriction here is that `--stdin` only
supports "stuck" arguments in the form of `--glob=foo`. This is because
"unstuck" arguments would also require us to read the next line, which
would add quite some complexity to the code. This restriction should be
fine for scripted usage though.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
revision: small readability improvement for reading from stdin
The code that reads lines from standard input manually compares whether
the read line matches "--", which is a bit awkward to read. Furthermore,
we're about to extend the code to also support reading pseudo-options
via standard input, requiring more elaborate handling of lines with a
leading dash.
Refactor the code by hoisting out the check for "--" outside of the
block that checks for a leading dash.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Thu, 15 Jun 2023 17:33:39 +0000 (19:33 +0200)]
ls-tree: fix documentation of %x format placeholder
ls-tree --format expands %x followed by two hexadecimal digits to the
character indicated by that hexadecimal number, e.g.:
$ git ls-tree --format=%x41 HEAD | head -1
A
It rejects % followed by a hexadecimal digit, e.g.:
$ git ls-tree --format=%41 HEAD | head -1
fatal: bad ls-tree format: element '41' does not start with '('
This functionality is provided by strbuf_expand_literal_cb(), which has
not been changed since it was factored out by fd2015b323 (strbuf:
separate callback for strbuf_expand:ing literals, 2019-01-28).
Adjust the documentation accordingly.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 15 Jun 2023 02:53:50 +0000 (02:53 +0000)]
doc: trailer: add more examples in DESCRIPTION
Be more up-front about what trailers are in practice with examples, to
give the reader a visual cue while they go on to read the rest of the
description.
Also add an example for multiline values.
Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 15 Jun 2023 02:53:45 +0000 (02:53 +0000)]
doc: trailer: examples: avoid the word "message" by itself
Previously, "message" could mean the input, output, commit message, or
"internal body text inside the commit message" (in the EXAMPLES
section). Avoid overloading this term by using the appropriate meanings
explicitly.
Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Linus Arver [Thu, 15 Jun 2023 02:53:44 +0000 (02:53 +0000)]
doc: trailer: drop "commit message part" phrasing
The command can take inputs that are either just a commit message, or
an email-like output such as git-format-patch which includes a commit
message, "---" divider, and patch part. The existing explanation blends
these two inputs together in the first sentence
This command reads some patches or commit messages
which then necessitates using the "commit message part" phrasing (as
opposed to just "commit message") because the input is ambiguous per the
above definition.
This change separates the two input types and explains them separately,
and so there is no longer a need to use the "commit message part"
phrase.
Signed-off-by: Linus Arver <linusa@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan [Wed, 14 Jun 2023 21:31:45 +0000 (14:31 -0700)]
CodingGuidelines: use octal escapes, not hex
Extend the shell-scripting section of CodingGuidelines to suggest octal
escape sequences (e.g. "\302\242") over hexadecimal (e.g. "\xc2\xa2")
since the latter can be a source of portability problems.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Josip Sokcevic [Wed, 14 Jun 2023 16:48:57 +0000 (09:48 -0700)]
diff-lib: honor override_submodule_config flag bit
When `diff.ignoreSubmodules = all` is set and submodule commits are
manually staged (e.g. via `git-update-index`), `git-commit` should
record the commit with updated submodules.
`index_differs_from` is called from `prepare_to_commit` with flags set to
`override_submodule_config = 1`. `index_differs_from` then merges the
default diff flags and passed flags.
When `diff.ignoreSubmodules` is set to "all", `flags` ends up having
both `override_submodule_config` and `ignore_submodules` set to 1. This
results in `git-commit` ignoring staged commits.
This patch restores original `flags.ignore_submodule` if
`flags.override_submodule_config` is set.
Signed-off-by: Josip Sokcevic <sokcevic@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Tue, 13 Jun 2023 19:29:46 +0000 (12:29 -0700)]
Merge branch 'zh/ls-files-format-atoms'
Some atoms that can be used in "--format=<format>" for "git ls-tree"
were not supported by "git ls-files", even though they were relevant
in the context of the latter.
* zh/ls-files-format-atoms:
ls-files: align format atoms with ls-tree
Junio C Hamano [Tue, 13 Jun 2023 19:29:45 +0000 (12:29 -0700)]
Merge branch 'jc/pack-ref-exclude-include'
"git pack-refs" learns "--include" and "--exclude" to tweak the ref
hierarchy to be packed using pattern matching.
* jc/pack-ref-exclude-include:
pack-refs: teach pack-refs --include option
pack-refs: teach --exclude option to exclude refs from being packed
docs: clarify git-pack-refs --all will pack all refs
Junio C Hamano [Tue, 13 Jun 2023 19:29:45 +0000 (12:29 -0700)]
Merge branch 'sa/doc-ls-remote'
Doc update.
* sa/doc-ls-remote:
ls-remote doc: document the output format
ls-remote doc: explain what each example does
ls-remote doc: show peeled tags in examples
ls-remote doc: remove redundant --tags example
show-branch doc: say <ref>, not <reference>
show-ref doc: update for internal consistency
Junio C Hamano [Tue, 13 Jun 2023 19:29:44 +0000 (12:29 -0700)]
Merge branch 'jc/diff-s-with-other-options'
The "-s" (silent, squelch) option of the "diff" family of commands
did not interact with other options that specify the output format
well. This has been cleaned up so that it will clear all the
formatting options given before.
* jc/diff-s-with-other-options:
diff: fix interaction between the "-s" option and other options
Junio C Hamano [Tue, 13 Jun 2023 19:29:44 +0000 (12:29 -0700)]
Merge branch 'kh/keep-tag-editmsg-upon-failure'
"git tag" learned to leave the "$GIT_DIR/TAG_EDITMSG" file when the
command failed, so that the user can salvage what they typed.
* kh/keep-tag-editmsg-upon-failure:
tag: keep the message file in case ref transaction fails
t/t7004-tag: add regression test for successful tag creation
doc: tag: document `TAG_EDITMSG`
Rubén Justo [Sun, 11 Jun 2023 18:50:36 +0000 (20:50 +0200)]
branch: fix a leak in setup_tracking
The commit d3115660b4 (branch: add flags and config to inherit tracking,
2021-12-20) replaced in "struct tracking", the member "char *src" by a
new "struct string_list *srcs".
This caused a modification in find_tracked_branch(). The string
returned by remote_find_tracking(), previously assigned to "src", is now
added to the string_list "srcs".
That string_list is initialized with STRING_LIST_INIT_DUP, which means
that what is added is not the given string, but a duplicate. Therefore,
the string returned by remote_find_tracking() is leaked.
The leak can be reviewed with:
$ git branch foo
$ git remote add local .
$ git fetch local
$ git branch --track bar local/foo
Direct leak of 24 byte(s) in 1 object(s) allocated from:
... in xrealloc wrapper.c
... in strbuf_grow strbuf.c
... in strbuf_add strbuf.c
... in match_name_with_pattern remote.c
... in query_refspecs remote.c
... in remote_find_tracking remote.c
... in find_tracked_branch branch.c
... in for_each_remote remote.c
... in setup_tracking branch.c
... in create_branch branch.c
... in cmd_branch builtin/branch.c
... in run_builtin git.c
Let's fix the leak, using the "_nodup" API to add to the string_list.
This way, the string itself will be added instead of being strdup()'d.
And when the string_list is cleared, the string will be freed.
Signed-off-by: Rubén Justo <rjusto@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>