Commit graph

10967 commits

Author SHA1 Message Date
Derrick Stolee
7cf3b49f47 add: check color.ui for interactive add
When 'git add -i' and 'git add -p' were converted to a builtin, they
introduced a color bug: the 'color.ui' config setting is ignored.

The included test demonstrates an example that is similar to the
previous test, which focuses on customizing colors. Here, we are
demonstrating that colors are not being used at all by comparing the raw
output and the color-decoded version of that output.

The fix is simple, to use git_color_default_config() as the fallback for
git_add_config(). A more robust change would instead encapsulate the
git_use_color_default global in methods that would check the config
setting if it has not been initialized yet. Some ideas are being
discussed on this front [1], but nothing has been finalized.

[1] https://lore.kernel.org/git/pull.1539.git.1685716420.gitgitgadget@gmail.com/

This test case naturally bisects down to 0527ccb1b5 (add -i: default to
the built-in implementation, 2021-11-30), but the fix makes it clear
that this would be broken even if we added the config to use the builtin
earlier than this.

Reported-by: Greg Alexander <gitgreg@galexander.org>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12 10:49:16 -07:00
Junio C Hamano
630501ceef Merge branch 'jc/countermand-format-attach'
The format.attach configuration variable lacked a way to override a
value defined in a lower-priority configuration file (e.g. the
system one) by redefining it in a higher-priority configuration
file.  Now, setting format.attach to an empty string means show the
patch inline in the e-mail message, without using MIME attachment.

This is a backward incompatible change.

* jc/countermand-format-attach:
  format.attach: allow empty value to disable multi-part messages
2023-02-27 10:08:57 -08:00
Junio C Hamano
7dc55a04d8 Merge branch 'mh/credential-password-expiry'
The credential subsystem learned that a password may have an
explicit expiration.

* mh/credential-password-expiry:
  credential: new attribute password_expiry_utc
2023-02-27 10:08:57 -08:00
Junio C Hamano
d180cc2979 Merge branch 'ma/fetch-parallel-use-online-cpus'
"git fetch --jobs=0" used to hit a BUG(), which has been corrected
to use the available CPUs.

* ma/fetch-parallel-use-online-cpus:
  fetch: choose a sensible default with --jobs=0 again
2023-02-24 22:54:00 -08:00
M Hickford
d208bfdfef credential: new attribute password_expiry_utc
Some passwords have an expiry date known at generation. This may be
years away for a personal access token or hours for an OAuth access
token.

When multiple credential helpers are configured, `credential fill` tries
each helper in turn until it has a username and password, returning
early. If Git authentication succeeds, `credential approve`
stores the successful credential in all helpers. If authentication
fails, `credential reject` erases matching credentials in all helpers.
Helpers implement corresponding operations: get, store, erase.

The credential protocol has no expiry attribute, so helpers cannot
store expiry information. Even if a helper returned an improvised
expiry attribute, git credential discards unrecognised attributes
between operations and between helpers.

This is a particular issue when a storage helper and a
credential-generating helper are configured together:

	[credential]
		helper = storage  # eg. cache or osxkeychain
		helper = generate  # eg. oauth

`credential approve` stores the generated credential in both helpers
without expiry information. Later `credential fill` may return an
expired credential from storage. There is no workaround, no matter how
clever the second helper. The user sees authentication fail (a retry
will succeed).

Introduce a password expiry attribute. In `credential fill`, ignore
expired passwords and continue to query subsequent helpers.

In the example above, `credential fill` ignores the expired password
and a fresh credential is generated. If authentication succeeds,
`credential approve` replaces the expired password in storage.
If authentication fails, the expired credential is erased by
`credential reject`. It is unnecessary but harmless for storage
helpers to self prune expired credentials.

Add support for the new attribute to credential-cache.
Eventually, I hope to see support in other popular storage helpers.

Example usage in a credential-generating helper
https://github.com/hickford/git-credential-oauth/pull/16

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
Reviewed-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-22 15:18:58 -08:00
Junio C Hamano
5048df67b2 Merge branch 'ab/hook-api-with-stdin'
Extend the run-hooks API to allow feeding data from the standard
input when running the hook script(s).

