Code clean-up.
* ma/worktree-cleanups:
worktree: use skip_prefix to parse target
worktree: rename copy-pasted variable
worktree: update renamed variable in comment
worktree: inline `worktree_ref()` into its only caller
wt-status: introduce wt_status_state_free_buffers()
wt-status: print to s->fp, not stdout
wt-status: replace sha1 mentions with oid
Instead of checking for "refs/heads/" using `starts_with()`, then
skipping past "refs/heads/" using `strlen()`, just use `skip_prefix()`.
In `is_worktree_being_rebased()`, we can adjust the indentation while
we're here and lose a pair of parentheses which isn't needed and which
might even make the reader wonder what they're missing and why that
grouping is there.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As the commit message of 04a3dfb8b5 ("worktree.c: check whether branch
is bisected in another worktree", 2016-04-22) indicates, the function
`is_worktree_being_bisected()` is based on the older function
`is_worktree_being_rebased()`. This heritage can also be seen in the
name of the variable where we store our return value: It was never
adapted while copy-editing and remains as `found_rebase`.
Rename the variable to make clear that we're looking for a bisect(ion),
nothing else.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The comment above `add_head_info()` mentions "head_sha1", but it was
renamed to "head_oid" in 0f05154c70 ("worktree: convert struct worktree
to object_id", 2017-10-15). Update the comment.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have `strbuf_worktree_ref()`, which works on a strbuf, and a wrapper
for it, `worktree_ref()` which returns a string. We even make this
wrapper available through worktree.h. But it only has a single caller,
sitting right next to it in worktree.c.
Just inline the wrapper into its only caller. This means the caller can
quite naturally reuse a single strbuf. We currently achieve something
similar by having a static strbuf in the wrapper.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we have a `struct wt_status_state`, we manually free its `branch`,
`onto` and `detached_from`, or sometimes just one or two of them.
Provide a function `wt_status_state_free_buffers()` which does the
freeing.
The callers are still aware of these fields, e.g., they check whether
`branch` was populated or not. But this way, they don't need to know
about *all* of them, and if `struct wt_status_state` gets more fields,
they will not need to learn to free them.
Users of `struct wt_status` (which contains a `wt_status_state`) already
have `wt_status_collect_free_buffers()` (corresponding to
`wt_status_collect()`) which we can also teach to use this new helper.
Finally, note that we're currently leaving dangling pointers behind.
Some callers work on a stack-allocated struct, where this is obviously
ok. But for the users of `run_status()` in builtin/commit.c, there are
ample opportunities for someone to mistakenly use those dangling
pointers. We seem to be ok for now, but it's a use-after-free waiting to
happen. Let's leave NULL-pointers behind instead.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The .git/worktrees/<id>/gitdir file points at the location of a linked
worktree's .git file. Its content must be of the form
/path/to/worktree/.git (from which the location of the worktree itself
can be derived by stripping the "/.git" suffix). If the gitdir file is
deleted or becomes corrupted or outdated, then Git will be unable to
find the linked worktree. An easy way for the gitdir file to become
outdated is for the user to move the worktree manually (without using
"git worktree move"). Although it is possible to manually update the
gitdir file to reflect the new linked worktree location, doing so
requires a level of knowledge about worktree internals beyond what a
user should be expected to know offhand.
Therefore, teach "git worktree repair" how to repair broken or outdated
.git/worktrees/<id>/gitdir files automatically. (For this to work, the
command must either be invoked from within the worktree whose gitdir
file requires repair, or from within the main or any linked worktree by
providing the path of the broken worktree as an argument to "git
worktree repair".)
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The .git file in a linked worktree is a "gitfile" which points back to
the .git/worktrees/<id> entry in the main worktree or bare repository.
If a worktree's .git file is deleted or becomes corrupted or outdated,
then the linked worktree won't know how to find the repository or any of
its own administrative files (such as 'index', 'HEAD', etc.). An easy
way for the .git file to become outdated is for the user to move the
main worktree or bare repository. Although it is possible to manually
update each linked worktree's .git file to reflect the new repository
location, doing so requires a level of knowledge about worktree
internals beyond what a user should be expected to know offhand.
Therefore, teach "git worktree repair" how to repair broken or outdated
worktree .git files automatically. (For this to work, the command must
be invoked from within the main worktree or bare repository, or from
within a worktree which has not become disconnected from the repository
-- such as one which was created after the repository was moved.)
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In order for "git-worktree list" to present consistent results,
get_main_worktree() performs manual normalization on the repository
path (returned by get_common_dir()) after passing it through
strbuf_add_absolute_path(). In particular, it cleans up the path for
three distinct cases when the current working directory is (1) the main
worktree, (2) the .git/ subdirectory, or (3) a bare repository.
The need for such special-cases is a direct consequence of employing
strbuf_add_absolute_path() which, for the sake of efficiency, doesn't
bother normalizing the path (such as folding out redundant path
components) after making it absolute. Lack of normalization is not
typically a problem since redundant path elements make no difference
when working with paths at the filesystem level. However, when preparing
paths for presentation, possible redundant path components make it
difficult to ensure consistency.
Eliminate the need for these special cases by instead making the path
absolute via strbuf_add_real_path() which normalizes the path for us.
Once normalized, the only case we need to handle manually is converting
it to the path of the main worktree by stripping the "/.git" suffix.
This stripping of the "/.git" suffix is a regular idiom in
worktree-related code; for instance, it is employed by
get_linked_worktree(), as well.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The content of .git/worktrees/<id>/gitdir must be a path of the form
"/path/to/worktree/.git". Any other content would be indicative of a
corrupt "gitdir" file. To determine the path of the worktree itself one
merely strips the "/.git" suffix, and this is indeed how the worktree
path was determined from inception.
However, 5193490442 (worktree: add a function to get worktree details,
2015-10-08) extended the path manipulation in a mysterious way. If it is
unable to strip "/.git" from the path, then it instead reports the
current working directory as the linked worktree's path:
if (!strbuf_strip_suffix(&worktree_path, "/.git")) {
strbuf_reset(&worktree_path);
strbuf_add_absolute_path(&worktree_path, ".");
strbuf_strip_suffix(&worktree_path, "/.");
}
This logic is clearly bogus; it can never be generally correct behavior.
It materialized out of thin air in 5193490442 with neither explanation
nor tests to illustrate a case in which it would be desirable.
It's possible that this logic was introduced to somehow deal with a
corrupt "gitdir" file, so that it returns _some_ sort of meaningful
value, but returning the current working directory is not helpful. In
fact, it is quite misleading (except in the one specific case when the
current directory is the worktree whose "gitdir" entry is corrupt).
Moreover, reporting the corrupt value to the user, rather than fibbing
about it and hiding it outright, is more helpful since it may aid in
diagnosing the problem.
Therefore, drop this bogus path munging and restore the logic to the
original behavior of merely stripping "/.git".
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This code has been unused since fa099d2322 (worktree.c: kill parse_ref()
in favor of refs_resolve_ref_unsafe(), 2017-04-24), so drop it.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The content of this strbuf is unconditionally detached several lines
before the strbuf_release() and the strbuf is never touched again after
that point.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
API cleanup for get_worktrees()
* es/get-worktrees-unsort:
worktree: drop get_worktrees() unused 'flags' argument
worktree: drop get_worktrees() special-purpose sorting option
get_worktrees() retrieves a list of all worktrees associated with a
repository, including the main worktree. The location of the main
worktree is determined by get_main_worktree() which needs to handle
three distinct cases for the main worktree after absolute-path
conversion:
* <bare-repository>/.
* <main-worktree>/.git/. (when $CWD is .git)
* <main-worktree>/.git (when $CWD is any worktree)
They all need to be normalized to just the <path> portion, dropping any
"/." or "/.git" suffix.
It turns out, however, that get_main_worktree() was only handling the
first and last cases, i.e.:
if (!strip_suffix(path, "/.git"))
strip_suffix(path, "/.");
This shortcoming was addressed by 45f274fbb1 (get_main_worktree(): allow
it to be called in the Git directory, 2020-02-23) by changing the logic
to:
strip_suffix(path, "/.");
if (!strip_suffix(path, "/.git"))
strip_suffix(path, "/.");
which makes the final strip_suffix() invocation dead-code.
Fix this oversight by enumerating the three distinct cases explicitly
rather than attempting to strip the suffix(es) incrementally.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
get_worktrees() accepts a 'flags' argument, however, there are no
existing flags (the lone flag GWT_SORT_LINKED was recently retired) and
no behavior which can be tweaked. Therefore, drop the 'flags' argument.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Of all the clients of get_worktrees(), only "git worktree list" wants
the list sorted in a very specific way; other clients simply don't care
about the order. Rather than imbuing get_worktrees() with special
knowledge about how various clients -- now and in the future -- may want
the list sorted, drop the sorting capability altogether and make it the
client's responsibility to sort the list if needed.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
SHA-256 transition continues.
* bc/sha-256-part-1-of-4: (22 commits)
fast-import: add options for rewriting submodules
fast-import: add a generic function to iterate over marks
fast-import: make find_marks work on any mark set
fast-import: add helper function for inserting mark object entries
fast-import: permit reading multiple marks files
commit: use expected signature header for SHA-256
worktree: allow repository version 1
init-db: move writing repo version into a function
builtin/init-db: add environment variable for new repo hash
builtin/init-db: allow specifying hash algorithm on command line
setup: allow check_repository_format to read repository format
t/helper: make repository tests hash independent
t/helper: initialize repository if necessary
t/helper/test-dump-split-index: initialize git repository
t6300: make hash algorithm independent
t6300: abstract away SHA-1-specific constants
t: use hash-specific lookup tables to define test constants
repository: require a build flag to use SHA-256
hex: add functions to parse hex object IDs in any algorithm
hex: introduce parsing variants taking hash algorithms
...
This commit continues the work started with previous commit.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Returning a shared buffer invites very subtle bugs due to reentrancy or
multi-threading, as demonstrated by the previous patch.
There was an unfinished effort to abolish this [1].
Let's finally rid of `real_path()`, using `strbuf_realpath()` instead.
This patch uses a local `strbuf` for most places where `real_path()` was
previously called.
However, two places return the value of `real_path()` to the caller. For
them, a `static` local `strbuf` was added, effectively pushing the
problem one level higher:
read_gitfile_gently()
get_superproject_working_tree()
[1] https://lore.kernel.org/git/1480964316-99305-1-git-send-email-bmwill@google.com/
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git push" should stop from updating a branch that is checked out
when receive.denyCurrentBranch configuration is set, but it failed
to pay attention to checkouts in secondary worktrees. This has
been corrected.
* hv/receive-denycurrent-everywhere:
t2402: test worktree path when called in .git directory
receive.denyCurrentBranch: respect all worktrees
t5509: use a bare repository for test push target
get_main_worktree(): allow it to be called in the Git directory
In rare cases "git worktree add <path>" could think that <path>
was already a registered worktree even when it wasn't and refuse
to add the new worktree. This has been corrected.
* es/worktree-avoid-duplication-fix:
worktree: don't allow "add" validation to be fooled by suffix matching
worktree: add utility to find worktree by pathname
worktree: improve find_worktree() documentation
find_worktree() employs heuristics to match user provided input -- which
may be a pathname or some sort of shorthand -- with an actual worktree.
Although this convenience allows a user to identify a worktree with
minimal typing, the black-box nature of these heuristics makes it
potentially difficult for callers which already know the exact path of a
worktree to be confident that the correct worktree will be returned for
any specific pathname (particularly a relative one), especially as the
heuristics are enhanced and updated.
Therefore, add a companion function, find_worktree_by_path(), which
deterministically identifies a worktree strictly by pathname with no
interpretation and no magic matching.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This code has been unused since fa099d2322 (worktree.c: kill parse_ref()
in favor of refs_resolve_ref_unsafe(), 2017-04-24), so drop it.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When called in the Git directory of a non-bare repository, this function
would not return the directory of the main worktree, but of the Git
directory instead.
The reason: when the Git directory is the current working directory, the
absolute path of the common directory will be reported with a trailing
`/.git/.`, which the code of `get_main_worktree()` does not handle
correctly.
Let's fix this.
Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git supports both repository versions 0 and 1. These formats are
identical except for the presence of extensions. When using an
extension, such as for a different hash algorithm, a check for only
version 0 causes the check to fail. Instead, call
verify_repository_format to verify that we have an appropriate version
and no unknown extensions.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git worktree add" used to fail when another worktree connected to
the same repository was corrupt, which has been corrected.
* nd/corrupt-worktrees:
worktree add: be tolerant of corrupt worktrees
find_worktree() can die() unexpectedly because it uses real_path()
instead of the gentler version. When it's used in 'git worktree add' [1]
and there's a bad worktree, this die() could prevent people from adding
new worktrees.
The "bad" condition to trigger this is when a parent of the worktree's
location is deleted. Then real_path() will complain.
Use the other version so that bad worktrees won't affect 'worktree
add'. The bad ones will eventually be pruned, we just have to tolerate
them for a bit.
[1] added in cb56f55c16 (worktree: disallow adding same path multiple
times, 2018-08-28), or since v2.20.0. Though the real bug in
find_worktree() is much older.
Reported-by: Shaheed Haque <shaheedhaque@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The logic to tell if a Git repository has a working tree protects
"git branch -D" from removing the branch that is currently checked
out by mistake. The implementation of this logic was broken for
repositories with unusual name, which unfortunately is the norm for
submodules these days. This has been fixed.
* jt/submodule-repo-is-with-worktree:
worktree: update is_bare heuristics
When "git branch -D <name>" is run, Git usually first checks if that
branch is currently checked out. But this check is not performed if the
Git directory of that repository is not at "<repo>/.git", which is the
case if that repository is a submodule that has its Git directory stored
as "super/.git/modules/<repo>", for example. This results in the branch
being deleted even though it is checked out.
This is because get_main_worktree() in worktree.c sets is_bare on a
worktree only using the heuristic that a repo is bare if the worktree's
path does not end in "/.git", and not bare otherwise. This is_bare code
was introduced in 92718b7438 ("worktree: add details to the worktree
struct", 2015-10-08), following a pre-core.bare heuristic. This patch
does 2 things:
- Teach get_main_worktree() to use is_bare_repository() instead,
introduced in 7d1864ce67 ("Introduce is_bare_repository() and
core.bare configuration variable", 2007-01-07) and updated in
e90fdc39b6 ("Clean up work-tree handling", 2007-08-01). This solves
the "git branch -D <name>" problem described above. However...
- If a repository has core.bare=1 but the "git" command is being run
from one of its secondary worktrees, is_bare_repository() returns
false (which is fine, since there is a worktree available). However,
treating the main worktree as non-bare when it is bare causes issues:
for example, failure to delete a branch from a secondary worktree
that is referred to by a main worktree's HEAD, even if that main
worktree is bare.
In order to avoid that, also check core.bare when setting is_bare. If
core.bare=1, trust it, and otherwise, use is_bare_repository().
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After we set up a `struct repository_format`, it owns various pieces of
allocated memory. We then either use those members, because we decide we
want to use the "candidate" repository format, or we discard the
candidate / scratch space. In the first case, we transfer ownership of
the memory to a few global variables. In the latter case, we just
silently drop the struct and end up leaking memory.
Introduce an initialization macro `REPOSITORY_FORMAT_INIT` and a
function `clear_repository_format()`, to be used on each side of
`read_repository_format()`. To have a clear and simple memory ownership,
let all users of `struct repository_format` duplicate the strings that
they take from it, rather than stealing the pointers.
Call `clear_...()` at the start of `read_...()` instead of just zeroing
the struct, since we sometimes enter the function multiple times. Thus,
it is important to initialize the struct before calling `read_...()`, so
document that. It's also important because we might not even call
`read_...()` before we call `clear_...()`, see, e.g., builtin/init-db.c.
Teach `read_...()` to clear the struct on error, so that it is reset to
a safe state, and document this. (In `setup_git_directory_gently()`, we
look at `repo_fmt.hash_algo` even if `repo_fmt.version` is -1, which we
weren't actually supposed to do per the API. After this commit, that's
ok.)
We inherit the existing code's combining "error" and "no version found".
Both are signalled through `version == -1` and now both cause us to
clear any partial configuration we have picked up. For "extensions.*",
that's fine, since they require a positive version number. For
"core.bare" and "core.worktree", we're already verifying that we have a
non-negative version number before using them.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The code to traverse objects for reachability, used to decide what
objects are unreferenced and expendable, have been taught to also
consider per-worktree refs of other worktrees as starting points to
prevent data loss.
* nd/per-worktree-ref-iteration:
git-worktree.txt: correct linkgit command name
reflog expire: cover reflog from all worktrees
fsck: check HEAD and reflog from other worktrees
fsck: move fsck_head_link() to get_default_heads() to avoid some globals
revision.c: better error reporting on ref from different worktrees
revision.c: correct a parameter name
refs: new ref types to make per-worktree refs visible to all worktrees
Add a place for (not) sharing stuff between worktrees
refs.c: indent with tabs, not spaces
A function prefixed with 'is_' would be expected to return a boolean,
however this function returns a string.
Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make use of the new ref aliases to pass refs from another worktree
around and access them from the current ref store instead. This does
not change any functionality, but when a problem arises, we would like
the reported messages to mention full ref aliases, like this:
fatal: bad object worktrees/ztemp/HEAD
warning: reflog of 'main-worktree/HEAD' references pruned commits
instead of
fatal: bad object HEAD
warning: reflog of 'HEAD' references pruned commits
which does not really tell where the refs are from.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
One of the problems with multiple worktree is accessing per-worktree
refs of one worktree from another worktree. This was sort of solved by
multiple ref store, where the code can open the ref store of another
worktree and has access to the ref space of that worktree.
The problem with this is reporting. "HEAD" in another ref space is
also called "HEAD" like in the current ref space. In order to
differentiate them, all the code must somehow carry the ref store
around and print something like "HEAD from this ref store".
But that is not feasible (or possible with a _lot_ of work). With the
current design, we pass a reference around as a string (so called
"refname"). Extending this design to pass a string _and_ a ref store
is a nightmare, especially when handling extended SHA-1 syntax.
So we do it another way. Instead of entering a separate ref space, we
make refs from other worktrees available in the current ref space. So
"HEAD" is always HEAD of the current worktree, but then we can have
"worktrees/blah/HEAD" to denote HEAD from a worktree named
"blah". This syntax coincidentally matches the underlying directory
structure which makes implementation a bit easier.
The main worktree has to be treated specially because well... it's
special from the beginning. So HEAD from the main worktree is
acccessible via the name "main-worktree/HEAD" instead of
"worktrees/main/HEAD" because "main" could be just another secondary
worktree.
This patch also makes it possible to specify refs from one worktree in
another one, e.g.
git log worktrees/foo/HEAD
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Callers don't expect library function find_worktree() to die(); they
expect it to return the named worktree if found, or NULL if not.
Although find_worktree() itself never invokes die(), it calls
real_pathdup() with 'die_on_error' incorrectly set to 'true', thus will
die() indirectly if the user-provided path is not to real_pathdup()'s
liking. This can be observed, for instance, with any git-worktree
command which searches for an existing worktree:
$ git worktree unlock foo
fatal: 'foo' is not a working tree
$ git worktree unlock foo/bar
fatal: Invalid path '.../foo': No such file or directory
The first error message is the expected one from "git worktree unlock"
not finding the specified worktree; the second is from find_worktree()
invoking real_pathdup() incorrectly and die()ing prematurely.
Aside from the inconsistent error message between the two cases, this
bug hasn't otherwise been a serious problem since existing callers all
die() anyhow when the worktree can't be found. However, that may not be
true of callers added in the future, so fix find_worktree() to avoid
die()ing.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In d8193743e0 (usage.c: add BUG() function, 2017-05-12), a new macro
was introduced to use for reporting bugs instead of die(). It was then
subsequently used to convert one single caller in 588a538ae5
(setup_git_env: convert die("BUG") to BUG(), 2017-05-12).
The cover letter of the patch series containing this patch
(cf 20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net) is not
terribly clear why only one call site was converted, or what the plan
is for other, similar calls to die() to report bugs.
Let's just convert all remaining ones in one fell swoop.
This trick was performed by this invocation:
sed -i 's/die("BUG: /BUG("/g' $(git grep -l 'die("BUG' \*.c)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git worktree remove" basically consists of two things
- delete $GIT_WORK_TREE
- delete $GIT_DIR (which is $SUPER_GIT_DIR/worktrees/something)
If $GIT_WORK_TREE is already gone for some reason, we should be able
to finish the job by deleting $GIT_DIR.
Two notes:
- $GIT_WORK_TREE _can_ be missing if the worktree is locked. In that
case we must not delete $GIT_DIR because the real $GIT_WORK_TREE may
be in a usb stick somewhere. This is already handled because we
check for lock first.
- validate_worktree() is still called because it may do more checks in
future (and it already does something else, like checking main
worktree, but that's irrelevant in this case)
Noticed-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This function is later used by "worktree move" and "worktree remove"
to ensure that we have a good connection between the repository and
the worktree. For example, if a worktree is moved manually, the
worktree location recorded in $GIT_DIR/worktrees/.../gitdir is
incorrect and we should not move that one.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Conversion from uchar[20] to struct object_id continues.
* bc/object-id: (25 commits)
refs/files-backend: convert static functions to object_id
refs: convert read_raw_ref backends to struct object_id
refs: convert peel_object to struct object_id
refs: convert resolve_ref_unsafe to struct object_id
worktree: convert struct worktree to object_id
refs: convert resolve_gitlink_ref to struct object_id
Convert remaining callers of resolve_gitlink_ref to object_id
sha1_file: convert index_path and index_fd to struct object_id
refs: convert reflog_expire parameter to struct object_id
refs: convert read_ref_at to struct object_id
refs: convert peel_ref to struct object_id
builtin/pack-objects: convert to struct object_id
pack-bitmap: convert traverse_bitmap_commit_list to object_id
refs: convert dwim_log to struct object_id
builtin/reflog: convert remaining unsigned char uses to object_id
refs: convert dwim_ref and expand_ref to struct object_id
refs: convert read_ref and read_ref_full to object_id
refs: convert resolve_refdup and refs_resolve_refdup to struct object_id
Convert check_connected to use struct object_id
refs: update ref transactions to use struct object_id
...
The refs_resolve_ref_unsafe() function may return NULL even
with a REF_ISSYMREF flag if a symref points to a broken ref.
As a result, it's possible for find_shared_symref() to
segfault when it passes NULL to strcmp().
This is hard to trigger for most code paths. We typically
pass HEAD to the function as the symref to resolve, and
programs like "git branch" will bail much earlier if HEAD
isn't valid.
I did manage to trigger it through one very obscure
sequence:
# You have multiple notes refs which conflict.
git notes add -m base
git notes --ref refs/notes/foo add -m foo
# There's left-over cruft in NOTES_MERGE_REF that
# makes it a broken symref (in this case we point
# to a syntactically invalid ref).
echo "ref: refs/heads/master.lock" >.git/NOTES_MERGE_REF
# You try to merge the notes. We read the broken value in
# order to complain that another notes-merge is
# in-progress, but we segfault in find_shared_symref().
git notes merge refs/notes/foo
This is obviously silly and almost certainly impossible to
trigger accidentally, but it does show that the bug is
triggerable from at least one code path. In addition, it
would trigger if we saw a transient filesystem error when
resolving the pointed-to ref.
We can fix this by treating NULL the same as a non-matching
symref. Arguably we'd prefer to know if a symref points to
"refs/heads/foo", but "refs/heads/foo" is broken. But
refs_resolve_ref_unsafe() isn't capable of giving us that
information, so this is the best we can do.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert resolve_ref_unsafe to take a pointer to struct object_id by
converting one remaining caller to use struct object_id, removing the
temporary NULL pointer check in expand_ref, converting the declaration
and definition, and applying the following semantic patch:
@@
expression E1, E2, E3, E4;
@@
- resolve_ref_unsafe(E1, E2, E3.hash, E4)
+ resolve_ref_unsafe(E1, E2, &E3, E4)
@@
expression E1, E2, E3, E4;
@@
- resolve_ref_unsafe(E1, E2, E3->hash, E4)
+ resolve_ref_unsafe(E1, E2, E3, E4)
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert the head_sha1 member to be head_oid instead. This is required
to convert resolve_ref_unsafe.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This allows us to get rid of two write-only variables, one of them
being a SHA1 buffer.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git gc" and friends when multiple worktrees are used off of a
single repository did not consider the index and per-worktree refs
of other worktrees as the root for reachability traversal, making
objects that are in use only in other worktrees to be subject to
garbage collection.
* nd/prune-in-worktree:
refs.c: reindent get_submodule_ref_store()
refs.c: remove fallback-to-main-store code get_submodule_ref_store()
rev-list: expose and document --single-worktree
revision.c: --reflog add HEAD reflog from all worktrees
files-backend: make reflog iterator go through per-worktree reflog
revision.c: --all adds HEAD from all worktrees
refs: remove dead for_each_*_submodule()
refs.c: move for_each_remote_ref_submodule() to submodule.c
revision.c: use refs_for_each*() instead of for_each_*_submodule()
refs: add refs_head_ref()
refs: move submodule slash stripping code to get_submodule_ref_store
refs.c: refactor get_submodule_ref_store(), share common free block
revision.c: --indexed-objects add objects from all worktrees
revision.c: refactor add_index_objects_to_pending()
refs.c: use is_dir_sep() in resolve_gitlink_ref()
revision.h: new flag in struct rev_info wrt. worktree-related refs
"git branch -M a b" while on a branch that is completely unrelated
to either branch a or branch b misbehaved when multiple worktree
was in use. This has been fixed.
* nd/worktree-kill-parse-ref:
branch: fix branch renaming not updating HEADs correctly
Unless single_worktree is set, --all now adds HEAD from all worktrees.
Since reachable.c code does not use setup_revisions(), we need to call
other_head_refs_submodule() explicitly there to have the same effect on
"git prune", so that we won't accidentally delete objects needed by some
other HEADs.
A new FIXME is added because we would need something like
int refs_other_head_refs(struct ref_store *, each_ref_fn, cb_data);
in addition to other_head_refs() to handle it, which might require
int get_submodule_worktrees(const char *submodule, int flags);
It could be a separate topic to reduce the scope of this one.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are two bugs that sort of work together and cause
problems. Let's start with one in replace_each_worktree_head_symref.
Before fa099d2322 (worktree.c: kill parse_ref() in favor of
refs_resolve_ref_unsafe() - 2017-04-24), this code looks like this:
if (strcmp(oldref, worktrees[i]->head_ref))
continue;
set_worktree_head_symref(...);
After fa099d2322, it is possible that head_ref can be NULL. However,
the updated code takes the wrong exit. In the error case (NULL
head_ref), we should "continue;" to the next worktree. The updated
code makes us _skip_ "continue;" and update HEAD anyway.
The NULL head_ref is triggered by the second bug in add_head_info (in
the same commit). With the flag RESOLVE_REF_READING, resolve_ref_unsafe()
will abort if it cannot resolve the target ref. For orphan checkouts,
HEAD always points to an unborned branch, resolving target ref will
always fail. Now we have NULL head_ref. Now we always update HEAD.
Correct the logic in replace_ function so that we don't accidentally
update HEAD on error. As it turns out, correcting the logic bug above
breaks branch renaming completely, thanks to the second bug.
"git branch -[Mm]" does two steps (on a normal checkout, no orphan!):
- rename the branch on disk (e.g. refs/heads/abc to refs/heads/def)
- update HEAD if it points to the branch being renamed.
At the second step, since the branch pointed to by HEAD (e.g. "abc") no
longer exists on disk, we run into a temporary orphan checkout situation
that has been just corrected to _not_ update HEAD. But we need to update
HEAD since it's not actually an orphan checkout. We need to update HEAD
to move out of that orphan state.
Correct add_head_info(), remove RESOLVE_REF_READING flag. With the flag
gone, we should always return good "head_ref" in orphan checkouts (either
temporary or permanent). With good head_ref, things start to work again.
Noticed-by: Nish Aravamudan <nish.aravamudan@canonical.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
