Skip to content

mingw: handle absolute paths in expand_user_path() #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Documentation/config.txt
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,15 @@ pathname::
tilde expansion happens to such a string: `~/`
is expanded to the value of `$HOME`, and `~user/` to the
specified user's home directory.
+
If a path starts with `%(prefix)/`, the remainder is interpreted as a
path relative to Git's "runtime prefix", i.e. relative to the location
where Git itself was installed. For example, `%(prefix)/bin/` refers to
the directory in which the Git executable itself lives. If Git was
compiled without runtime prefix support, the compiled-in prefix will be
subsituted instead. In the unlikely event that a literal path needs to
be specified that should _not_ be expanded, it needs to be prefixed by
`./`, like so: `./%(prefix)/bin`.


Variables
Expand Down
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2826,6 +2826,11 @@ ifdef GIT_TEST_INDEX_VERSION
endif
ifdef GIT_TEST_PERL_FATAL_WARNINGS
@echo GIT_TEST_PERL_FATAL_WARNINGS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_PERL_FATAL_WARNINGS)))'\' >>$@+
endif
ifdef RUNTIME_PREFIX
@echo RUNTIME_PREFIX=\'true\' >>$@+
else
@echo RUNTIME_PREFIX=\'false\' >>$@+
endif
@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi

Expand Down
2 changes: 1 addition & 1 deletion builtin/credential-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ static char *get_socket_path(void)
{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

> From: Johannes Schindelin <[email protected]>
>
> It is not immediately clear what `expand_user_path()` means, so let's
> rename it to `interpolate_path()`. This also opens the path for
> interpolating more than just a home directory.
>
> Signed-off-by: Johannes Schindelin <[email protected]>
> ---
> ...
> diff --git a/cache.h b/cache.h
> index ba04ff8bd36..87e4cbe9c5f 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1246,7 +1246,7 @@ typedef int create_file_fn(const char *path, void *cb);
>  int raceproof_create_file(const char *path, create_file_fn fn, void *cb);
>  
>  int mkdir_in_gitdir(const char *path);
> -char *expand_user_path(const char *path, int real_home);
> +char *interpolate_path(const char *path, int real_home);

This of course breaks any topic in flight that adds more places to
use expand_user_path().

I think Fabian's "ssh signing" is not as ready as this topic, and it
can afford to wait by rebasing on top of this topic.  By the time
"ssh signing" gets into testable shape (right now, it does not pass
tests when merged to 'seen'), hopefully the "expand install-prefix"
topic may already be in 'next' if not in 'master'.

In the meantime, I am adding this band-aid at the tip of this topic
to help these two topics play together better.

Thanks.


diff --git a/cache.h b/cache.h
index 87e4cbe9c5..679a27e17c 100644
--- a/cache.h
+++ b/cache.h
@@ -1247,6 +1247,8 @@ int raceproof_create_file(const char *path, create_file_fn fn, void *cb);
 
 int mkdir_in_gitdir(const char *path);
 char *interpolate_path(const char *path, int real_home);
+/* NEEDSWORK: remove this synonym once in-flight topics have migrated */
+#define expand_user_path interpolate_path
 const char *enter_repo(const char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Fabian Stelzer wrote (reply to this):

On 27.07.21 00:05, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <[email protected]>
> writes:
>
>> From: Johannes Schindelin <[email protected]>
>>
>> It is not immediately clear what `expand_user_path()` means, so let's
>> rename it to `interpolate_path()`. This also opens the path for
>> interpolating more than just a home directory.
>>
>> Signed-off-by: Johannes Schindelin <[email protected]>
>> ---
>> ...
>> diff --git a/cache.h b/cache.h
>> index ba04ff8bd36..87e4cbe9c5f 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -1246,7 +1246,7 @@ typedef int create_file_fn(const char *path, void *cb);
>>   int raceproof_create_file(const char *path, create_file_fn fn, void *cb);
>>   
>>   int mkdir_in_gitdir(const char *path);
>> -char *expand_user_path(const char *path, int real_home);
>> +char *interpolate_path(const char *path, int real_home);
> This of course breaks any topic in flight that adds more places to
> use expand_user_path().
>
> I think Fabian's "ssh signing" is not as ready as this topic, and it
> can afford to wait by rebasing on top of this topic.  By the time
> "ssh signing" gets into testable shape (right now, it does not pass
> tests when merged to 'seen'), hopefully the "expand install-prefix"
> topic may already be in 'next' if not in 'master'.
I think the test problem is not due to my patch.
Like Ævar wrote it's "hn/reftable probably interacting with my 
ab/refs-files-cleanup" [1]
The failed tests i can see in [2] are either t0031-reftable.sh or a 
compile failure referencing reftable as well.
If i merge the ssh code into the current seen branch everything works 
fine for me. Let me know if you have any other results / CI runs that 
might help.

[1] https://lore.kernel.org/git/[email protected]/
[2] https://github.com/git/git/actions/runs/1053603028
>
> In the meantime, I am adding this band-aid at the tip of this topic
> to help these two topics play together better.
>
> Thanks.
>
>
> diff --git a/cache.h b/cache.h
> index 87e4cbe9c5..679a27e17c 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1247,6 +1247,8 @@ int raceproof_create_file(const char *path, create_file_fn fn, void *cb);
>   
>   int mkdir_in_gitdir(const char *path);
>   char *interpolate_path(const char *path, int real_home);
> +/* NEEDSWORK: remove this synonym once in-flight topics have migrated */
> +#define expand_user_path interpolate_path
>   const char *enter_repo(const char *path, int strict);
>   static inline int is_absolute_path(const char *path)
>   {

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Fabian Stelzer <[email protected]> writes:

>> I think Fabian's "ssh signing" is not as ready as this topic, and it
>> can afford to wait by rebasing on top of this topic.  By the time
>> "ssh signing" gets into testable shape (right now, it does not pass
>> tests when merged to 'seen'), hopefully the "expand install-prefix"
>> topic may already be in 'next' if not in 'master'.
> I think the test problem is not due to my patch.

I've been seeing these test failers locally, every time
fs/ssh-signing topic is merged to 'seen' (without the reftable
thing).

Test Summary Report
-------------------
t5534-push-signed.sh                             (Wstat: 256 Tests: 13 Failed: 2)
  Failed tests:  8, 12
  Non-zero exit status: 1
t7528-signed-commit-ssh.sh                       (Wstat: 256 Tests: 23 Failed: 2)
  Failed tests:  13, 17
  Non-zero exit status: 1

When reftable thing is merged, either compilation fails or t0031
fails, and I suspect that these are not due to the ssh signing
topic.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Junio C Hamano <[email protected]> writes:

> Fabian Stelzer <[email protected]> writes:
>
>>> I think Fabian's "ssh signing" is not as ready as this topic, and it
>>> can afford to wait by rebasing on top of this topic.  By the time
>>> "ssh signing" gets into testable shape (right now, it does not pass
>>> tests when merged to 'seen'), hopefully the "expand install-prefix"
>>> topic may already be in 'next' if not in 'master'.
>> I think the test problem is not due to my patch.
>
> I've been seeing these test failers locally, every time
> fs/ssh-signing topic is merged to 'seen' (without the reftable
> thing).
>
> Test Summary Report
> -------------------
> t5534-push-signed.sh                             (Wstat: 256 Tests: 13 Failed: 2)
>   Failed tests:  8, 12
>   Non-zero exit status: 1
> t7528-signed-commit-ssh.sh                       (Wstat: 256 Tests: 23 Failed: 2)
>   Failed tests:  13, 17
>   Non-zero exit status: 1
>
> When reftable thing is merged, either compilation fails or t0031
> fails, and I suspect that these are not due to the ssh signing
> topic.

Interesting.  It seems that the failure has some correlation with
the use of --root=<trash directory> option.

    $ sh t5534-push-signed.sh -i
    ok 1 - setup
    ok 2 - unsigned push does not send push certificate
    ok 3 - talking with a receiver without push certificate support
    ok 4 - push --signed fails with a receiver without push certificate
    support
    ok 5 - push --signed=1 is accepted
    ok 6 - no certificate for a signed push with no update
    ok 7 - signed push sends push certificate
    ok 8 - ssh signed push sends push certificate
    ok 9 - inconsistent push options in signed push not allowed
    ok 10 - fail without key and heed user.signingkey
    ok 11 - fail without key and heed user.signingkey x509
    ok 12 - fail without key and heed user.signingkey ssh
    ok 13 - failed atomic push does not execute GPG
    # passed all 13 test(s)
    1..13

passes just fine, but

    $ TESTPEN=/dev/shm/testpen.$$
    $ rm -fr "$TESTPEN" && mkdir "$TESTPEN"
    $ sh t5534-push-signed.sh --root=$TESTPEN -i -v

dies like this:

Initialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/.git/
expecting success of 5534.1 'setup': 
	# main, ff and noff branches pointing at the same commit
	test_tick &&
	git commit --allow-empty -m initial &&

	git checkout -b noop &&
	git checkout -b ff &&
	git checkout -b noff &&

	# noop stays the same, ff advances, noff rewrites
	test_tick &&
	git commit --allow-empty --amend -m rewritten &&
	git checkout ff &&

	test_tick &&
	git commit --allow-empty -m second

[main (root-commit) 66fe8b3] initial
 Author: A U Thor <[email protected]>
Switched to a new branch 'noop'
Switched to a new branch 'ff'
Switched to a new branch 'noff'
[noff 6391b7f] rewritten
 Author: A U Thor <[email protected]>
 Date: Thu Apr 7 15:13:13 2005 -0700
Switched to branch 'ff'
[ff 566fbd3] second
 Author: A U Thor <[email protected]>
ok 1 - setup

expecting success of 5534.2 'unsigned push does not send push certificate': 
	prepare_dst &&
	mkdir -p dst/.git/hooks &&
	write_script dst/.git/hooks/post-receive <<-\EOF &&
	# discard the update list
	cat >/dev/null
	# record the push certificate
	if test -n "${GIT_PUSH_CERT-}"
	then
		git cat-file blob $GIT_PUSH_CERT >../push-cert
	fi
	EOF

	git push dst noop ff +noff &&
	! test -f dst/push-cert

Initialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/
To dst
 * [new branch]      main -> noop
 * [new branch]      main -> ff
 * [new branch]      main -> noff
To dst
   66fe8b3..566fbd3  ff -> ff
 + 66fe8b3...6391b7f noff -> noff (forced update)
ok 2 - unsigned push does not send push certificate

expecting success of 5534.3 'talking with a receiver without push certificate support': 
	prepare_dst &&
	mkdir -p dst/.git/hooks &&
	write_script dst/.git/hooks/post-receive <<-\EOF &&
	# discard the update list
	cat >/dev/null
	# record the push certificate
	if test -n "${GIT_PUSH_CERT-}"
	then
		git cat-file blob $GIT_PUSH_CERT >../push-cert
	fi
	EOF

	git push dst noop ff +noff &&
	! test -f dst/push-cert

Initialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/
To dst
 * [new branch]      main -> noop
 * [new branch]      main -> ff
 * [new branch]      main -> noff
To dst
   66fe8b3..566fbd3  ff -> ff
 + 66fe8b3...6391b7f noff -> noff (forced update)
ok 3 - talking with a receiver without push certificate support

expecting success of 5534.4 'push --signed fails with a receiver without push certificate support': 
	prepare_dst &&
	mkdir -p dst/.git/hooks &&
	test_must_fail git push --signed dst noop ff +noff 2>err &&
	test_i18ngrep "the receiving end does not support" err

Initialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/
To dst
 * [new branch]      main -> noop
 * [new branch]      main -> ff
 * [new branch]      main -> noff
fatal: the receiving end does not support --signed push
ok 4 - push --signed fails with a receiver without push certificate support

expecting success of 5534.5 'push --signed=1 is accepted': 
	prepare_dst &&
	mkdir -p dst/.git/hooks &&
	test_must_fail git push --signed=1 dst noop ff +noff 2>err &&
	test_i18ngrep "the receiving end does not support" err

Initialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/
To dst
 * [new branch]      main -> noop
 * [new branch]      main -> ff
 * [new branch]      main -> noff
fatal: the receiving end does not support --signed push
ok 5 - push --signed=1 is accepted

checking prerequisite: GPG

mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-GPG" &&
(
	cd "$TRASH_DIRECTORY/prereq-test-dir-GPG" &&
	gpg_version=$(gpg --version 2>&1)
	test $? != 127 || exit 1

	# As said here: http://www.gnupg.org/documentation/faqs.html#q6.19
	# the gpg version 1.0.6 did not parse trust packets correctly, so for
	# that version, creation of signed tags using the generated key fails.
	case "$gpg_version" in
	"gpg (GnuPG) 1.0.6"*)
		say "Your version of gpg (1.0.6) is too buggy for testing"
		exit 1
		;;
	*)
		# Available key info:
		# * Type DSA and Elgamal, size 2048 bits, no expiration date,
		#   name and email: C O Mitter <[email protected]>
		# * Type RSA, size 2048 bits, no expiration date,
		#   name and email: Eris Discordia <[email protected]>
		# No password given, to enable non-interactive operation.
		# To generate new key:
		#	gpg --homedir /tmp/gpghome --gen-key
		# To write armored exported key to keyring:
		#	gpg --homedir /tmp/gpghome --export-secret-keys \
		#		--armor 0xDEADBEEF >> lib-gpg/keyring.gpg
		#	gpg --homedir /tmp/gpghome --export \
		#		--armor 0xDEADBEEF >> lib-gpg/keyring.gpg
		# To export ownertrust:
		#	gpg --homedir /tmp/gpghome --export-ownertrust \
		#		> lib-gpg/ownertrust
		mkdir "$GNUPGHOME" &&
		chmod 0700 "$GNUPGHOME" &&
		(gpgconf --kill gpg-agent || : ) &&
		gpg --homedir "${GNUPGHOME}" --import \
			"$TEST_DIRECTORY"/lib-gpg/keyring.gpg &&
		gpg --homedir "${GNUPGHOME}" --import-ownertrust \
			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null \
			--sign -u [email protected]
		;;
	esac

)
gpg: keybox '/dev/shm/testpen.9441/trash directory.t5534-push-signed/gpghome/pubring.kbx' created
gpg: /dev/shm/testpen.9441/trash directory.t5534-push-signed/gpghome/trustdb.gpg: trustdb created
gpg: key 13B6F51ECDDE430D: public key "C O Mitter <[email protected]>" imported
gpg: key 13B6F51ECDDE430D: secret key imported
gpg: key 61092E85B7227189: public key "Eris Discordia <[email protected]>" imported
gpg: key 61092E85B7227189: secret key imported
gpg: key 13B6F51ECDDE430D: "C O Mitter <[email protected]>" not changed
gpg: key 61092E85B7227189: "Eris Discordia <[email protected]>" not changed
gpg: Total number processed: 4
gpg:               imported: 2
gpg:              unchanged: 2
gpg:       secret keys read: 2
gpg:   secret keys imported: 2
gpg: inserting ownertrust of 6
gpg: inserting ownertrust of 3
prerequisite GPG ok
expecting success of 5534.6 'no certificate for a signed push with no update': 
	prepare_dst &&
	mkdir -p dst/.git/hooks &&
	write_script dst/.git/hooks/post-receive <<-\EOF &&
	if test -n "${GIT_PUSH_CERT-}"
	then
		git cat-file blob $GIT_PUSH_CERT >../push-cert
	fi
	EOF
	git push dst noop &&
	! test -f dst/push-cert

Initialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/
To dst
 * [new branch]      main -> noop
 * [new branch]      main -> ff
 * [new branch]      main -> noff
Everything up-to-date
ok 6 - no certificate for a signed push with no update

expecting success of 5534.7 'signed push sends push certificate': 
	prepare_dst &&
	mkdir -p dst/.git/hooks &&
	git -C dst config receive.certnonceseed sekrit &&
	write_script dst/.git/hooks/post-receive <<-\EOF &&
	# discard the update list
	cat >/dev/null
	# record the push certificate
	if test -n "${GIT_PUSH_CERT-}"
	then
		git cat-file blob $GIT_PUSH_CERT >../push-cert
	fi &&

	cat >../push-cert-status <<E_O_F
	SIGNER=${GIT_PUSH_CERT_SIGNER-nobody}
	KEY=${GIT_PUSH_CERT_KEY-nokey}
	STATUS=${GIT_PUSH_CERT_STATUS-nostatus}
	NONCE_STATUS=${GIT_PUSH_CERT_NONCE_STATUS-nononcestatus}
	NONCE=${GIT_PUSH_CERT_NONCE-nononce}
	E_O_F

	EOF

	git push --signed dst noop ff +noff &&

	(
		cat <<-\EOF &&
		SIGNER=C O Mitter <[email protected]>
		KEY=13B6F51ECDDE430D
		STATUS=G
		NONCE_STATUS=OK
		EOF
		sed -n -e "s/^nonce /NONCE=/p" -e "/^$/q" dst/push-cert
	) >expect &&

	noop=$(git rev-parse noop) &&
	ff=$(git rev-parse ff) &&
	noff=$(git rev-parse noff) &&
	grep "$noop $ff refs/heads/ff" dst/push-cert &&
	grep "$noop $noff refs/heads/noff" dst/push-cert &&
	test_cmp expect dst/push-cert-status

Initialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/
To dst
 * [new branch]      main -> noop
 * [new branch]      main -> ff
 * [new branch]      main -> noff
To dst
   66fe8b3..566fbd3  ff -> ff
 + 66fe8b3...6391b7f noff -> noff (forced update)
66fe8b3f2df5c2a6e67944af865f3a0893093d69 566fbd34a75c18947f0bcd052512caf55e7144ba refs/heads/ff
66fe8b3f2df5c2a6e67944af865f3a0893093d69 6391b7f36bc1393eab3cad0aaf8c08cdacbe78fa refs/heads/noff
ok 7 - signed push sends push certificate

checking prerequisite: GPGSSH

mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-GPGSSH" &&
(
	cd "$TRASH_DIRECTORY/prereq-test-dir-GPGSSH" &&
	ssh_version=$(ssh-keygen -Y find-principals -n "git" 2>&1)
	test $? != 127 || exit 1
	echo $ssh_version | grep -q "find-principals:missing signature file"
	test $? = 0 || exit 1;
	mkdir -p "${GNUPGHOME}" &&
	chmod 0700 "${GNUPGHOME}" &&
	ssh-keygen -t ed25519 -N "" -f "${GNUPGHOME}/ed25519_ssh_signing_key" >/dev/null &&
	ssh-keygen -t rsa -b 2048 -N "" -f "${GNUPGHOME}/rsa_2048_ssh_signing_key" >/dev/null &&
	ssh-keygen -t ed25519 -N "super_secret" -f "${GNUPGHOME}/protected_ssh_signing_key" >/dev/null &&
	find "${GNUPGHOME}" -name *ssh_signing_key.pub -exec cat {} \; | awk "{print \"\\\"principal with number \" NR \"\\\" \" \$0}" > "${GNUPGHOME}/ssh.all_valid.allowedSignersFile" &&
	cat "${GNUPGHOME}/ssh.all_valid.allowedSignersFile" &&
	ssh-keygen -t ed25519 -N "" -f "${GNUPGHOME}/untrusted_ssh_signing_key" >/dev/null

)
"principal with number 1" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIK+hDFZT7sYN3M+1ciMGLTM6pu/xmJrNDTK/vN+QIVnq [email protected]
"principal with number 2" ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDWLnqVpDYNstR7jCPKr1FaWzxt7NNw/kEV61GgKThW9S54/p/0WN+SCtUI0j7dCv/NkNhy87WNhohC9rCvZowPf/HRflHZ28vVk5d0DERCmlLBHnTq65Buyzj2R5xMtyVOBBMd+Io6tXk6GfJ6Dvq9l1wIJLv3OodOLBym3idV+8C+GOw9teTOlQWwkZD2hAt0n1Dy5WtaeGm3O2aUwRyg7KuxxmsgadBJ13a9thMYpwkS1McnwB+7oS81VXeeF0WcKAa2tGvxlg8NdBZuvvLJkl5vcISnpjt0NIuhqHzMlBIprpKujkZ5ze18YHXA52w6Y+9hG40n819EKjQfBVtD [email protected]
"principal with number 3" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAgtFx51cu1d0gzZOjIdw4M9oBYgV+tX6Bsm2L+riv/Z [email protected]
prerequisite GPGSSH ok
expecting success of 5534.8 'ssh signed push sends push certificate': 
	prepare_dst &&
	mkdir -p dst/.git/hooks &&
	git -C dst config gpg.ssh.allowedSignersFile "${SIGNING_ALLOWED_SIGNERS}" &&
	git -C dst config receive.certnonceseed sekrit &&
	write_script dst/.git/hooks/post-receive <<-\EOF &&
	# discard the update list
	cat >/dev/null
	# record the push certificate
	if test -n "${GIT_PUSH_CERT-}"
	then
		git cat-file blob $GIT_PUSH_CERT >../push-cert
	fi &&

	cat >../push-cert-status <<E_O_F
	SIGNER=${GIT_PUSH_CERT_SIGNER-nobody}
	KEY=${GIT_PUSH_CERT_KEY-nokey}
	STATUS=${GIT_PUSH_CERT_STATUS-nostatus}
	NONCE_STATUS=${GIT_PUSH_CERT_NONCE_STATUS-nononcestatus}
	NONCE=${GIT_PUSH_CERT_NONCE-nononce}
	E_O_F

	EOF

	test_config gpg.format ssh &&
	test_config user.signingkey "${SIGNING_KEY_PRIMARY}" &&
	FINGERPRINT=$(ssh-keygen -lf "${SIGNING_KEY_PRIMARY}" | awk "{print \$2;}") &&
	git push --signed dst noop ff +noff &&

	(
		cat <<-\EOF &&
		SIGNER=principal with number 1
		KEY=FINGERPRINT
		STATUS=G
		NONCE_STATUS=OK
		EOF
		sed -n -e "s/^nonce /NONCE=/p" -e "/^$/q" dst/push-cert
	) | sed -e "s|FINGERPRINT|$FINGERPRINT|" >expect &&

	noop=$(git rev-parse noop) &&
	ff=$(git rev-parse ff) &&
	noff=$(git rev-parse noff) &&
	grep "$noop $ff refs/heads/ff" dst/push-cert &&
	grep "$noop $noff refs/heads/noff" dst/push-cert &&
	test_cmp expect dst/push-cert-status

Initialized empty Git repository in /dev/shm/testpen.9441/trash directory.t5534-push-signed/dst/.git/
To dst
 * [new branch]      main -> noop
 * [new branch]      main -> ff
 * [new branch]      main -> noff
To dst
   66fe8b3..566fbd3  ff -> ff
 + 66fe8b3...6391b7f noff -> noff (forced update)
66fe8b3f2df5c2a6e67944af865f3a0893093d69 566fbd34a75c18947f0bcd052512caf55e7144ba refs/heads/ff
66fe8b3f2df5c2a6e67944af865f3a0893093d69 6391b7f36bc1393eab3cad0aaf8c08cdacbe78fa refs/heads/noff
--- expect	2021-07-28 00:11:20.863019887 +0000
+++ dst/push-cert-status	2021-07-28 00:11:20.855019156 +0000
@@ -1,4 +1,4 @@
-SIGNER=principal with number 1
+SIGNER=principal with number 3
 KEY=SHA256:Szd5rzYOrMBJFTR+gnRUu60YRVqg98UvpcSvmAm89rE
 STATUS=G
 NONCE_STATUS=OK
not ok 8 - ssh signed push sends push certificate
#	
#		prepare_dst &&
#		mkdir -p dst/.git/hooks &&
#		git -C dst config gpg.ssh.allowedSignersFile "${SIGNING_ALLOWED_SIGNERS}" &&
#		git -C dst config receive.certnonceseed sekrit &&
#		write_script dst/.git/hooks/post-receive <<-\EOF &&
#		# discard the update list
#		cat >/dev/null
#		# record the push certificate
#		if test -n "${GIT_PUSH_CERT-}"
#		then
#			git cat-file blob $GIT_PUSH_CERT >../push-cert
#		fi &&
#	
#		cat >../push-cert-status <<E_O_F
#		SIGNER=${GIT_PUSH_CERT_SIGNER-nobody}
#		KEY=${GIT_PUSH_CERT_KEY-nokey}
#		STATUS=${GIT_PUSH_CERT_STATUS-nostatus}
#		NONCE_STATUS=${GIT_PUSH_CERT_NONCE_STATUS-nononcestatus}
#		NONCE=${GIT_PUSH_CERT_NONCE-nononce}
#		E_O_F
#	
#		EOF
#	
#		test_config gpg.format ssh &&
#		test_config user.signingkey "${SIGNING_KEY_PRIMARY}" &&
#		FINGERPRINT=$(ssh-keygen -lf "${SIGNING_KEY_PRIMARY}" | awk "{print \$2;}") &&
#		git push --signed dst noop ff +noff &&
#	
#		(
#			cat <<-\EOF &&
#			SIGNER=principal with number 1
#			KEY=FINGERPRINT
#			STATUS=G
#			NONCE_STATUS=OK
#			EOF
#			sed -n -e "s/^nonce /NONCE=/p" -e "/^$/q" dst/push-cert
#		) | sed -e "s|FINGERPRINT|$FINGERPRINT|" >expect &&
#	
#		noop=$(git rev-parse noop) &&
#		ff=$(git rev-parse ff) &&
#		noff=$(git rev-parse noff) &&
#		grep "$noop $ff refs/heads/ff" dst/push-cert &&
#		grep "$noop $noff refs/heads/noff" dst/push-cert &&
#		test_cmp expect dst/push-cert-status
#	

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Junio C Hamano <[email protected]> writes:

> Junio C Hamano <[email protected]> writes:
>
>> Fabian Stelzer <[email protected]> writes:
>>
>>>> I think Fabian's "ssh signing" is not as ready as this topic, and it
>>>> can afford to wait by rebasing on top of this topic.  By the time
>>>> "ssh signing" gets into testable shape (right now, it does not pass
>>>> tests when merged to 'seen'), hopefully the "expand install-prefix"
>>>> topic may already be in 'next' if not in 'master'.
>>> I think the test problem is not due to my patch.
>>
>> I've been seeing these test failers locally, every time
>> fs/ssh-signing topic is merged to 'seen' (without the reftable
>> thing).
>>
>> Test Summary Report
>> -------------------
>> t5534-push-signed.sh                             (Wstat: 256 Tests: 13 Failed: 2)
>>   Failed tests:  8, 12
>>   Non-zero exit status: 1
>> t7528-signed-commit-ssh.sh                       (Wstat: 256 Tests: 23 Failed: 2)
>>   Failed tests:  13, 17
>>   Non-zero exit status: 1
>>
>> When reftable thing is merged, either compilation fails or t0031
>> fails, and I suspect that these are not due to the ssh signing
>> topic.
>
> Interesting.  It seems that the failure has some correlation with
> the use of --root=<trash directory> option.
>
>     $ sh t5534-push-signed.sh -i

And 7528 fails with --root set to a /dev/shm/ trash directory.

So,... this "principal with number #" differences come from the
differences in location of the trash directory?  Why?

> --- expect	2021-07-28 00:11:20.863019887 +0000
> +++ dst/push-cert-status	2021-07-28 00:11:20.855019156 +0000
> @@ -1,4 +1,4 @@
> -SIGNER=principal with number 1
> +SIGNER=principal with number 3
>  KEY=SHA256:Szd5rzYOrMBJFTR+gnRUu60YRVqg98UvpcSvmAm89rE
>  STATUS=G
>  NONCE_STATUS=OK

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Junio C Hamano <[email protected]> writes:

> Junio C Hamano <[email protected]> writes:
>
>> Junio C Hamano <[email protected]> writes:
>>
>>> Fabian Stelzer <[email protected]> writes:
>>>
>>>>> I think Fabian's "ssh signing" is not as ready as this topic, and it
>>>>> can afford to wait by rebasing on top of this topic.  By the time
>>>>> "ssh signing" gets into testable shape (right now, it does not pass
>>>>> tests when merged to 'seen'), hopefully the "expand install-prefix"
>>>>> topic may already be in 'next' if not in 'master'.
>>>> I think the test problem is not due to my patch.
>>>
>>> I've been seeing these test failers locally, every time
>>> fs/ssh-signing topic is merged to 'seen' (without the reftable
>>> thing).
>>> ...
>> Interesting.  It seems that the failure has some correlation with
>> the use of --root=<trash directory> option.
>>
>>     $ sh t5534-push-signed.sh -i
>
> And 7528 fails with --root set to a /dev/shm/ trash directory.

An update.  The same failure can be seen _without_ merging the topic
to 'seen'.  The topic by itself will fail t5534 and t7528 when run
with --root= set to somewhere:

    $ make
    $ testpen=/dev/shm/testpen.$$
    $ rm -fr "$testpen" && mkdir "$testpen"
    $ cd t
    $ sh t5534-*.sh --root=$testpen -i
    $ sh t7528-*.sh --root=$testpen -i

on the branch itself, without getting interference by any other
topic, should hopefully be an easy enough way to reproduce the
problem.

Thanks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Fabian Stelzer wrote (reply to this):

On 28.07.21 07:43, Junio C Hamano wrote:

> Junio C Hamano <[email protected]> writes:
>
>> Junio C Hamano <[email protected]> writes:
>>
>>> Junio C Hamano <[email protected]> writes:
>>>
>>>> Fabian Stelzer <[email protected]> writes:
>>>>
>>>>>> I think Fabian's "ssh signing" is not as ready as this topic, and it
>>>>>> can afford to wait by rebasing on top of this topic.  By the time
>>>>>> "ssh signing" gets into testable shape (right now, it does not pass
>>>>>> tests when merged to 'seen'), hopefully the "expand install-prefix"
>>>>>> topic may already be in 'next' if not in 'master'.
>>>>> I think the test problem is not due to my patch.
>>>> I've been seeing these test failers locally, every time
>>>> fs/ssh-signing topic is merged to 'seen' (without the reftable
>>>> thing).
>>>> ...
>>> Interesting.  It seems that the failure has some correlation with
>>> the use of --root=<trash directory> option.
>>>
>>>      $ sh t5534-push-signed.sh -i
>> And 7528 fails with --root set to a /dev/shm/ trash directory.
> An update.  The same failure can be seen _without_ merging the topic
> to 'seen'.  The topic by itself will fail t5534 and t7528 when run
> with --root= set to somewhere:
>
>      $ make
>      $ testpen=/dev/shm/testpen.$$
>      $ rm -fr "$testpen" && mkdir "$testpen"
>      $ cd t
>      $ sh t5534-*.sh --root=$testpen -i
>      $ sh t7528-*.sh --root=$testpen -i
>
> on the branch itself, without getting interference by any other
> topic, should hopefully be an easy enough way to reproduce the
> problem.
>
> Thanks.


ok, funny issue. in the ssh test setup i generated a few ssh keys for 
testing and (wanting to be clever) concatenated them with a prefixed 
principal into an allowedSigners file using find & awk.

Turns out the directory entries in /dev/shm are the other way around.

touch ./t1 ./t2 /dev/shm/t1 /dev/shm/t2

find ./ -name 't[0-9]' results in:
./t1
./t2

a find /dev/shm -name 't[0-9]' returns:
/dev/shm/t2
/dev/shm/t1

I'll change the test setup code to do this statically for each key. Not 
such a good idea to rely on the file order in the dir anyway.

Thanks

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Fabian Stelzer <[email protected]> writes:

> ok, funny issue. in the ssh test setup i generated a few ssh keys for
> testing and (wanting to be clever) concatenated them with a prefixed 
> principal into an allowedSigners file using find & awk.
>
> Turns out the directory entries in /dev/shm are the other way around.

Good finding.  Yes, relying on the order "find" discovers filesystem
entities is asking for trouble.

> I'll change the test setup code to do this statically for each
> key. Not such a good idea to rely on the file order in the dir anyway.

Thanks.

struct stat sb;
char *old_dir, *socket;
old_dir = expand_user_path("~/.git-credential-cache", 0);
old_dir = interpolate_path("~/.git-credential-cache", 0);
if (old_dir && !stat(old_dir, &sb) && S_ISDIR(sb.st_mode))
socket = xstrfmt("%s/socket", old_dir);
else
Expand Down
2 changes: 1 addition & 1 deletion builtin/credential-store.c
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix)
if (file) {
string_list_append(&fns, file);
} else {
if ((file = expand_user_path("~/.git-credentials", 0)))
if ((file = interpolate_path("~/.git-credentials", 0)))
string_list_append_nodup(&fns, file);
file = xdg_config_home("credentials");
if (file)
Expand Down
2 changes: 1 addition & 1 deletion builtin/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1542,7 +1542,7 @@ static char *launchctl_service_filename(const char *name)
struct strbuf filename = STRBUF_INIT;
strbuf_addf(&filename, "~/Library/LaunchAgents/%s.plist", name);

expanded = expand_user_path(filename.buf, 1);
expanded = interpolate_path(filename.buf, 1);
if (!expanded)
die(_("failed to expand path '%s'"), filename.buf);

Expand Down
2 changes: 1 addition & 1 deletion cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,7 @@ typedef int create_file_fn(const char *path, void *cb);
int raceproof_create_file(const char *path, create_file_fn fn, void *cb);

int mkdir_in_gitdir(const char *path);
char *expand_user_path(const char *path, int real_home);
char *interpolate_path(const char *path, int real_home);
const char *enter_repo(const char *path, int strict);
static inline int is_absolute_path(const char *path)
{
Expand Down
8 changes: 4 additions & 4 deletions config.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc
if (!path)
return config_error_nonbool("include.path");

expanded = expand_user_path(path, 0);
expanded = interpolate_path(path, 0);
if (!expanded)
return error(_("could not expand include path '%s'"), path);
path = expanded;
Expand Down Expand Up @@ -185,7 +185,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
char *expanded;
int prefix = 0;

expanded = expand_user_path(pat->buf, 1);
expanded = interpolate_path(pat->buf, 1);
if (expanded) {
strbuf_reset(pat);
strbuf_addstr(pat, expanded);
Expand Down Expand Up @@ -1270,7 +1270,7 @@ int git_config_pathname(const char **dest, const char *var, const char *value)
{
if (!value)
return config_error_nonbool(var);
*dest = expand_user_path(value, 0);
*dest = interpolate_path(value, 0);
if (!*dest)
die(_("failed to expand user dir in: '%s'"), value);
return 0;
Expand Down Expand Up @@ -1844,7 +1844,7 @@ void git_global_config(char **user_out, char **xdg_out)
char *xdg_config = NULL;

if (!user_config) {
user_config = expand_user_path("~/.gitconfig", 0);
user_config = interpolate_path("~/.gitconfig", 0);
xdg_config = xdg_config_home("config");
}

Expand Down
19 changes: 13 additions & 6 deletions path.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "packfile.h"
#include "object-store.h"
#include "lockfile.h"
#include "exec-cmd.h"

static int get_st_mode_bits(const char *path, int *mode)
{
Expand Down Expand Up @@ -719,19 +720,25 @@ static struct passwd *getpw_str(const char *username, size_t len)
}

/*
* Return a string with ~ and ~user expanded via getpw*. If buf != NULL,
* then it is a newly allocated string. Returns NULL on getpw failure or
* if path is NULL.
* Return a string with ~ and ~user expanded via getpw*. Returns NULL on getpw
* failure or if path is NULL.
*
* If real_home is true, strbuf_realpath($HOME) is used in the expansion.
* If real_home is true, strbuf_realpath($HOME) is used in the `~/` expansion.
*
* If the path starts with `%(prefix)/`, the remainder is interpreted as
* relative to where Git is installed, and expanded to the absolute path.
*/
char *expand_user_path(const char *path, int real_home)
char *interpolate_path(const char *path, int real_home)
{
struct strbuf user_path = STRBUF_INIT;
const char *to_copy = path;

if (path == NULL)
goto return_null;

if (skip_prefix(path, "%(prefix)/", &path))
return system_path(path);

if (path[0] == '~') {
const char *first_slash = strchrnul(path, '/');
const char *username = path + 1;
Expand Down Expand Up @@ -812,7 +819,7 @@ const char *enter_repo(const char *path, int strict)
strbuf_add(&validated_path, path, len);

if (used_path.buf[0] == '~') {
char *newpath = expand_user_path(used_path.buf, 0);
char *newpath = interpolate_path(used_path.buf, 0);
if (!newpath)
return NULL;
strbuf_attach(&used_path, newpath, strlen(newpath),
Expand Down
2 changes: 1 addition & 1 deletion sequencer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ N_("Your name and email address were configured automatically based\n"

static const char *implicit_ident_advice(void)
{
char *user_config = expand_user_path("~/.gitconfig", 0);
char *user_config = interpolate_path("~/.gitconfig", 0);
char *xdg_config = xdg_config_home("config");
int config_exists = file_exists(user_config) || file_exists(xdg_config);

Expand Down
26 changes: 26 additions & 0 deletions t/t0060-path-utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -525,4 +525,30 @@ test_expect_success MINGW 'is_valid_path() on Windows' '
"PRN./abc"
'

test_lazy_prereq RUNTIME_PREFIX '
test true = "$RUNTIME_PREFIX"
'

test_lazy_prereq CAN_EXEC_IN_PWD '
cp "$GIT_EXEC_PATH"/git$X ./ &&
./git rev-parse
'

test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' '
mkdir -p pretend/bin pretend/libexec/git-core &&
echo "echo HERE" | write_script pretend/libexec/git-core/git-here &&
cp "$GIT_EXEC_PATH"/git$X pretend/bin/ &&
GIT_EXEC_PATH= ./pretend/bin/git here >actual &&
echo HERE >expect &&
test_cmp expect actual'

test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' '
mkdir -p pretend/bin &&
cp "$GIT_EXEC_PATH"/git$X pretend/bin/ &&
git config yes.path "%(prefix)/yes" &&
GIT_EXEC_PATH= ./pretend/bin/git config --path yes.path >actual &&
echo "$(pwd)/pretend/yes" >expect &&
test_cmp expect actual
'

test_done