alternates: re-allow relative paths from environment
Commit 670c359da (link_alt_odb_entry: handle normalize_path
errors, 2016-10-03) regressed the handling of relative paths
in the GIT_ALTERNATE_OBJECT_DIRECTORIES variable. It's not
entirely clear this was ever meant to work, but it _has_
worked for several years, so this commit restores the
original behavior.
When we get a path in GIT_ALTERNATE_OBJECT_DIRECTORIES, we
add it the path to the list of alternate object directories
as if it were found in objects/info/alternates, but with one
difference: we do not provide the link_alt_odb_entry()
function with a base for relative paths. That function
doesn't turn it into an absolute path, and we end up feeding
the relative path to the strbuf_normalize_path() function.
Most relative paths break out of the top-level directory
(e.g., "../foo.git/objects"), and thus normalizing fails.
Prior to 670c359da, we simply ignored the error, and due to
the way normalize_path_copy() was implemented it happened to
return the original path in this case. We then accessed the
alternate objects using this relative path.
By storing the relative path in the alt_odb list, the path
is relative to wherever we happen to be at the time we do an
object lookup. That means we look from $GIT_DIR in a bare
repository, and from the top of the worktree in a non-bare
repository.
If this were being designed from scratch, it would make
sense to pick a stable location (probably $GIT_DIR, or even
the object directory) and use that as the relative base,
turning the result into an absolute path. However, given
the history, at this point the minimal fix is to match the
pre-670c359da behavior.
We can do this simply by ignoring the error when we have no
relative base and using the original value (which we now
reliably have, thanks to strbuf_normalize_path()).
That still leaves us with a relative path that foils our
duplicate detection, and may act strangely if we ever
chdir() later in the process. We could solve that by storing
an absolute path based on getcwd(). That may be a good
future direction; for now we'll do just the minimum to fix
the regression.
The new t5615 script demonstrates the fix in its final three
tests. Since we didn't have any tests of the alternates
environment variable at all, it also adds some tests of
absolute paths.
Reported-by: Bryan Turner <bturner@atlassian.com>
Signed-off-by: Jeff King <peff@peff.net>
2016-11-08 05:50:17 +01:00
|
|
|
#!/bin/sh
|
|
|
|
|
|
|
|
|
|
test_description='handling of alternates in environment variables'
|
|
|
|
|
. ./test-lib.sh
|
|
|
|
|
|
|
|
|
|
check_obj () {
|
|
|
|
|
alt=$1; shift
|
|
|
|
|
while read obj expect
|
|
|
|
|
do
|
|
|
|
|
echo "$obj" >&3 &&
|
|
|
|
|
echo "$obj $expect" >&4
|
|
|
|
|
done 3>input 4>expect &&
|
|
|
|
|
GIT_ALTERNATE_OBJECT_DIRECTORIES=$alt \
|
|
|
|
|
git "$@" cat-file --batch-check='%(objectname) %(objecttype)' \
|
|
|
|
|
<input >actual &&
|
|
|
|
|
test_cmp expect actual
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
test_expect_success 'create alternate repositories' '
|
|
|
|
|
git init --bare one.git &&
|
|
|
|
|
one=$(echo one | git -C one.git hash-object -w --stdin) &&
|
|
|
|
|
git init --bare two.git &&
|
|
|
|
|
two=$(echo two | git -C two.git hash-object -w --stdin)
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'objects inaccessible without alternates' '
|
|
|
|
|
check_obj "" <<-EOF
|
|
|
|
|
$one missing
|
|
|
|
|
$two missing
|
|
|
|
|
EOF
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'access alternate via absolute path' '
|
2016-11-11 18:31:48 +01:00
|
|
|
check_obj "$PWD/one.git/objects" <<-EOF
|
alternates: re-allow relative paths from environment
Commit 670c359da (link_alt_odb_entry: handle normalize_path
errors, 2016-10-03) regressed the handling of relative paths
in the GIT_ALTERNATE_OBJECT_DIRECTORIES variable. It's not
entirely clear this was ever meant to work, but it _has_
worked for several years, so this commit restores the
original behavior.
When we get a path in GIT_ALTERNATE_OBJECT_DIRECTORIES, we
add it the path to the list of alternate object directories
as if it were found in objects/info/alternates, but with one
difference: we do not provide the link_alt_odb_entry()
function with a base for relative paths. That function
doesn't turn it into an absolute path, and we end up feeding
the relative path to the strbuf_normalize_path() function.
Most relative paths break out of the top-level directory
(e.g., "../foo.git/objects"), and thus normalizing fails.
Prior to 670c359da, we simply ignored the error, and due to
the way normalize_path_copy() was implemented it happened to
return the original path in this case. We then accessed the
alternate objects using this relative path.
By storing the relative path in the alt_odb list, the path
is relative to wherever we happen to be at the time we do an
object lookup. That means we look from $GIT_DIR in a bare
repository, and from the top of the worktree in a non-bare
repository.
If this were being designed from scratch, it would make
sense to pick a stable location (probably $GIT_DIR, or even
the object directory) and use that as the relative base,
turning the result into an absolute path. However, given
the history, at this point the minimal fix is to match the
pre-670c359da behavior.
We can do this simply by ignoring the error when we have no
relative base and using the original value (which we now
reliably have, thanks to strbuf_normalize_path()).
That still leaves us with a relative path that foils our
duplicate detection, and may act strangely if we ever
chdir() later in the process. We could solve that by storing
an absolute path based on getcwd(). That may be a good
future direction; for now we'll do just the minimum to fix
the regression.
The new t5615 script demonstrates the fix in its final three
tests. Since we didn't have any tests of the alternates
environment variable at all, it also adds some tests of
absolute paths.
Reported-by: Bryan Turner <bturner@atlassian.com>
Signed-off-by: Jeff King <peff@peff.net>
2016-11-08 05:50:17 +01:00
|
|
|
$one blob
|
|
|
|
|
$two missing
|
|
|
|
|
EOF
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
test_expect_success 'access multiple alternates' '
|
2016-11-11 18:31:48 +01:00
|
|
|
check_obj "$PWD/one.git/objects:$PWD/two.git/objects" <<-EOF
|
alternates: re-allow relative paths from environment
Commit 670c359da (link_alt_odb_entry: handle normalize_path
errors, 2016-10-03) regressed the handling of relative paths
in the GIT_ALTERNATE_OBJECT_DIRECTORIES variable. It's not
entirely clear this was ever meant to work, but it _has_
worked for several years, so this commit restores the
original behavior.
When we get a path in GIT_ALTERNATE_OBJECT_DIRECTORIES, we
add it the path to the list of alternate object directories
as if it were found in objects/info/alternates, but with one
difference: we do not provide the link_alt_odb_entry()
function with a base for relative paths. That function
doesn't turn it into an absolute path, and we end up feeding
the relative path to the strbuf_normalize_path() function.
Most relative paths break out of the top-level directory
(e.g., "../foo.git/objects"), and thus normalizing fails.
Prior to 670c359da, we simply ignored the error, and due to
the way normalize_path_copy() was implemented it happened to
return the original path in this case. We then accessed the
alternate objects using this relative path.
By storing the relative path in the alt_odb list, the path
is relative to wherever we happen to be at the time we do an
object lookup. That means we look from $GIT_DIR in a bare
repository, and from the top of the worktree in a non-bare
repository.
If this were being designed from scratch, it would make
sense to pick a stable location (probably $GIT_DIR, or even
the object directory) and use that as the relative base,
turning the result into an absolute path. However, given
the history, at this point the minimal fix is to match the
pre-670c359da behavior.
We can do this simply by ignoring the error when we have no
relative base and using the original value (which we now
reliably have, thanks to strbuf_normalize_path()).
That still leaves us with a relative path that foils our
duplicate detection, and may act strangely if we ever
chdir() later in the process. We could solve that by storing
an absolute path based on getcwd(). That may be a good
future direction; for now we'll do just the minimum to fix
the regression.
The new t5615 script demonstrates the fix in its final three
tests. Since we didn't have any tests of the alternates
environment variable at all, it also adds some tests of
absolute paths.
Reported-by: Bryan Turner <bturner@atlassian.com>
Signed-off-by: Jeff King <peff@peff.net>
2016-11-08 05:50:17 +01:00
|
|
|
$one blob
|
|
|
|
|
$two blob
|
|
|
|
|
EOF
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
# bare paths are relative from $GIT_DIR
|
|
|
|
|
test_expect_success 'access alternate via relative path (bare)' '
|
|
|
|
|
git init --bare bare.git &&
|
|
|
|
|
check_obj "../one.git/objects" -C bare.git <<-EOF
|
|
|
|
|
$one blob
|
|
|
|
|
EOF
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
# non-bare paths are relative to top of worktree
|
|
|
|
|
test_expect_success 'access alternate via relative path (worktree)' '
|
|
|
|
|
git init worktree &&
|
|
|
|
|
check_obj "../one.git/objects" -C worktree <<-EOF
|
|
|
|
|
$one blob
|
|
|
|
|
EOF
|
|
|
|
|
'
|
|
|
|
|
|
|
|
|
|
# path is computed after moving to top-level of worktree
|
|
|
|
|
test_expect_success 'access alternate via relative path (subdir)' '
|
|
|
|
|
mkdir subdir &&
|
|
|
|
|
check_obj "one.git/objects" -C subdir <<-EOF
|
|
|
|
|
$one blob
|
|
|
|
|
EOF
|
|
|
|
|
'
|
|
|
|
|
|
alternates: accept double-quoted paths
We read lists of alternates from objects/info/alternates
files (delimited by newline), as well as from the
GIT_ALTERNATE_OBJECT_DIRECTORIES environment variable
(delimited by colon or semi-colon, depending on the
platform).
There's no mechanism for quoting the delimiters, so it's
impossible to specify an alternate path that contains a
colon in the environment, or one that contains a newline in
a file. We've lived with that restriction for ages because
both alternates and filenames with colons are relatively
rare, and it's only a problem when the two meet. But since
722ff7f87 (receive-pack: quarantine objects until
pre-receive accepts, 2016-10-03), which builds on the
alternates system, every push causes the receiver to set
GIT_ALTERNATE_OBJECT_DIRECTORIES internally.
It would be convenient to have some way to quote the
delimiter so that we can represent arbitrary paths.
The simplest thing would be an escape character before a
quoted delimiter (e.g., "\:" as a literal colon). But that
creates a backwards compatibility problem: any path which
uses that escape character is now broken, and we've just
shifted the problem. We could choose an unlikely escape
character (e.g., something from the non-printable ASCII
range), but that's awkward to use.
Instead, let's treat names as unquoted unless they begin
with a double-quote, in which case they are interpreted via
our usual C-stylke quoting rules. This also breaks
backwards-compatibility, but in a smaller way: it only
matters if your file has a double-quote as the very _first_
character in the path (whereas an escape character is a
problem anywhere in the path). It's also consistent with
many other parts of git, which accept either a bare pathname
or a double-quoted one, and the sender can choose to quote
or not as required.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-12 20:52:22 +01:00
|
|
|
# set variables outside test to avoid quote insanity; the \057 is '/',
|
|
|
|
|
# which doesn't need quoting, but just confirms that de-quoting
|
|
|
|
|
# is working.
|
|
|
|
|
quoted='"one.git\057objects"'
|
|
|
|
|
unquoted='two.git/objects'
|
|
|
|
|
test_expect_success 'mix of quoted and unquoted alternates' '
|
|
|
|
|
check_obj "$quoted:$unquoted" <<-EOF
|
|
|
|
|
$one blob
|
|
|
|
|
$two blob
|
|
|
|
|
'
|
|
|
|
|
|
2016-12-21 22:33:43 +01:00
|
|
|
test_expect_success !MINGW 'broken quoting falls back to interpreting raw' '
|
alternates: accept double-quoted paths
We read lists of alternates from objects/info/alternates
files (delimited by newline), as well as from the
GIT_ALTERNATE_OBJECT_DIRECTORIES environment variable
(delimited by colon or semi-colon, depending on the
platform).
There's no mechanism for quoting the delimiters, so it's
impossible to specify an alternate path that contains a
colon in the environment, or one that contains a newline in
a file. We've lived with that restriction for ages because
both alternates and filenames with colons are relatively
rare, and it's only a problem when the two meet. But since
722ff7f87 (receive-pack: quarantine objects until
pre-receive accepts, 2016-10-03), which builds on the
alternates system, every push causes the receiver to set
GIT_ALTERNATE_OBJECT_DIRECTORIES internally.
It would be convenient to have some way to quote the
delimiter so that we can represent arbitrary paths.
The simplest thing would be an escape character before a
quoted delimiter (e.g., "\:" as a literal colon). But that
creates a backwards compatibility problem: any path which
uses that escape character is now broken, and we've just
shifted the problem. We could choose an unlikely escape
character (e.g., something from the non-printable ASCII
range), but that's awkward to use.
Instead, let's treat names as unquoted unless they begin
with a double-quote, in which case they are interpreted via
our usual C-stylke quoting rules. This also breaks
backwards-compatibility, but in a smaller way: it only
matters if your file has a double-quote as the very _first_
character in the path (whereas an escape character is a
problem anywhere in the path). It's also consistent with
many other parts of git, which accept either a bare pathname
or a double-quoted one, and the sender can choose to quote
or not as required.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-12 20:52:22 +01:00
|
|
|
mv one.git \"one.git &&
|
|
|
|
|
check_obj \"one.git/objects <<-EOF
|
|
|
|
|
$one blob
|
|
|
|
|
EOF
|
|
|
|
|
'
|
|
|
|
|
|
alternates: re-allow relative paths from environment
Commit 670c359da (link_alt_odb_entry: handle normalize_path
errors, 2016-10-03) regressed the handling of relative paths
in the GIT_ALTERNATE_OBJECT_DIRECTORIES variable. It's not
entirely clear this was ever meant to work, but it _has_
worked for several years, so this commit restores the
original behavior.
When we get a path in GIT_ALTERNATE_OBJECT_DIRECTORIES, we
add it the path to the list of alternate object directories
as if it were found in objects/info/alternates, but with one
difference: we do not provide the link_alt_odb_entry()
function with a base for relative paths. That function
doesn't turn it into an absolute path, and we end up feeding
the relative path to the strbuf_normalize_path() function.
Most relative paths break out of the top-level directory
(e.g., "../foo.git/objects"), and thus normalizing fails.
Prior to 670c359da, we simply ignored the error, and due to
the way normalize_path_copy() was implemented it happened to
return the original path in this case. We then accessed the
alternate objects using this relative path.
By storing the relative path in the alt_odb list, the path
is relative to wherever we happen to be at the time we do an
object lookup. That means we look from $GIT_DIR in a bare
repository, and from the top of the worktree in a non-bare
repository.
If this were being designed from scratch, it would make
sense to pick a stable location (probably $GIT_DIR, or even
the object directory) and use that as the relative base,
turning the result into an absolute path. However, given
the history, at this point the minimal fix is to match the
pre-670c359da behavior.
We can do this simply by ignoring the error when we have no
relative base and using the original value (which we now
reliably have, thanks to strbuf_normalize_path()).
That still leaves us with a relative path that foils our
duplicate detection, and may act strangely if we ever
chdir() later in the process. We could solve that by storing
an absolute path based on getcwd(). That may be a good
future direction; for now we'll do just the minimum to fix
the regression.
The new t5615 script demonstrates the fix in its final three
tests. Since we didn't have any tests of the alternates
environment variable at all, it also adds some tests of
absolute paths.
Reported-by: Bryan Turner <bturner@atlassian.com>
Signed-off-by: Jeff King <peff@peff.net>
2016-11-08 05:50:17 +01:00
|
|
|
test_done
|