| Message ID | 19fd9c3c803a300b586c76736301a7379c4c6226.1627164413.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a03b097d6307763c6778f5a1f194fbcbd158a5f7 |
| Headers | show |
| Series | mingw: handle absolute paths in expand_user_path() | expand |
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > 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 <johannes.schindelin@gmx.de> > --- > ... > 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) {
On 27.07.21 00:05, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > >> From: Johannes Schindelin <johannes.schindelin@gmx.de> >> >> 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 <johannes.schindelin@gmx.de> >> --- >> ... >> 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/87a6mevkrx.fsf@evledraar.gmail.com/ [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) > {
Fabian Stelzer <fs@gigacodes.de> 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.
Junio C Hamano <gitster@pobox.com> writes: > Fabian Stelzer <fs@gigacodes.de> 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 <author@example.com> 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 <author@example.com> Date: Thu Apr 7 15:13:13 2005 -0700 Switched to branch 'ff' [ff 566fbd3] second Author: A U Thor <author@example.com> 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 <committer@example.com> # * Type RSA, size 2048 bits, no expiration date, # name and email: Eris Discordia <discord@example.net> # 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 committer@example.com ;; 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 <committer@example.com>" imported gpg: key 13B6F51ECDDE430D: secret key imported gpg: key 61092E85B7227189: public key "Eris Discordia <discord@example.net>" imported gpg: key 61092E85B7227189: secret key imported gpg: key 13B6F51ECDDE430D: "C O Mitter <committer@example.com>" not changed gpg: key 61092E85B7227189: "Eris Discordia <discord@example.net>" 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 <committer@example.com> 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 jch@gitster.c.googlers.com "principal with number 2" ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDWLnqVpDYNstR7jCPKr1FaWzxt7NNw/kEV61GgKThW9S54/p/0WN+SCtUI0j7dCv/NkNhy87WNhohC9rCvZowPf/HRflHZ28vVk5d0DERCmlLBHnTq65Buyzj2R5xMtyVOBBMd+Io6tXk6GfJ6Dvq9l1wIJLv3OodOLBym3idV+8C+GOw9teTOlQWwkZD2hAt0n1Dy5WtaeGm3O2aUwRyg7KuxxmsgadBJ13a9thMYpwkS1McnwB+7oS81VXeeF0WcKAa2tGvxlg8NdBZuvvLJkl5vcISnpjt0NIuhqHzMlBIprpKujkZ5ze18YHXA52w6Y+9hG40n819EKjQfBVtD jch@gitster.c.googlers.com "principal with number 3" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAgtFx51cu1d0gzZOjIdw4M9oBYgV+tX6Bsm2L+riv/Z jch@gitster.c.googlers.com 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 #
Junio C Hamano <gitster@pobox.com> writes: > Junio C Hamano <gitster@pobox.com> writes: > >> Fabian Stelzer <fs@gigacodes.de> 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
Junio C Hamano <gitster@pobox.com> writes: > Junio C Hamano <gitster@pobox.com> writes: > >> Junio C Hamano <gitster@pobox.com> writes: >> >>> Fabian Stelzer <fs@gigacodes.de> 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.
On 28.07.21 07:43, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> Junio C Hamano <gitster@pobox.com> writes: >> >>> Junio C Hamano <gitster@pobox.com> writes: >>> >>>> Fabian Stelzer <fs@gigacodes.de> 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
Fabian Stelzer <fs@gigacodes.de> 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.
diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c index 76a6ba37223..e8a74157471 100644 --- a/builtin/credential-cache.c +++ b/builtin/credential-cache.c @@ -90,7 +90,7 @@ static char *get_socket_path(void) { 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 diff --git a/builtin/credential-store.c b/builtin/credential-store.c index ae3c1ba75fe..62a4f3c2653 100644 --- a/builtin/credential-store.c +++ b/builtin/credential-store.c @@ -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) diff --git a/builtin/gc.c b/builtin/gc.c index f05d2f0a1ac..6ce5ca45126 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -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); 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); const char *enter_repo(const char *path, int strict); static inline int is_absolute_path(const char *path) { diff --git a/config.c b/config.c index f9c400ad306..c17b9797292 100644 --- a/config.c +++ b/config.c @@ -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; @@ -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); @@ -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; @@ -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"); } diff --git a/path.c b/path.c index bf329e535cf..8dc5ad2cb55 100644 --- a/path.c +++ b/path.c @@ -724,7 +724,7 @@ static struct passwd *getpw_str(const char *username, size_t len) * * If real_home is true, strbuf_realpath($HOME) is used in the `~/` expansion. */ -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; @@ -811,7 +811,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), diff --git a/sequencer.c b/sequencer.c index 0bec01cf38e..007a851804d 100644 --- a/sequencer.c +++ b/sequencer.c @@ -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);