* ab/hook-api-with-stdin:
  hook: support a --to-stdin=<path> option
  sequencer: use the new hook API for the simpler "post-rewrite" call
  hook API: support passing stdin to hooks, convert am's 'post-rewrite'
  run-command: allow stdin for run_processes_parallel
  run-command.c: remove dead assignment in while-loop
2023-02-22 14:55:45 -08:00
Junio C Hamano
72972ea0b9 Merge branch 'ab/various-leak-fixes'
Leak fixes.

* ab/various-leak-fixes:
  push: free_refs() the "local_refs" in set_refspecs()
  push: refactor refspec_append_mapped() for subsequent leak-fix
  receive-pack: release the linked "struct command *" list
  grep API: plug memory leaks by freeing "header_list"
  grep.c: refactor free_grep_patterns()
  builtin/merge.c: free "&buf" on "Your local changes..." error
  builtin/merge.c: use fixed strings, not "strbuf", fix leak
  show-branch: free() allocated "head" before return
  commit-graph: fix a parse_options_concat() leak
  http-backend.c: fix cmd_main() memory leak, refactor reg{exec,free}()
  http-backend.c: fix "dir" and "cmd_arg" leaks in cmd_main()
  worktree: fix a trivial leak in prune_worktrees()
  repack: fix leaks on error with "goto cleanup"
  name-rev: don't xstrdup() an already dup'd string
  various: add missing clear_pathspec(), fix leaks
  clone: use free() instead of UNLEAK()
  commit-graph: use free_commit_graph() instead of UNLEAK()
  bundle.c: don't leak the "args" in the "struct child_process"
  tests: mark tests as passing with SANITIZE=leak
2023-02-22 14:55:45 -08:00
Junio C Hamano
6aac634f81 Merge branch 'jk/doc-ls-remote-matching'
Doc update.

* jk/doc-ls-remote-matching:
  doc/ls-remote: clarify pattern format
  doc/ls-remote: cosmetic cleanups for examples
2023-02-22 14:55:45 -08:00
Junio C Hamano
24fb150dcd Merge branch 'ab/the-index-compatibility'
Remove more remaining uses of macros that relies on the_index
singleton instance without explicitly spelling it out.

* ab/the-index-compatibility:
  cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS"
  cache-tree API: remove redundant update_main_cache_tree()
  cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*"
  cocci & cache.h: apply pending "index_cache_pos" rule
  cocci & cache.h: fully apply "active_nr" part of index-compatibility
  builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"
2023-02-22 14:55:44 -08:00
Junio C Hamano
5fc6d00b65 Merge branch 'en/name-rev-make-taggerdate-much-less-important'
"git name-rev" heuristics update.

* en/name-rev-make-taggerdate-much-less-important:
  name-rev: fix names by dropping taggerdate workaround
2023-02-22 14:55:44 -08:00
Matthias Aßhauer
c39952b925 fetch: choose a sensible default with --jobs=0 again
prior to 51243f9 (run-command API: don't fall back on online_cpus(),
2022-10-12) `git fetch --multiple --jobs=0` would choose some default amount
of jobs, similar to `git -c fetch.parallel=0 fetch --multiple`. While our
documentation only ever promised that `fetch.parallel` would fall back to a
"sensible default", it makes sense to do the same for `--jobs`. So fall back
to online_cpus() and not BUG() out.

This fixes https://github.com/git-for-windows/git/issues/4302

Reported-by: Drew Noakes <drnoakes@microsoft.com>
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-21 12:09:40 -08:00
Junio C Hamano
50bebf98d9 format.attach: allow empty value to disable multi-part messages
When a lower precedence configuration file (e.g. /etc/gitconfig)
defines format.attach in any way, there was no way to disable it in
a more specific configuration file (e.g. $HOME/.gitconfig).

