git/branch.c, branch jch

branch: advice using git-help(1) instead of man(1)

2025-12-03T08:16:05Z

8fbd903e (branch: advise about ref syntax rules, 2024-03-05) added an advice about checking git-check-ref-format(1) for the ref syntax rules. The advice uses man(1). But git(1) is a multi-platform tool and man(1) may not be available on some platforms. It might also be slightly jarring to see a suggestion for running a command which is not from the Git suite. Let’s instead use git-help(1) in order to stay inside the land of git(1). This also means that `help.format` (for `man`, `html` or other formats) will be used if set. Also change to using single quotes (') to quote the command since that is more conventional. While here let’s also update the test to use `{SQ}`, which is more readable and easier to edit. Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano

config: drop `git_config_get_multivar_gently()` wrapper

2025-07-23T15:15:21Z

In 036876a1067 (config: hide functions using `the_repository` by default, 2024-08-13) we have moved around a bunch of functions in the config subsystem that depend on `the_repository`. Those function have been converted into mere wrappers around their equivalent function that takes in a repository as parameter, and the intent was that we'll eventually remove those wrappers to make the dependency on the global repository variable explicit at the callsite. Follow through with that intent and remove `git_config_get_multivar_gently()`. All callsites are adjusted so that they use `repo_config_get_multivar_gently(the_repository, ...)` instead. While some callsites might already have a repository available, this mechanical conversion is the exact same as the current situation and thus cannot cause any regression. Those sites should eventually be cleaned up in a later patch series. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

config: drop `git_config_set_gently()` wrapper

2025-07-23T15:15:20Z

In 036876a1067 (config: hide functions using `the_repository` by default, 2024-08-13) we have moved around a bunch of functions in the config subsystem that depend on `the_repository`. Those function have been converted into mere wrappers around their equivalent function that takes in a repository as parameter, and the intent was that we'll eventually remove those wrappers to make the dependency on the global repository variable explicit at the callsite. Follow through with that intent and remove `git_config_set_gently()`. All callsites are adjusted so that they use `repo_config_set_gently(the_repository, ...)` instead. While some callsites might already have a repository available, this mechanical conversion is the exact same as the current situation and thus cannot cause any regression. Those sites should eventually be cleaned up in a later patch series. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

config: drop `git_config_get_string()` wrapper

2025-07-23T15:15:19Z

In 036876a1067 (config: hide functions using `the_repository` by default, 2024-08-13) we have moved around a bunch of functions in the config subsystem that depend on `the_repository`. Those function have been converted into mere wrappers around their equivalent function that takes in a repository as parameter, and the intent was that we'll eventually remove those wrappers to make the dependency on the global repository variable explicit at the callsite. Follow through with that intent and remove `git_config_get_string()`. All callsites are adjusted so that they use `repo_config_get_string(the_repository, ...)` instead. While some callsites might already have a repository available, this mechanical conversion is the exact same as the current situation and thus cannot cause any regression. Those sites should eventually be cleaned up in a later patch series. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

remote: remove branch->merge_name and fix branch_release()

2025-06-23T23:38:55Z

The branch structure has both branch->merge_name and branch->merge for tracking the merge information. The former is allocated by add_merge() and stores the names read from the configuration file. The latter is allocated by set_merge() which is called by branch_get() when an external caller requests a branch. This leads to the confusing situation where branch->merge_nr tracks both the size of branch->merge (once its allocated) and branch->merge_name. The branch_release() function incorrectly assumes that branch->merge is always set when branch->merge_nr is non-zero, and can potentially crash if read_config() is called without branch_get() being called on every branch. In addition, branch_release() fails to free some of the memory associated with the structure including: * Failure to free the refspec_item containers in branch->merge[i] * Failure to free the strings in branch->merge_name[i] * Failure to free the branch->merge_name parent array. The set_merge() function sets branch->merge_nr to 0 when there is no valid remote_name, to avoid external callers seeing a non-zero merge_nr but a NULL merge array. This results in failure to release most of the merge data as well. These issues could be fixed directly, and indeed I initially proposed such a change at [1] in the past. While this works, there was some confusion during review because of the inconsistencies. Instead, its time to clean up the situation properly. Remove branch->merge_name entirely. Instead, allocate branch->merge earlier within add_merge() instead of within set_merge(). Instead of having set_merge() copy from merge_name[i] to merge[i]->src, just have add_merge() directly initialize merge[i]->src. Modify the add_merge() to call xstrdup() itself, instead of having the caller of add_merge() do so. This makes it more obvious which code owns the memory. Update all callers which use branch->merge_name[i] to use branch->merge[i]->src instead. Add a merge_clear() function which properly releases all of the merge-related memory, and which sets branch->merge_nr to zero. Use this both in branch_release() and in set_merge(), fixing the leak when set_merge() finds no valid remote_name. Add a set_merge variable to the branch structure, which indicates whether set_merge() has been called. This replaces the previous use of a NULL check against the branch->merge array. With these changes, the merge array is always allocated when merge_nr is non-zero. This use of refspec_item to store the names should be safe. External callers should be using branch_get() to obtain a pointer to the branch, which will call set_merge(), and the callers internal to remote.c already handle the partially initialized refpsec_item structure safely. This end result is cleaner, and avoids duplicating the merge names twice. Signed-off-by: Jacob Keller Link: [1] https://lore.kernel.org/git/20250617-jk-submodule-helper-use-url-v2-1-04cbb003177d@gmail.com/ Signed-off-by: Junio C Hamano

hash: stop depending on `the_repository` in `null_oid()`

2025-03-10T20:16:20Z

The `null_oid()` function returns the object ID that only consists of zeroes. Naturally, this ID also depends on the hash algorithm used, as the number of zeroes is different between SHA1 and SHA256. Consequently, the function returns the hash-algorithm-specific null object ID. This is currently done by depending on `the_hash_algo`, which implicitly makes us depend on `the_repository`. Refactor the function to instead pass in the hash algorithm for which we want to retrieve the null object ID. Adapt callsites accordingly by passing in `the_repository`, thus bubbling up the dependency on that global variable by one layer. There are a couple of trivial exceptions for subsystems that already got rid of `the_repository`. These subsystems instead use the repository that is available via the calling context: - "builtin/grep.c" - "grep.c" - "refs/debug.c" There are also two non-trivial exceptions: - "diff-no-index.c": Here we know that we may not have a repository initialized at all, so we cannot rely on `the_repository`. Instead, we adapt `diff_no_index()` to get a `struct git_hash_algo` as parameter. The only caller is located in "builtin/diff.c", where we know to call `repo_set_hash_algo()` in case we're running outside of a Git repository. Consequently, it is fine to continue passing `the_repository->hash_algo` even in this case. - "builtin/ls-files.c": There is an in-flight patch series that drops `USE_THE_REPOSITORY_VARIABLE` in this file, which causes a semantic conflict because we use `null_oid()` in `show_submodule()`. The value is passed to `repo_submodule_init()`, which may use the object ID to resolve a tree-ish in the superproject from which we want to read the submodule config. As such, the object ID should refer to an object in the superproject, and consequently we need to use its hash algorithm. This means that we could in theory just not bother about this edge case at all and just use `the_repository` in "diff-no-index.c". But doing so would feel misdesigned. Remove the `USE_THE_REPOSITORY_VARIABLE` preprocessor define in "hash.c". Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

worktree: return allocated string from `get_worktree_git_dir()`

2025-02-07T17:59:23Z

The `get_worktree_git_dir()` function returns a string constant that does not need to be free'd by the caller. This string is computed for three different cases: - If we don't have a worktree we return a path into the Git directory. The returned string is owned by `the_repository`, so there is no need for the caller to free it. - If we have a worktree, but no worktree ID then the caller requests the main worktree. In this case we return a path into the common directory, which again is owned by `the_repository` and thus does not need to be free'd. - In the third case, where we have an actual worktree, we compute the path relative to "$GIT_COMMON_DIR/worktrees/". This string does not need to be released either, even though `git_common_path()` ends up allocating memory. But this doesn't result in a memory leak either because we write into a buffer returned by `get_pathname()`, which returns one out of four static buffers. We're about to drop `git_common_path()` in favor of `repo_common_path()`, which doesn't use the same mechanism but instead returns an allocated string owned by the caller. While we could adapt `get_worktree_git_dir()` to also use `get_pathname()` and print the derived common path into that buffer, the whole schema feels a lot like premature optimization in this context. There are some callsites where we call `get_worktree_git_dir()` in a loop that iterates through all worktrees. But none of these loops seem to be even remotely in the hot path, so saving a single allocation there does not feel worth it. Refactor the function to instead consistently return an allocated path so that we can start using `repo_common_path()` in a subsequent commit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'jc/forbid-head-as-tagname'

2024-12-16T01:54:26Z

"git tag" has been taught to refuse to create refs/tags/HEAD as such a tag will be confusing in the context of UI provided by the Git Porcelain commands. * jc/forbid-head-as-tagname: tag: "git tag" refuses to use HEAD as a tagname t5604: do not expect that HEAD can be a valid tagname refs: drop strbuf_ prefix from helpers refs: move ref name helpers around

refs: drop strbuf_ prefix from helpers

2024-12-03T03:38:49Z

The helper functions (strbuf_branchname, strbuf_check_branch_ref, and strbuf_check_tag_ref) are about handling branch and tag names, and it is a non-essential fact that these functions use strbuf to hold these names. Rename them to make it clarify that these are more about "ref". Signed-off-by: Junio C Hamano

refs: allow passing flags when setting up a transaction

2024-11-20T22:59:14Z

Allow passing flags when setting up a transaction such that the behaviour of the transaction itself can be altered. This functionality will be used in a subsequent patch. Adapt callers accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano