Commit graph

7 commits

Author SHA1 Message Date
Eric Sunshine bb69b3b009 worktree: don't allow "add" validation to be fooled by suffix matching
"git worktree add <path>" performs various checks before approving
<path> as a valid location for the new worktree. Aside from ensuring
that <path> does not already exist, one of the questions it asks is
whether <path> is already a registered worktree. To perform this check,
it queries find_worktree() and disallows the "add" operation if
find_worktree() finds a match for <path>. As a convenience, however,
find_worktree() casts an overly wide net to allow users to identify
worktrees by shorthand in order to keep typing to a minimum. For
instance, it performs suffix matching which, given subtrees "foo/bar"
and "foo/baz", can correctly select the latter when asked only for
"baz".

"add" validation knows the exact path it is interrogating, so this sort
of heuristic-based matching is, at best, questionable for this use-case
and, at worst, may may accidentally interpret <path> as matching an
existing worktree and incorrectly report it as already registered even
when it isn't. (In fact, validate_worktree_add() already contains a
special case to avoid accidentally matching against the main worktree,
precisely due to this problem.)

Avoid the problem of potential accidental matching against an existing
worktree by instead taking advantage of find_worktree_by_path() which
matches paths deterministically, without applying any sort of magic
shorthand matching performed by find_worktree().

Reported-by: Cameron Gunnin <cameron.gunnin@synopsys.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-24 13:05:07 -08:00
Junio C Hamano 8feb47e882 Merge branch 'dl/t5520-cleanup'
Test cleanup.

* dl/t5520-cleanup:
  t5520: replace `! git` with `test_must_fail git`
  t5520: remove redundant lines in test cases
  t5520: replace $(cat ...) comparison with test_cmp
  t5520: don't put git in upstream of pipe
  t5520: test single-line files by git with test_cmp
  t5520: use test_cmp_rev where possible
  t5520: replace test -{n,z} with test-lib functions
  t5520: use test_line_count where possible
  t5520: remove spaces after redirect operator
  t5520: replace test -f with test-lib functions
  t5520: let sed open its own input
  t5520: use sq for test case names
  t5520: improve test style
  t: teach test_cmp_rev to accept ! for not-equals
  t0000: test multiple local assignment
2019-12-05 12:52:47 -08:00
Denton Liu 2c9e125b27 t: teach test_cmp_rev to accept ! for not-equals
In the case where we are using test_cmp_rev() to report not-equals, we
write `! test_cmp_rev`. However, since test_cmp_rev() contains

	r1=$(git rev-parse --verify "$1") &&
	r2=$(git rev-parse --verify "$2") &&

`! test_cmp_rev` will succeed if any of the rev-parses fail. This
behavior is not desired. We want the rev-parses to _always_ be
successful.

Rewrite test_cmp_rev() to optionally accept "!" as the first argument to
do a not-equals comparison. Rewrite `! test_cmp_rev` to `test_cmp_rev !`
in all tests to take advantage of this new functionality.

Also, rewrite the rev-parse logic to end with a `|| return 1` instead of
&&-chaining into the rev-comparison logic. This makes it obvious to
future readers that we explicitly intend on returning early if either of
the rev-parses fail.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-21 09:41:51 +09:00
Philippe Blain 4782cf2ab6 worktree: teach "add" to ignore submodule.recurse config
"worktree add" internally calls "reset --hard", but if
submodule.recurse is set, reset tries to recurse into
initialized submodules, which makes start_command try to
cd into non-existing submodule paths and die.

Fix that by making sure that the call to reset in "worktree add"
does not recurse.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-30 09:57:15 +09:00
Junio C Hamano 000bce0ee4 Merge branch 'nd/corrupt-worktrees'
"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-06-13 13:19:41 -07:00
Nguyễn Thái Ngọc Duy 1de16aecf5 worktree add: sanitize worktree names
Worktree names are based on $(basename $GIT_WORK_TREE). They aren't
significant until 3a3b9d8cde (refs: new ref types to make per-worktree
refs visible to all worktrees - 2018-10-21), where worktree name could
be part of a refname and must follow refname rules.

Update 'worktree add' code to remove special characters to follow
these rules. In the future the user will be able to specify the
worktree name by themselves if they're not happy with this dumb
character substitution.

Reported-by: Konstantin Kharlamov <hi-angel@yandex.ru>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-15 13:56:43 +09:00
Thomas Gummerer a0cc58450a move worktree tests to t24*
The 'git worktree' command used to be just another mode in 'git
checkout', namely 'git checkout --to'.  When the tests for the latter
were retrofitted for the former, the test name was adjusted, but the
test number was kept, even though the test is testing a different
command now.  t/README states: "Second digit tells the particular
command we are testing.", so 'git worktree' should have a separate
number just for itself.

Move the worktree tests to t24* to adhere to that guideline. We're
going to make use of the free'd up numbers in a subsequent commit.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-02 15:28:04 -08:00
Renamed from t/t2025-worktree-add.sh (Browse further)