Change the behaviour of setting it to an empty string.  It used to
mean that the result is still a multipart message with only dashes
used as a multi-part separator, but now it resets the setting to
the default (which would be to give an inline patch, unless other
command line options are in effect).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-17 15:43:09 -08:00
Junio C Hamano
06bca9708a Merge branch 'ab/retire-scripted-add-p'
Finally retire the scripted "git add -p/-i" implementation and have
everybody use the one reimplemented in C.

* ab/retire-scripted-add-p:
  docs & comments: replace mentions of "git-add--interactive.perl"
  add API: remove run_add_interactive() wrapper function
  add: remove "add.interactive.useBuiltin" & Perl "git add--interactive"
2023-02-15 17:11:53 -08:00
Junio C Hamano
c5f7b2a6fe Merge branch 'rs/size-t-fixes'
Type fixes.

* rs/size-t-fixes:
  pack-objects: use strcspn(3) in name_cmp_len()
  read-cache: use size_t for {base,df}_name_compare()
2023-02-15 17:11:53 -08:00
Junio C Hamano
a232de58f2 Merge branch 'ab/sequencer-unleak'
Plug leaks in sequencer subsystem and its users.

* ab/sequencer-unleak:
  commit.c: free() revs.commit in get_fork_point()
  builtin/rebase.c: free() "options.strategy_opts"
  sequencer.c: always free() the "msgbuf" in do_pick_commit()
  builtin/rebase.c: fix "options.onto_name" leak
  builtin/revert.c: move free-ing of "revs" to replay_opts_release()
  sequencer API users: fix get_replay_opts() leaks
  sequencer.c: split up sequencer_remove_state()
  rebase: use "cleanup" pattern in do_interactive_rebase()
2023-02-15 17:11:52 -08:00
Junio C Hamano
4f59836451 Merge branch 'ds/bundle-uri-5'
The bundle-URI subsystem adds support for creation-token heuristics
to help incremental fetches.

* ds/bundle-uri-5:
  bundle-uri: test missing bundles with heuristic
  bundle-uri: store fetch.bundleCreationToken
  fetch: fetch from an external bundle URI
  bundle-uri: drop bundle.flag from design doc
  clone: set fetch.bundleURI if appropriate
  bundle-uri: download in creationToken order
  bundle-uri: parse bundle.<id>.creationToken values
  bundle-uri: parse bundle.heuristic=creationToken
  t5558: add tests for creationToken heuristic
  bundle: verify using check_connected()
  bundle: test unbundling with incomplete history
2023-02-15 17:11:52 -08:00
Junio C Hamano
c867e4fa18 Sync with Git 2.39.2 2023-02-13 17:03:55 -08:00
Jeff King
d9ec3b0dc0 doc/ls-remote: clarify pattern format
We document that you can specify "refs" to ls-remote, but we don't
explain any further than that they are "matched" as patterns. Since this
can be interpreted in a lot of ways, let's clarify that they are
tail-matched globs.

Likewise, let's use the word "patterns" to refer to them consistently,
rather than "refs" (both here and in the quick "-h" help), and mention
more explicitly that only one pattern needs to be matched (though there
is also an example already that shows this in action).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-10 21:57:51 -08:00
Ævar Arnfjörð Bjarmason
dfd0a89374 cocci & cache.h: remove "USE_THE_INDEX_COMPATIBILITY_MACROS"
Have the last users of "USE_THE_INDEX_COMPATIBILITY_MACROS" use the
underlying *_index() variants instead. Now all previous users of
"USE_THE_INDEX_COMPATIBILITY_MACROS" have been migrated away from the
wrapper macros, and if applicable to use the "USE_THE_INDEX_VARIABLE"
added in [1].

Let's leave the "index-compatibility.cocci" in place, even though it
won't be doing anything on "master". It will benefit any out-of-tree
code that need to use these compatibility macros. We can eventually
remove it.

1. bdafeae0b9 (cache.h & test-tool.h: add & use
   "USE_THE_INDEX_VARIABLE", 2022-11-19)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-10 11:38:40 -08:00
Ævar Arnfjörð Bjarmason
fcb864bce7 cache-tree API: remove redundant update_main_cache_tree()
Remove the redundant update_main_cache_tree() function, and make its
users use cache_tree_update() instead.

