summaryrefslogtreecommitdiff
path: root/worktree.c
AgeCommit message (Collapse)AuthorFilesLines
2020-07-31worktree: retire special-case normalization of main worktree pathLibravatar Eric Sunshine1-4/+2
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>
2020-07-31worktree: drop bogus and unnecessary path mungingLibravatar Eric Sunshine1-6/+1
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>
2020-07-31worktree: drop unused code from get_linked_worktree()Libravatar Eric Sunshine1-3/+0
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>
2020-07-31worktree: drop pointless strbuf_release()Libravatar Eric Sunshine1-2/+0
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>
2020-07-06Merge branch 'es/worktree-code-cleanup'Libravatar Junio C Hamano1-3/+3
Code cleanup. * es/worktree-code-cleanup: worktree: avoid dead-code in conditional
2020-07-06Merge branch 'es/get-worktrees-unsort'Libravatar Junio C Hamano1-17/+3
API cleanup for get_worktrees() * es/get-worktrees-unsort: worktree: drop get_worktrees() unused 'flags' argument worktree: drop get_worktrees() special-purpose sorting option
2020-06-24worktree: avoid dead-code in conditionalLibravatar Eric Sunshine1-3/+3
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>
2020-06-22worktree: drop get_worktrees() unused 'flags' argumentLibravatar Eric Sunshine1-3/+3
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>
2020-06-22worktree: drop get_worktrees() special-purpose sorting optionLibravatar Eric Sunshine1-14/+0
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>
2020-03-26Merge branch 'bc/sha-256-part-1-of-4'Libravatar Junio C Hamano1-6/+4
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 ...
2020-03-10real_path_if_valid(): remove unsafe APILibravatar Alexandr Miloslavskiy1-2/+5
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>
2020-03-10real_path: remove unsafe APILibravatar Alexandr Miloslavskiy1-1/+4
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>
2020-03-05Merge branch 'hv/receive-denycurrent-everywhere'Libravatar Junio C Hamano1-0/+1
"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
2020-03-05Merge branch 'es/worktree-avoid-duplication-fix'Libravatar Junio C Hamano1-6/+10
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
2020-02-24worktree: add utility to find worktree by pathnameLibravatar Eric Sunshine1-6/+10
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>
2020-02-24worktree: drop unused code from get_main_worktree()Libravatar Eric Sunshine1-4/+0
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>
2020-02-24get_main_worktree(): allow it to be called in the Git directoryLibravatar Hariom Verma1-0/+1
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>
2020-02-24worktree: allow repository version 1Libravatar brian m. carlson1-6/+4
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>
2019-06-13Merge branch 'nd/corrupt-worktrees'Libravatar Junio C Hamano1-2/+5
"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
2019-05-15worktree add: be tolerant of corrupt worktreesLibravatar Nguyễn Thái Ngọc Duy1-2/+5
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>
2019-05-09Merge branch 'jt/submodule-repo-is-with-worktree'Libravatar Junio C Hamano1-4/+10
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
2019-04-21worktree: update is_bare heuristicsLibravatar Jonathan Tan1-4/+10
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>
2019-03-01setup: fix memory leaks with `struct repository_format`Libravatar Martin Ågren1-1/+3
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>
2018-11-13Merge branch 'nd/per-worktree-ref-iteration'Libravatar Junio C Hamano1-3/+76
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
2018-10-31worktree: rename is_worktree_locked to worktree_lock_reasonLibravatar Nickolai Belakovski1-1/+1
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>
2018-10-22revision.c: better error reporting on ref from different worktreesLibravatar Nguyễn Thái Ngọc Duy1-3/+46
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>
2018-10-22refs: new ref types to make per-worktree refs visible to all worktreesLibravatar Nguyễn Thái Ngọc Duy1-0/+30
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>
2018-08-30worktree: don't die() in library function find_worktree()Libravatar Eric Sunshine1-1/+5
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>
2018-05-06Replace all die("BUG: ...") calls by BUG() onesLibravatar Johannes Schindelin1-1/+1
In d8193743e08 (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 588a538ae55 (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>
2018-02-12worktree remove: allow it when $GIT_WORK_TREE is already goneLibravatar Nguyễn Thái Ngọc Duy1-1/+8
"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>
2018-02-12worktree.c: add update_worktree_location()Libravatar Nguyễn Thái Ngọc Duy1-0/+17
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-24worktree.c: add validate_worktree()Libravatar Nguyễn Thái Ngọc Duy1-0/+72
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>
2017-11-06Merge branch 'bc/object-id'Libravatar Junio C Hamano1-1/+1
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 ...
2017-10-21worktree: handle broken symrefs in find_shared_symref()Libravatar Jeff King1-1/+2
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>
2017-10-16refs: convert resolve_ref_unsafe to struct object_idLibravatar brian m. carlson1-1/+1
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>
2017-10-16worktree: convert struct worktree to object_idLibravatar brian m. carlson1-1/+1
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>
2017-09-24refs: pass NULL to refs_resolve_ref_unsafe() if hash is not neededLibravatar René Scharfe1-2/+1
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>
2017-09-19Merge branch 'nd/prune-in-worktree'Libravatar Junio C Hamano1-0/+22
"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
2017-09-10Merge branch 'nd/worktree-kill-parse-ref'Libravatar Junio C Hamano1-1/+1
"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
2017-08-24revision.c: --all adds HEAD from all worktreesLibravatar Nguyễn Thái Ngọc Duy1-0/+22
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>
2017-08-24branch: fix branch renaming not updating HEADs correctlyLibravatar Nguyễn Thái Ngọc Duy1-1/+1
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>
2017-06-23path: convert strbuf_git_common_path to take a 'struct repository'Libravatar Brandon Williams1-1/+2
Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-29Merge branch 'js/plug-leaks'Libravatar Junio C Hamano1-1/+1
Fix memory leaks pointed out by Coverity (and people). * js/plug-leaks: (26 commits) checkout: fix memory leak submodule_uses_worktrees(): plug memory leak show_worktree(): plug memory leak name-rev: avoid leaking memory in the `deref` case remote: plug memory leak in match_explicit() add_reflog_for_walk: avoid memory leak shallow: avoid memory leak line-log: avoid memory leak receive-pack: plug memory leak in update() fast-export: avoid leaking memory in handle_tag() mktree: plug memory leaks reported by Coverity pack-redundant: plug memory leak setup_discovered_git_dir(): plug memory leak setup_bare_git_dir(): help static analysis split_commit_in_progress(): simplify & fix memory leak checkout: fix memory leak cat-file: fix memory leak mailinfo & mailsplit: check for EOF while parsing status: close file descriptor after reading git-rebase-todo difftool: address a couple of resource/memory leaks ...
2017-05-16Merge branch 'nd/worktree-kill-parse-ref'Libravatar Junio C Hamano1-75/+27
"git gc" did not interact well with "git worktree"-managed per-worktree refs. * nd/worktree-kill-parse-ref: refs: kill set_worktree_head_symref() worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe() refs: introduce get_worktree_ref_store() refs: add REFS_STORE_ALL_CAPS refs.c: make submodule ref store hashmap generic environment.c: fix potential segfault by get_git_common_dir()
2017-05-08submodule_uses_worktrees(): plug memory leakLibravatar Johannes Schindelin1-1/+1
There is really no reason why we would need to hold onto the allocated string longer than necessary. Reported by Coverity. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-24worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()Libravatar Nguyễn Thái Ngọc Duy1-75/+27
The manual parsing code is replaced with a call to refs_resolve_ref_unsafe(). The manual parsing code must die because only refs/files-backend.c should do that. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21prefix_filename: return newly allocated stringLibravatar Jeff King1-1/+4
The prefix_filename() function returns a pointer to static storage, which makes it easy to use dangerously. We already fixed one buggy caller in hash-object recently, and the calls in apply.c are suspicious (I didn't dig in enough to confirm that there is a bug, but we call the function once in apply_all_patches() and then again indirectly from parse_chunk()). Let's make it harder to get wrong by allocating the return value. For simplicity, we'll do this even when the prefix is empty (and we could just return the original file pointer). That will cause us to allocate sometimes when we wouldn't otherwise need to, but this function isn't called in performance critical code-paths (and it already _might_ allocate on any given call, so a caller that cares about performance is questionable anyway). The downside is that the callers need to remember to free() the result to avoid leaking. Most of them already used xstrdup() on the result, so we know they are OK. The remainder have been converted to use free() as appropriate. I considered retaining a prefix_filename_unsafe() for cases where we know the static lifetime is OK (and handling the cleanup is awkward). This is only a handful of cases, though, and it's not worth the mental energy in worrying about whether the "unsafe" variant is OK to use in any situation. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21prefix_filename: drop length parameterLibravatar Jeff King1-1/+1
This function takes the prefix as a ptr/len pair, but in every caller the length is exactly strlen(ptr). Let's simplify the interface and just take the string. This saves callers specifying it (and in some cases handling a NULL prefix). In a handful of cases we had the length already without calling strlen, so this is technically slower. But it's not likely to matter (after all, if the prefix is non-empty we'll allocate and copy it into a buffer anyway). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-12Merge branch 'js/realpath-pathdup-fix'Libravatar Junio C Hamano1-1/+1
Git v2.12 was shipped with an embarrassing breakage where various operations that verify paths given from the user stopped dying when seeing an issue, and instead later triggering segfault. * js/realpath-pathdup-fix: real_pathdup(): fix callsites that wanted it to die on error t1501: demonstrate NULL pointer access with invalid GIT_WORK_TREE
2017-03-10Merge branch 'rs/strbuf-add-real-path'Libravatar Junio C Hamano1-1/+1
An helper function to make it easier to append the result from real_path() to a strbuf has been added. * rs/strbuf-add-real-path: strbuf: add strbuf_add_real_path() cocci: use ALLOC_ARRAY