The behavior of populating the "the_index.cache_tree" if it wasn't
present already was needed when this function was introduced in [1],
but it hasn't been needed since [2]; The "cache_tree_update()" will
now lazy-allocate, so there's no need for the wrapper.

1. 996277c520 (Refactor cache_tree_update idiom from commit,
   2011-12-06)
2. fb0882648e (cache-tree: clean up cache_tree_update(), 2021-01-23)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-10 11:38:14 -08:00
Ævar Arnfjörð Bjarmason
99370863e2 cocci & cache-tree.h: migrate "write_cache_as_tree" to "*_index_*"
Add a trivial rule for "write_cache_as_tree" to
"index-compatibility.cocci", and apply it. This was left out of the
rules added in 0e6550a2c6 (cocci: add a
index-compatibility.pending.cocci, 2022-11-19) because this
compatibility wrapper lived in "cache-tree.h", not "cache.h"

But it's like the other "USE_THE_INDEX_COMPATIBILITY_MACROS", so let's
migrate it too.

The replacement of "USE_THE_INDEX_COMPATIBILITY_MACROS" here with
"USE_THE_INDEX_VARIABLE" is a manual change on top, now that these
files only use "&the_index", and don't need any compatibility
macros (or functions).

The wrapping of some argument lists is likewise manual, as coccinelle
would otherwise give us overly long argument lists.

The reason for putting the "O" in the cocci rule on the "-" and "+"
lines is because I couldn't get correct whitespacing otherwise,
i.e. I'd end up with "oid,&the_index", not "oid, &the_index".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-10 11:37:49 -08:00
Ævar Arnfjörð Bjarmason
babed893f5 cocci & cache.h: apply pending "index_cache_pos" rule
Apply the rule added in [1] to change "cache_name_pos" to
"index_name_pos", which allows us to get rid of another
"USE_THE_INDEX_COMPATIBILITY_MACROS" macro.

The replacement of "USE_THE_INDEX_COMPATIBILITY_MACROS" here with
"USE_THE_INDEX_VARIABLE" is a manual change on top, now that these
files only use "&the_index", and don't need any compatibility
macros (or functions).

1. 0e6550a2c6 (cocci: add a index-compatibility.pending.cocci,
   2022-11-19)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-10 11:37:27 -08:00
Ævar Arnfjörð Bjarmason
cec13b9514 cocci & cache.h: fully apply "active_nr" part of index-compatibility
Apply the "active_nr" part of "index-compatibility.pending.cocci",
which was left out in [1] due to an in-flight conflict. As of [2] the
topic we conflicted with has been merged to "master", so we can fully
apply this rule.

1. dc594180d9 (cocci & cache.h: apply variable section of "pending"
   index-compatibility, 2022-11-19)
2. 9ea1378d04 (Merge branch 'ab/various-leak-fixes', 2022-12-14)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-10 11:31:18 -08:00
Ævar Arnfjörð Bjarmason
6193aaa9f9 builtin/rm.c: use narrower "USE_THE_INDEX_VARIABLE"
Replace the "USE_THE_INDEX_COMPATIBILITY_MACROS" define with the
narrower "USE_THE_INDEX_VARIABLE". This could have been done in
07047d6829 (cocci: apply "pending" index-compatibility to some
"builtin/*.c", 2022-11-19), but I missed it at the time.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-10 11:31:16 -08:00
Junio C Hamano
6d1b2e48fe Merge branch 'ew/free-island-marks'
"git pack-objects" learned to release delta-island bitmap data when
it is done using it, saving peak heap memory usage.

* ew/free-island-marks:
  delta-islands: free island_marks and bitmaps
2023-02-09 14:40:47 -08:00
Elijah Newren
b2182a8730 name-rev: fix names by dropping taggerdate workaround
Commit 7550424804 ("name-rev: include taggerdate in considering the best
name", 2016-04-22) introduced the idea of using taggerdate in the
criteria for selecting the best name.  At the time, a certain commit in
linux.git -- namely, aed06b9cfcab -- was being named by name-rev as
    v4.6-rc1~9^2~792
which, while correct, was very suboptimal.  Some investigation found
that tweaking the MERGE_TRAVERSAL_WEIGHT to lower it could give
alternate answers such as
    v3.13-rc7~9^2~14^2~42
or
    v3.13~5^2~4^2~2^2~1^2~42
A manual solution involving looking at tagger dates came up with
    v3.13-rc1~65^2^2~42
which is much nicer.  That workaround was then implemented in name-rev.

Unfortunately, the taggerdate heuristic is causing bugs.  I was pointed
to a case in a private repository where name-rev reports a name of the
form
    v2022.10.02~86
when users expected to see one of the form
    v2022.10.01~2
(I've modified the names and numbers a bit from the real testcase.)  As
you can probably guess, v2022.10.01 was created after v2022.10.02 (by a
few hours), even though it pointed to an older commit.  While the
condition is unusual even in the repository in question, it is not the
only problematic set of tags in that repository.  The taggerdate logic
is causing problems.

Further, it turns out that this taggerdate heuristic isn't even helping
anymore.  Due to the fix to naming logic in 3656f84278 ("name-rev:
prefer shorter names over following merges", 2021-12-04), we get
improved names without the taggerdate heuristic.  For the original
commit of interest in linux.git, a modern git without the taggerdate
heuristic still provides the same optimal answer of interest, namely:
    v3.13-rc1~65^2^2~42

So, the taggerdate is no longer providing benefit, and it is causing
problems.  Simply get rid of it.

However, note that "taggerdate" as a variable is used to store things
besides a taggerdate these days.  Ever since commit ef1e74065c
("name-rev: favor describing with tags and use committer date to
tiebreak", 2017-03-29), this has been used to store committer dates and
there it is used as a fallback tiebreaker (as opposed to a primary
criteria overriding effective distance calculations).  We do not want to
remove that fallback tiebreaker, so not all instances of "taggerdate"
are removed in this change.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-09 09:01:36 -08:00
Emily Shaffer
0414b3891c hook: support a --to-stdin=<path> option
Expose the "path_to_stdin" API added in the preceding commit in the
"git hook run" command.

For now we won't be using this command interface outside of the tests,
but exposing this functionality makes it easier to test the hook
API. The plan is to use this to extend the "sendemail-validate"
hook[1][2].

1. https://lore.kernel.org/git/ad152e25-4061-9955-d3e6-a2c8b1bd24e7@amd.com
2. https://lore.kernel.org/git/20230120012459.920932-1-michael.strawbridge@amd.com

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-08 12:50:03 -08:00
Emily Shaffer
917e080249 hook API: support passing stdin to hooks, convert am's 'post-rewrite'
Convert the invocation of the 'post-rewrite' hook run by 'git am' to
use the hook.h library. To do this we need to add a "path_to_stdin"
member to "struct run_hooks_opt".

In our API this is supported by asking for a file path, rather
than by reading stdin. Reading directly from stdin would involve caching
the entire stdin (to memory or to disk) once the hook API is made to
support "jobs" larger than 1, along with support for executing N hooks
at a time (i.e. the upcoming config-based hooks).

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-08 12:50:03 -08:00
Ævar Arnfjörð Bjarmason
a535040887 builtin/rebase.c: free() "options.strategy_opts"
When the "strategy_opts" member was added in ba1905a5fe (builtin
rebase: add support for custom merge strategies, 2018-09-04) the
corresponding free() for it at the end of cmd_rebase() wasn't added,
let's do so.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 16:03:53 -08:00
Ævar Arnfjörð Bjarmason
94ad545d47 builtin/rebase.c: fix "options.onto_name" leak
Similar to the existing "squash_onto_name" added in [1] we need to
free() the xstrdup()'d "options.onto.name" added for "--keep-base" in
[2]..

1. 9dba809a69 (builtin rebase: support --root, 2018-09-04)
2. 414d924beb (rebase: teach rebase --keep-base, 2019-08-27)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 16:03:52 -08:00
Ævar Arnfjörð Bjarmason
a6a700a43c builtin/revert.c: move free-ing of "revs" to replay_opts_release()
In [1] and [2] I added the code being moved here to cmd_revert() and
cmd_cherry_pick(), now that we've got a "replay_opts_release()" for
the "struct replay_opts" it should know how to free these "revs",
rather than having these users reach into the struct to free its
individual members.

1. d1ec656d68 (cherry-pick: free "struct replay_opts" members,
   2022-11-08)
2. fd74ac95ac (revert: free "struct replay_opts" members, 2022-07-01)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 16:03:52 -08:00
Ævar Arnfjörð Bjarmason
9ff2f06069 sequencer API users: fix get_replay_opts() leaks
Make the replay_opts_release() function added in the preceding commit
non-static, and use it for freeing the "struct replay_opts"
constructed for "rebase" and "revert".

To safely call our new replay_opts_release() we'll need to stop
calling it in sequencer_remove_state(), and instead call it where we
allocate the "struct replay_opts" itself.

This is because in e.g. do_interactive_rebase() we construct a "struct
replay_opts" with "get_replay_opts()", and then call
"complete_action()". If we get far enough in that function without
encountering errors we'll call "pick_commits()" which (indirectly)
calls sequencer_remove_state() at the end.

But if we encounter errors anywhere along the way we'd punt out early,
and not free() the memory we allocated. Remembering whether we
previously called sequencer_remove_state() would be a hassle.

Using a FREE_AND_NULL() pattern would also work, as it would be safe
to call replay_opts_release() repeatedly. But let's fix this properly
instead, by having the owner of the data free() it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 16:03:52 -08:00
Ævar Arnfjörð Bjarmason
01fd5fb14b rebase: use "cleanup" pattern in do_interactive_rebase()
Use a "goto cleanup" pattern in do_interactive_rebase(). This
eliminates some duplicated free() code added in 53bbcfbde7 (rebase
-i: implement the main part of interactive rebase as a builtin,
2018-09-27), and sets us up for a subsequent commit which'll make
further use of the "cleanup" label.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 16:03:52 -08:00
Ævar Arnfjörð Bjarmason
c65d18cb52 push: free_refs() the "local_refs" in set_refspecs()
Fix a memory leak that's been with us since this code was added in
ca02465b41 (push: use remote.$name.push as a refmap, 2013-12-03).

The "remote = remote_get(...)" added in the same commit would seem to
leak based only on the context here, but that function is a wrapper
for sticking the remotes we fetch into "the_repository->remote_state".

See fd3cb0501e (remote: move static variables into per-repository
struct, 2021-11-17) for the addition of code in repository.c that
free's the "remote" allocated here.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:40 -08:00
Ævar Arnfjörð Bjarmason
aa561208d9 push: refactor refspec_append_mapped() for subsequent leak-fix
The set_refspecs() caller of refspec_append_mapped() (added in [1])
left open the question[2] of whether the "remote" we lazily fetch
might be NULL in the "[...]uniquely name our ref?" case, as
remote_get() can return NULL.

If we got past the "[...]uniquely name our ref?" case we'd have
already segfaulted if we tried to dereference it as
"remote->push.nr". In these cases the config mechanism & previous
remote validation will have bailed out earlier.

Let's refactor this code to clarify that, we'll now BUG() out if we
can't get a "remote", and will no longer retrieve it for these common
cases where we don't need it.

1. ca02465b41 (push: use remote.$name.push as a refmap, 2013-12-03)
2. https://lore.kernel.org/git/c0c07b89-7eaf-21cd-748e-e14ea57f09fd@web.de/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:40 -08:00
Ævar Arnfjörð Bjarmason
1fdd31cf52 receive-pack: release the linked "struct command *" list
Fix a memory leak that's been with us since this code was introduced
in [1]. Later in [2] we started using FLEX_ALLOC_MEM() to allocate the
"struct command *".

1. 575f497456 (Add first cut at "git-receive-pack", 2005-06-29)
2. eb1af2df0b (git-receive-pack: start parsing ref update commands,
   2005-06-29)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:40 -08:00
Ævar Arnfjörð Bjarmason
41211db10f builtin/merge.c: free "&buf" on "Your local changes..." error
Plug a memory leak introduced in [1], since that change didn't follow
the "goto done" pattern introduced in [2] we'd leak the "&buf" memory.

1. e4cdfe84a0 (merge: abort if index does not match HEAD for trivial
   merges, 2022-07-23)
2. d5a35c114a (Copy resolve_ref() return value for longer use,
   2011-11-13)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:39 -08:00
Ævar Arnfjörð Bjarmason
345e216f63 builtin/merge.c: use fixed strings, not "strbuf", fix leak
Follow-up 465028e0e2 (merge: add missing strbuf_release(),
2021-10-07) and address the "msg" memory leak in this block. We could
free "&msg" before the "goto done" here, but even better is to avoid
allocating it in the first place.

By repeating the "Fast-forward" string here we can avoid using a
"struct strbuf" altogether.

Suggested-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:39 -08:00
Ævar Arnfjörð Bjarmason
81559612a9 show-branch: free() allocated "head" before return
Stop leaking the "head" variable, which we've been leaking since it
was originally added in [1], and in its current form since [2]

1. ed378ec7e8 (Make ref resolution saner, 2006-09-11)
2. d9e557a320 (show-branch: store resolved head in heap buffer,
   2017-02-14).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:39 -08:00
Ævar Arnfjörð Bjarmason
9d01cfed69 commit-graph: fix a parse_options_concat() leak
When the parse_options_concat() was added to this file in
84e4484f12 (commit-graph: use parse_options_concat(), 2021-08-23) we
wouldn't free() it if we returned early in these cases.

Since "result" is 0 by default we can "goto cleanup" in both cases,
and only need to set "result" if write_commit_graph_reachable() fails.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:38 -08:00
Ævar Arnfjörð Bjarmason
9f24f3c719 worktree: fix a trivial leak in prune_worktrees()
We were leaking both the "struct strbuf" in prune_worktrees(), as well
as the "path" we got from should_prune_worktree(). Since these were
the only two uses of the "struct string_list" let's change it to a
"DUP" and push these to it with "string_list_append_nodup()".

For the string_list_append_nodup() we could also string_list_append()
the main_path.buf, and then strbuf_release(&main_path) right away. But
doing it this way avoids an allocation, as we already have the "struct
strbuf" prepared for appending to "kept".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:38 -08:00
Ævar Arnfjörð Bjarmason
90428ddccf repack: fix leaks on error with "goto cleanup"
In cmd_repack() when we hit an error, replace "return ret" with "goto
cleanup" to ensure we free the necessary data structures.

Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:37 -08:00
Ævar Arnfjörð Bjarmason
486620ae0c name-rev: don't xstrdup() an already dup'd string
When "add_to_tip_table()" is called with a non-zero
"shorten_unambiguous" we always return an xstrdup()'d string, which
we'd then xstrdup() again, leaking memory. See [1] and [2] for how
this leak came about.

We could xstrdup() only if "shorten_unambiguous" wasn't true, but
let's instead inline this code, so that information on whether we need
to xstrdup() is contained within add_to_tip_table().

1. 98c5c4ad01 (name-rev: allow to specify a subpath for --refs
   option, 2013-06-18)
2. b23e0b9353 (name-rev: allow converting the exact object name at
   the tip of a ref, 2013-07-07)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:37 -08:00
Ævar Arnfjörð Bjarmason
7615cf94d2 various: add missing clear_pathspec(), fix leaks
Fix memory leaks resulting from a missing clear_pathspec().

- archive.c: Plug a leak in the "struct archiver_args", and
  clear_pathspec() the "pathspec" member that the "parse_pathspec_arg()"
  call in this function populates.

- builtin/clean.c: Fix a memory leak that's been with us since
  893d839970 (clean: convert to use parse_pathspec, 2013-07-14).

- builtin/reset.c: Add clear_pathspec() calls to cmd_reset(),
  including to the codepaths where we'd return early.

- builtin/stash.c: Call clear_pathspec() on the pathspec initialized
  in push_stash().

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:37 -08:00
Ævar Arnfjörð Bjarmason
81e5c39cf6 clone: use free() instead of UNLEAK()
Change an UNLEAK() added in 0c4542738e (clone: free or UNLEAK further
pointers when finished, 2021-03-14) to use a "to_free" pattern
instead. In this case the "repo" can be either this absolute_pathdup()
value, or in the "else if" branch seen in the context the the
"argv[0]" argument to "main()".

We can only free() the value in the former case, hence the "to_free"
pattern.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:37 -08:00
Ævar Arnfjörð Bjarmason
e8ed0a8ac5 commit-graph: use free_commit_graph() instead of UNLEAK()
In 0bfb48e672 (builtin/commit-graph.c: UNLEAK variables, 2018-10-03)
this was made to UNLEAK(), but we can just as easily invoke the
free_commit_graph() function added in c3756d5b7f (commit-graph: add
free_commit_graph, 2018-07-11) instead.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:34:36 -08:00
Ævar Arnfjörð Bjarmason
5a7d41d849 docs & comments: replace mentions of "git-add--interactive.perl"
Now that we've removed "git-add--interactive.perl" let's replace
mentions of it with "add-interactive.c". In the case of the "git add"
documentation we were using it as an example filename, so the mention
wasn't wrong, but using a dead file is slightly confusing.

The "borrowed" comment here likewise isn't wrong, but let's mention
the successor file instead. In the case of pathspec.c the implied TODO
item should refer to the current code (and the comment may not even be
current, I didn't check).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:03:34 -08:00
Ævar Arnfjörð Bjarmason
d21878f073 add API: remove run_add_interactive() wrapper function
Now that the Perl "git-add--interactive" has gone away in the
preceding commit we don't need to pass along our desire for a mode as
a string, and can instead directly use the "enum add_p_mode", see
d2a233cb8b (built-in add -p: prepare for patch modes other than
"stage", 2019-12-21) for its introduction.

As a result of that the run_add_interactive() function would become a
trivial wrapper which would only run run_add_i() if a 0 (or now,
"NULL") "patch_mode" was provided. Let's instead remove it, and have
the one callsite that wanted the "NULL" case (interactive_add())
handle it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:03:34 -08:00
Ævar Arnfjörð Bjarmason
20b813d7d3 add: remove "add.interactive.useBuiltin" & Perl "git add--interactive"
Since [1] first released with Git v2.37.0 the built-in version of "add
-i" has been the default. That built-in implementation was added in
[2], first released with Git v2.25.0.

At this point enough time has passed to allow for finding any
remaining bugs in this new implementation, so let's remove the
fallback code.

As with similar migrations for "stash"[3] and "rebase"[4] we're
keeping a mention of "add.interactive.useBuiltin" in the
documentation, but adding a warning() to notify any outstanding users
that the built-in is now the default. As with [5] and [6] we should
follow-up in the future and eventually remove that warning.

1. 0527ccb1b5 (add -i: default to the built-in implementation,
   2021-11-30)
2. f83dff60a7 (Start to implement a built-in version of `git add
   --interactive`, 2019-11-13)
3. 8a2cd3f512 (stash: remove the stash.useBuiltin setting,
   2020-03-03)
4. d03ebd411c (rebase: remove the rebase.useBuiltin setting,
   2019-03-18)
5. deeaf5ee07 (stash: remove documentation for `stash.useBuiltin`,
   2022-01-27)
6. 9bcde4d531 (rebase: remove transitory rebase.useBuiltin setting &
   env, 2021-03-23)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 15:03:34 -08:00
René Scharfe
e65b868d07 pack-objects: use strcspn(3) in name_cmp_len()
Call strcspn(3) to find the length of a string terminated by NUL, NL or
slash instead of open-coding it.  Adopt its return type, size_t, to
support strings of arbitrary length.  Use that type in callers as well
for variables and function parameters that receive the return value.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-06 14:31:11 -08:00