Allow writing invalid OIDs into refs for testing purposes
cover Allow writing invalid OIDs into refs for testing purposes
this covers a few cases of direct filesystem access to the ref database in tests.
Han-Wen Nienhuys (2):
refs: update comment.
refs: allow skipping OID verification
refs.h | 8 ++++++- refs/files-backend.c | 53 +++++++++++++++++++++++++------------------- t/t1006-cat-file.sh | 5 ++--- t/t3800-mktag.sh | 6 +++-- 4 files changed, 43 insertions(+), 29 deletions(-)
base-commit: 5f439a0ecfb4657100ec1e56ef9c6eca963b5a94
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1147%2Fhanwen%2Fskip-verification-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1147/hanwen/skip-verification-v1
Pull-Request: https://github.com/git/git/pull/1147
Signature · 1 line
gitgitgadget
PATCH 1/2 refs: update comment.
From: Han-Wen Nienhuys <[email protected]>
REF_IS_PRUNING is right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 "refs: tidy up and adjust visibility of the ref_update flags").
--- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
refs/files-backend.c +1 -2
@@ -16,8 +16,7 @@* This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, -* REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in -* `ref_update::flags`. +* or REF_HAVE_OLD, which are also stored in `ref_update::flags`. */ /*
Signature · 1 line
gitgitgadget
PATCH 2/2 refs: allow skipping OID verification
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This introduces the REF_SKIP_OID_VERIFICATION flag, which lets the ref-store test helper write non-existent or unparsable objects into the ref storage.
Use this to make t1006 and t3800 independent of the files storage backend.
--- refs.h | 8 ++++++- refs/files-backend.c | 50 +++++++++++++++++++++++++------------------- t/t1006-cat-file.sh | 5 ++--- t/t3800-mktag.sh | 6 ++++-- 4 files changed, 42 insertions(+), 27 deletions(-)
refs.h +7 -1
@@ -611,12 +611,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);*/ #define REF_FORCE_CREATE_REFLOG (1 << 1) +/* +* Blindly write an object_id. This is useful for testing data corruption +* scenarios. +*/ +#define REF_SKIP_OID_VERIFICATION (1 << 10) +/* * Bitmask of all of the flags that are allowed to be passed in to * ref_transaction_update() and friends: */ #define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG) + (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION)/* * Add a reference update to transaction. `new_oid` is the value that
refs/files-backend.c +29 -21
91 lines not on this page · raw message
t/t1006-cat-file.sh +2 -3
@@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus # types. head=$(git rev-parse --verify HEAD) && - test_when_finished "rm -rf .git/refs/replace" && - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID 1024 && cat >expect <<-EOF && commit
t/t3800-mktag.sh +4 -2
@@ -72,7 +72,8 @@ check_verify_failure () { # Manually create the broken, we cannot do it with # update-ref - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && # Unlike fsck-ing unreachable content above, this # will always fail.
@@ -83,7 +84,8 @@ check_verify_failure () { # Make sure the earlier test created it for us git rev-parse "$bad_tag" && - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected && git -C bad-tag for-each-ref "$tag_ref" >actual &&
Signature · 1 line
gitgitgadget
Re: [PATCH 2/2] refs: allow skipping OID verification
Quoted 4 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
This introduces the
REF_SKIP_OID_VERIFICATIONflag, which lets the ref-store test helper write non-existent or unparsable objects into the ref storage.
Yay.
t/t1006-cat-file.sh test_expect_success 'the --allow-unknown-type option does not consider replaceme · line 456
Quoted 13 lines
t/t1006-cat-file.sh +2
-3@@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus # types. head=$(git rev-parse --verify HEAD) && - test_when_finished "rm -rf .git/refs/replace" && - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID 1024 &&
Double yay. Tests that are independent of backend is indeed much nicer.
v2 cover Allow writing invalid OIDs into refs for testing purposes
In reply to Han-Wen Nienhuys via GitGitGadgetthis covers a few cases of direct filesystem access to the ref database in tests.
v2 folds in more fixes for making t1430 reftable-proof.
* add REF_SKIP_REFNAME_VERIFICATION as well
* upgrade some more bits of t1430
Han-Wen Nienhuys (6):
test-ref-store: plug memory leak in cmd_delete_refs
refs: update comment.
refs: allow skipping OID verification
refs: add REF_SKIP_REFNAME_VERIFICATION flag
t1430: remove refs using test-tool
t1430: create valid symrefs using test-helper
refs.c | 7 +-- refs.h | 16 ++++++- refs/files-backend.c | 53 ++++++++++++---------- t/helper/test-ref-store.c | 6 ++- t/t1006-cat-file.sh | 5 +-- t/t1430-bad-ref-name.sh | 95 ++++++++++++++++++++------------------- t/t3800-mktag.sh | 6 ++- 7 files changed, 108 insertions(+), 80 deletions(-)
base-commit: 35151cf0720460a897cde9b8039af364743240e7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1147%2Fhanwen%2Fskip-verification-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1147/hanwen/skip-verification-v2
Pull-Request: https://github.com/git/git/pull/1147
Range-diff vs v1:
-: ----------- > 1: 7fa5c247c8b test-ref-store: plug memory leak in cmd_delete_refs 1: b83bfda2443 = 2: 82971ddbfcf refs: update comment. 2: 900cea2ade9 = 3: 93fe3f03fb2 refs: allow skipping OID verification -: ----------- > 4: 0a297f0c3e8 refs: add REF_SKIP_REFNAME_VERIFICATION flag -: ----------- > 5: 06c8ff7df15 t1430: remove refs using test-tool -: ----------- > 6: 3c100702fda t1430: create valid symrefs using test-helper
Signature · 1 line
gitgitgadget
PATCH v2 1/6 test-ref-store: plug memory leak in cmd_delete_refs
From: Han-Wen Nienhuys <[email protected]>
--- t/helper/test-ref-store.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
t/helper/test-ref-store.c +4 -2
@@ -86,11 +86,13 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv)unsigned int flags =arg_flags (*argv++,"flags" );const char *msg = *argv++;struct string_list refnames =STRING_LIST_INIT_NODUP ; - +int result;while (*argv)string_list_append (&refnames, *argv++); -return refs_delete_refs (refs, msg, &refnames, flags); + result =refs_delete_refs (refs, msg, &refnames, flags); +string_list_clear (&refnames,0 ); +return result; }static int cmd_rename_ref (struct ref_store *refs,const char **argv)
Signature · 1 line
gitgitgadget
Re: [PATCH v2 1/6] test-ref-store: plug memory leak in cmd_delete_refs
Quoted 12 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
Signed-off-by Han-Wen Nienhuys <[email protected]>--- t/helper/test-ref-store.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)t/helper/test-ref-store.c +0
-0
t/helper/test-ref-store.c static int cmd_delete_refs(struct ref_store *refs, const char **argv) · line 89
Quoted 5 lines
t/helper/test-ref-store.c +1
-1@@ -86,11 +86,13 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv)unsigned int flags =arg_flags (*argv++,"flags" );const char *msg = *argv++;struct string_list refnames =STRING_LIST_INIT_NODUP ; - +int result;
Thanks, but let's not lose the blank line between the declarations and the first statement, which serves for readability.
Quoted 10 lines
t/helper/test-ref-store.c +3
-1while (*argv)string_list_append (&refnames, *argv++); -return refs_delete_refs (refs, msg, &refnames, flags); + result =refs_delete_refs (refs, msg, &refnames, flags); +string_list_clear (&refnames,0 ); +return result; }static int cmd_rename_ref (struct ref_store *refs,const char **argv)
PATCH v2 2/6 refs: update comment.
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
REF_IS_PRUNING is right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 "refs: tidy up and adjust visibility of the ref_update flags").
--- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
refs/files-backend.c +1 -2
@@ -16,8 +16,7 @@* This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, -* REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in -* `ref_update::flags`. +* or REF_HAVE_OLD, which are also stored in `ref_update::flags`. */ /*
Signature · 1 line
gitgitgadget
Re: [PATCH v2 2/6] refs: update comment.
Quoted 5 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
REF_IS_PRUNINGis right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit5ac95fee(Nov 5, 2017 "refs: tidy up and adjust visibility of theref_updateflags").
Thanks for carefully reading and correcting.
Quoted 20 lines
Signed-off-by Han-Wen Nienhuys <[email protected]>--- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)refs/files-backend.c +1
-2@@ -16,8 +16,7 @@* This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, -* REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in -* `ref_update::flags`. +* or REF_HAVE_OLD, which are also stored in `ref_update::flags`. */ /*
PATCH v2 3/6 refs: allow skipping OID verification
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This introduces the REF_SKIP_OID_VERIFICATION flag, which lets the ref-store test helper write non-existent or unparsable objects into the ref storage.
Use this to make t1006 and t3800 independent of the files storage backend.
--- refs.h | 8 ++++++- refs/files-backend.c | 50 +++++++++++++++++++++++++------------------- t/t1006-cat-file.sh | 5 ++--- t/t3800-mktag.sh | 6 ++++-- 4 files changed, 42 insertions(+), 27 deletions(-)
refs.h +7 -1
@@ -611,12 +611,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);*/ #define REF_FORCE_CREATE_REFLOG (1 << 1) +/* +* Blindly write an object_id. This is useful for testing data corruption +* scenarios. +*/ +#define REF_SKIP_OID_VERIFICATION (1 << 10) +/* * Bitmask of all of the flags that are allowed to be passed in to * ref_transaction_update() and friends: */ #define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG) + (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION)/* * Add a reference update to transaction. `new_oid` is the value that
refs/files-backend.c +29 -21
91 lines not on this page · raw message
t/t1006-cat-file.sh +2 -3
@@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus # types. head=$(git rev-parse --verify HEAD) && - test_when_finished "rm -rf .git/refs/replace" && - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID 1024 && cat >expect <<-EOF && commit
t/t3800-mktag.sh +4 -2
@@ -72,7 +72,8 @@ check_verify_failure () { # Manually create the broken, we cannot do it with # update-ref - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && # Unlike fsck-ing unreachable content above, this # will always fail.
@@ -83,7 +84,8 @@ check_verify_failure () { # Make sure the earlier test created it for us git rev-parse "$bad_tag" && - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 && printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected && git -C bad-tag for-each-ref "$tag_ref" >actual &&
Signature · 1 line
gitgitgadget
Re: [PATCH v2 3/6] refs: allow skipping OID verification
refs.h struct ref_transaction *ref_transaction_begin(struct strbuf *err); · line 618
Quoted 5 lines · Han-Wen Nienhuys via GitGitGadget
refs.h +5
-0+/* +* Blindly write an object_id. This is useful for testing data corruption +* scenarios. +*/ +#define REF_SKIP_OID_VERIFICATION (1 << 10)
That makes sense. Readers would expect that this flag is passed only from test-helper but not in the regular code, at least without an option like "git update-ref --skip-oid-verification ..."?
After applying these 6 patches, I do not seem to find anybody who actually flips this bit on. Perhaps I missed a step or so?
PATCH v2 4/6 refs: add REF_SKIP_REFNAME_VERIFICATION flag
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
Use this flag with the test-helper in t1430, to avoid direct writes to the ref database.
--- refs.c | 7 ++--- refs.h | 10 +++++-- t/t1430-bad-ref-name.sh | 59 ++++++++++++++++++++++------------------- 3 files changed, 44 insertions(+), 32 deletions(-)
refs.c +4 -3
@@ -1078,9 +1078,10 @@ int ref_transaction_update(struct ref_transaction *transaction, {assert (err); -if ((new_oid && !is_null_oid (new_oid)) ? -check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : - !refname_is_safe (refname)) { +if (!(flags &REF_SKIP_REFNAME_VERIFICATION ) && + ((new_oid && !is_null_oid (new_oid)) ? +check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : + !refname_is_safe (refname))) {strbuf_addf (err,_ ("refusing to update ref with bad name '%s'" ), refname);return -1 ;
refs.h +8 -2
@@ -617,12 +617,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);*/ #define REF_SKIP_OID_VERIFICATION (1 << 10) +/* +* Skip verifying refname. This is useful for testing data corruption scenarios. +*/ +#define REF_SKIP_REFNAME_VERIFICATION (1 << 11) +/* * Bitmask of all of the flags that are allowed to be passed in to * ref_transaction_update() and friends: */ -#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION) +#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ + (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION | \ + REF_SKIP_REFNAME_VERIFICATION)/* * Add a reference update to transaction. `new_oid` is the value that
t/t1430-bad-ref-name.sh +32 -27
156 lines not on this page · raw message
Signature · 1 line
gitgitgadget
Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
Quoted 4 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
Use this flag with the test-helper in t1430, to avoid direct writes to the ref database.
OK, this is similar to 3/6 where invalid object names are allowed. This is about allowing invalid refnames. Makes sense.
Quoted 12 lines
Signed-off-by Han-Wen Nienhuys <[email protected]>--- refs.c | 7 ++--- refs.h | 10 +++++-- t/t1430-bad-ref-name.sh | 59 ++++++++++++++++++++++------------------- 3 files changed, 44 insertions(+), 32 deletions(-)refs.c +0
-0
refs.c int ref_transaction_update(struct ref_transaction *transaction, · line 1084
Quoted 10 lines
refs.c +4
-3@@ -1078,9 +1078,10 @@ int ref_transaction_update(struct ref_transaction *transaction, {assert (err); -if ((new_oid && !is_null_oid (new_oid)) ? -check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : - !refname_is_safe (refname)) { +if (!(flags &REF_SKIP_REFNAME_VERIFICATION ) && + ((new_oid && !is_null_oid (new_oid)) ? +check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : + !refname_is_safe (refname))) {
OK. Initially my eyes went "Huh?", but it was just re-indentation that fooled them.
refs.c struct ref_transaction *ref_transaction_begin(struct strbuf *err); · line 623
Quoted 8 lines
refs.c +4
-0@@ -617,12 +617,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);*/ #define REF_SKIP_OID_VERIFICATION (1 << 10) +/* +* Skip verifying refname. This is useful for testing data corruption scenarios. +*/ +#define REF_SKIP_REFNAME_VERIFICATION (1 << 11)
OK.
Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
In reply to Han-Wen Nienhuys via GitGitGadgetrefs.c int ref_transaction_update(struct ref_transaction *transaction, · line 1084
Quoted 4 lines · Han-Wen Nienhuys via GitGitGadget
refs.c +4
-0+if (!(flags &REF_SKIP_REFNAME_VERIFICATION ) && + ((new_oid && !is_null_oid (new_oid)) ? +check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : + !refname_is_safe (refname))) {
So, if somebody passes REF_SKIP_REFNAME_VERIFICATION in flags, we will not do the check.
Again, like 3/6, this new bit is flipped on by test-helper somewhere? Again I do not see anybody doing so in these 6 patches, but I should double check.
Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
Quoted 11 lines · Junio C Hamano
refs.c int ref_transaction_update(struct ref_transaction *transaction, · line 1084
Quoted 4 lines
refs.c +4
-0+if (!(flags &REF_SKIP_REFNAME_VERIFICATION ) && + ((new_oid && !is_null_oid (new_oid)) ? +check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : + !refname_is_safe (refname))) {So, if somebody passes
REF_SKIP_REFNAME_VERIFICATIONin flags, we will not do the check.Again, like 3/6, this new bit is flipped on by test-helper somewhere? Again I do not see anybody doing so in these 6 patches, but I should double check.
The test helper takes the flag as an argument, in decimal. If you look for 2048, you should find it.
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
Quoted 9 lines · Han-Wen Nienhuys
Quoted 8 lines · Junio C Hamano
refs.c int ref_transaction_update(struct ref_transaction *transaction, · line 1084
Quoted 4 lines
refs.c +4
-0+if (!(flags &REF_SKIP_REFNAME_VERIFICATION ) && + ((new_oid && !is_null_oid (new_oid)) ? +check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : + !refname_is_safe (refname))) {So, if somebody passes
REF_SKIP_REFNAME_VERIFICATIONin flags, we will not do the check.
Quoted 6 lines
Quoted 3 lines
Again, like 3/6, this new bit is flipped on by test-helper somewhere? Again I do not see anybody doing so in these 6 patches, but I should double check.
The test helper takes the flag as an argument, in decimal. If you look for 2048, you should find it.
Awful---when the symbolic constants change in the code, the test will silently break?
It has been this way since 80f2a609 (t/helper: add test-ref-store to test ref-store functions, 2017-03-26), so it is nothing new, but at some point, we should do a better job. Even if it is used only as a tool for testing, we shouldn't have to force developers to write in assembly ;-)
Perhaps when the dust settles after this series graduates to be a part of released version. It may be a good bite-sized microproject material for aspiring new developers.
Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
Quoted 5 lines · Junio C Hamano
Quoted 2 lines
The test helper takes the flag as an argument, in decimal. If you look for 2048, you should find it.
Awful---when the symbolic constants change in the code, the test will silently break?
Agreed, this is quite nasty.
Quoted 5 lines
It has been this way since
80f2a609(t/helper: add test-ref-store to test ref-store functions, 2017-03-26), so it is nothing new, but at some point, we should do a better job. Even if it is used only as a tool for testing, we shouldn't have to force developers to write in assembly ;-)
Sort of. The code to pass the flags was added then, but nobody was using it until now.
At least for ref updates. Symref creation allowed "1" for force, but that is a true boolean. It looks like some pack-refs calls pass "3" for PACK_REFS_PRUNE|PACK_REFS_ALL.
So it may be considered a "new" issue in that sense.
Quoted 3 lines
Perhaps when the dust settles after this series graduates to be a part of released version. It may be a good bite-sized microproject material for aspiring new developers.
It is annoying to have to plumb through the names of the flags, but in practice I don't think we need all (or even most) of them. And it's equally annoying not to be able to grep for the flags. I already had a hard enough time just grepping for the callers, and resorted to:
t/helper/test-ref-store.c +2 -0
@@ -271,6 +271,8 @@ int cmd__ref_store(int argc, const char **argv)const char *func;struct command *cmd; +trace_argv_printf (argv,"trace: test-tool:" ); +setup_git_directory (); argv =get_store (argv +1 , &refs);
to find them.
-Peff
Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
Quoted 10 lines · Jeff King
Quoted 5 lines · Junio C Hamano
Quoted 2 lines
The test helper takes the flag as an argument, in decimal. If you look for 2048, you should find it.
Awful---when the symbolic constants change in the code, the test will silently break?
Agreed, this is quite nasty.
I've added parsing symbolic constants in v3.
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
Quoted 7 lines · Han-Wen Nienhuys
Quoted 6 lines · Jeff King
Quoted 3 lines · Junio C Hamano
Quoted 2 lines
The test helper takes the flag as an argument, in decimal. If you look for 2048, you should find it.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Awful---when the symbolic constants change in the code, the test will silently break?
Agreed, this is quite nasty.
I've added parsing symbolic constants in v3.
Thanks for going an extra mile.
PATCH v2 5/6 t1430: remove refs using test-tool
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- t/t1430-bad-ref-name.sh | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
t/t1430-bad-ref-name.sh +12 -12
@@ -154,7 +154,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' git branch shadow one && test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow && - test_when_finished "rm -f .git/refs/tags/shadow" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && test_cmp expect actual &&
@@ -165,9 +165,9 @@ test_expect_success 'for-each-ref emits warnings for broken names' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git for-each-ref >output 2>error && ! grep -e "broken\.\.\.ref" output && ! grep -e "badname" output &&
@@ -204,7 +204,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output &&
@@ -216,7 +216,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
@@ -225,7 +225,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output &&
@@ -234,7 +234,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok test_expect_success 'branch -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
@@ -246,7 +246,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && test_must_be_empty output &&
@@ -255,7 +255,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_expect_success 'update-ref --no-deref -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output &&
@@ -264,7 +264,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name test_expect_success 'branch -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output &&
@@ -273,7 +273,7 @@ test_expect_success 'branch -d can delete symref with broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output &&
@@ -282,7 +282,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br test_expect_success 'branch -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output &&
Signature · 1 line
gitgitgadget
PATCH v2 6/6 t1430: create valid symrefs using test-helper
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This still leaves some other direct filesystem access. Currently, the files backend does not allow invalidly named symrefs. Fixes for this are currently in the 'seen' branch
--- t/t1430-bad-ref-name.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
t/t1430-bad-ref-name.sh +6 -6
@@ -153,7 +153,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && git branch shadow one && test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && - printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow && + test-tool ref-store main create-symref refs/tags/shadow refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err &&
@@ -203,7 +203,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -215,7 +215,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -224,7 +224,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -233,7 +233,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok ' test_expect_success 'branch -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -245,7 +245,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref &&
Signature · 1 line
gitgitgadget
Quoted 7 lines · Han-Wen Nienhuys via GitGitGadget
this covers a few cases of direct filesystem access to the ref database in tests.
v2 folds in more fixes for making t1430 reftable-proof.
* add
REF_SKIP_REFNAME_VERIFICATIONas well
* upgrade some more bits of t1430
Thanks, will replace.
v3 cover Allow writing invalid OIDs into refs for testing purposes
In reply to Han-Wen Nienhuys via GitGitGadgetthis covers a few cases of direct filesystem access to the ref database in tests.
v3:
* parse symbolic constants in ref-store test-helper.
Han-Wen Nienhuys (8):
test-ref-store: remove force-create argument for create-reflog
test-ref-store: parse symbolic flag constants
test-ref-store: plug memory leak in cmd_delete_refs
refs: update comment.
refs: introduce REF_SKIP_OID_VERIFICATION flag
refs: introduce REF_SKIP_REFNAME_VERIFICATION flag
t1430: remove refs using test-tool
t1430: create valid symrefs using test-helper
refs.c | 7 +-- refs.h | 16 +++++- refs/files-backend.c | 53 ++++++++++--------- t/helper/test-ref-store.c | 78 ++++++++++++++++++++++++---- t/t1006-cat-file.sh | 5 +- t/t1405-main-ref-store.sh | 8 ++- t/t1406-submodule-ref-store.sh | 2 +- t/t1430-bad-ref-name.sh | 93 ++++++++++++++++++---------------- t/t3800-mktag.sh | 6 ++- 9 files changed, 175 insertions(+), 93 deletions(-)
base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1147%2Fhanwen%2Fskip-verification-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1147/hanwen/skip-verification-v3
Pull-Request: https://github.com/git/git/pull/1147
Range-diff vs v2:
-: ----------- > 1: 3649ef6d0fa test-ref-store: remove force-create argument for create-reflog
-: ----------- > 2: 3cdebd2dbca test-ref-store: parse symbolic flag constants
1: 7fa5c247c8b ! 3: 536005d65a2 test-ref-store: plug memory leak in cmd_delete_refs
@@ Commit message
## t/helper/test-ref-store.c ##
@@ t/helper/test-ref-store.c: static int cmd_delete_refs(struct ref_store *refs, const char **argv)
- unsigned int flags = arg_flags(*argv++, "flags");
+ unsigned int flags = arg_flags(*argv++, "flags", transaction_flags);
const char *msg = *argv++;
struct string_list refnames = STRING_LIST_INIT_NODUP;
--
+ int result;
+
while (*argv)
string_list_append(&refnames, *argv++); 2: 82971ddbfcf = 4: 466b4451015 refs: update comment.
3: 93fe3f03fb2 ! 5: 89d692a34b8 refs: allow skipping OID verification
@@ Metadata
Author: Han-Wen Nienhuys <[email protected]>
## Commit message ##
- refs: allow skipping OID verification
+ refs: introduce REF_SKIP_OID_VERIFICATION flag
- This introduces the REF_SKIP_OID_VERIFICATION flag, which lets the ref-store
- test helper write non-existent or unparsable objects into the ref storage.
+ This lets the ref-store test helper write non-existent or unparsable objects
+ into the ref storage.
Use this to make t1006 and t3800 independent of the files storage backend.
@@ refs/files-backend.c: static int lock_ref_for_update(struct files_ref_store *ref
/*
+ ## t/helper/test-ref-store.c ##
+@@ t/helper/test-ref-store.c: static int cmd_create_symref(struct ref_store *refs, const char **argv)
+ static struct flag_definition transaction_flags[] = {
+ FLAG_DEF(REF_NO_DEREF),
+ FLAG_DEF(REF_FORCE_CREATE_REFLOG),
++ FLAG_DEF(REF_SKIP_OID_VERIFICATION),
+ { NULL, 0 },
+ };
+
+
## t/t1006-cat-file.sh ##
@@ t/t1006-cat-file.sh: test_expect_success 'the --allow-unknown-type option does not consider replaceme
# Create it manually, as "git replace" will die on bogus
@@ t/t1006-cat-file.sh: test_expect_success 'the --allow-unknown-type option does n
- mkdir -p .git/refs/replace &&
- echo $head >.git/refs/replace/$bogus_short_sha1 &&
+ test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" &&
-+ test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID 1024 &&
++ test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID REF_SKIP_OID_VERIFICATION &&
cat >expect <<-EOF &&
commit
@@ t/t3800-mktag.sh: check_verify_failure () {
# update-ref
- echo "$bad_tag" >"bad-tag/$tag_ref" &&
+ test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" &&
-+ test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 &&
++ test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION &&
# Unlike fsck-ing unreachable content above, this
# will always fail.
@@ t/t3800-mktag.sh: check_verify_failure () {
- echo "$bad_tag" >"bad-tag/$tag_ref" &&
+ test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" &&
-+ test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID 1024 &&
++ test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION &&
printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected &&
git -C bad-tag for-each-ref "$tag_ref" >actual &&
4: 0a297f0c3e8 ! 6: af9e94935bf refs: add REF_SKIP_REFNAME_VERIFICATION flag
@@ Metadata
Author: Han-Wen Nienhuys <[email protected]>
## Commit message ##
- refs: add REF_SKIP_REFNAME_VERIFICATION flag
+ refs: introduce REF_SKIP_REFNAME_VERIFICATION flag
Use this flag with the test-helper in t1430, to avoid direct writes to the ref
database.
@@ refs.h: struct ref_transaction *ref_transaction_begin(struct strbuf *err);
/*
* Add a reference update to transaction. `new_oid` is the value that
+ ## t/helper/test-ref-store.c ##
+@@ t/helper/test-ref-store.c: static struct flag_definition transaction_flags[] = {
+ FLAG_DEF(REF_NO_DEREF),
+ FLAG_DEF(REF_FORCE_CREATE_REFLOG),
+ FLAG_DEF(REF_SKIP_OID_VERIFICATION),
++ FLAG_DEF(REF_SKIP_REFNAME_VERIFICATION),
+ { NULL, 0 },
+ };
+
+
## t/t1430-bad-ref-name.sh ##
@@ t/t1430-bad-ref-name.sh: TEST_PASSES_SANITIZE_LEAK=true
@@ t/t1430-bad-ref-name.sh: test_expect_success 'fast-import: fail on invalid branc
test_expect_success 'git branch shows badly named ref as warning' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
git branch >output 2>error &&
test_i18ngrep -e "ignoring ref with broken name refs/heads/broken\.\.\.ref" error &&
! grep -e "broken\.\.\.ref" output
@@ t/t1430-bad-ref-name.sh: test_expect_success 'fast-import: fail on invalid branc
test_expect_success 'branch -d can delete badly named ref' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
git branch -d broken...ref &&
git branch >output 2>error &&
! grep -e "broken\.\.\.ref" error &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete badly named r
test_expect_success 'branch -D can delete badly named ref' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
git branch -D broken...ref &&
git branch >output 2>error &&
! grep -e "broken\.\.\.ref" error &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -D cannot delete absolute p
test_expect_success 'git branch cannot create a badly named ref' '
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
test_must_fail git branch broken...ref &&
git branch >output 2>error &&
! grep -e "broken\.\.\.ref" error &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'git branch cannot create a badly n
test_expect_success 'branch -m cannot rename to a bad ref name' '
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
test_might_fail git branch -D goodref &&
git branch goodref &&
test_must_fail git branch -m goodref broken...ref &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -m cannot rename to a bad r
test_expect_failure 'branch -m can rename from a bad ref name' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
git branch -m broken...ref renamed &&
test_cmp_rev main renamed &&
git branch >output 2>error &&
@@ t/t1430-bad-ref-name.sh: test_expect_failure 'branch -m can rename from a bad re
test_expect_success 'push cannot create a badly named ref' '
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
test_must_fail git push "file://$(pwd)" HEAD:refs/heads/broken...ref &&
git branch >output 2>error &&
! grep -e "broken\.\.\.ref" error &&
@@ t/t1430-bad-ref-name.sh: test_expect_failure 'push --mirror can delete badly nam
test_commit two &&
git checkout --detach &&
- cp .git/refs/heads/main .git/refs/heads/broken...ref
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION
) &&
git -C src push --mirror "file://$top/dest" &&
git -C dest branch >output 2>error &&
@@ t/t1430-bad-ref-name.sh: test_expect_failure 'push --mirror can delete badly nam
test_expect_success 'rev-parse skips symref pointing to broken name' '
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
git branch shadow one &&
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow &&
test_when_finished "rm -f .git/refs/tags/shadow" &&
git rev-parse --verify one >expect &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'rev-parse skips symref pointing to
test_expect_success 'for-each-ref emits warnings for broken names' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
test_when_finished "rm -f .git/refs/heads/badname" &&
printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'for-each-ref emits warnings for br
test_expect_success 'update-ref -d can delete broken name' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
git update-ref -d refs/heads/broken...ref >output 2>error &&
test_must_be_empty output &&
test_must_be_empty error &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken na
test_expect_success 'branch -d can delete broken name' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
git branch -d broken...ref >output 2>error &&
test_i18ngrep "Deleted branch broken...ref (was broken)" output &&
test_must_be_empty error &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete broken name'
test_expect_success 'update-ref --no-deref -d can delete symref to broken name' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
test_when_finished "rm -f .git/refs/heads/badname" &&
git update-ref --no-deref -d refs/heads/badname >output 2>error &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet
test_expect_success 'branch -d can delete symref to broken name' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
test_when_finished "rm -f .git/refs/heads/badname" &&
git branch -d badname >output 2>error &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete dangling symr
test_expect_success 'update-ref -d can delete broken name through symref' '
- cp .git/refs/heads/main .git/refs/heads/broken...ref &&
- test_when_finished "rm -f .git/refs/heads/broken...ref" &&
-+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
++ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
test_when_finished "rm -f .git/refs/heads/badname" &&
git update-ref -d refs/heads/badname >output 2>error &&
5: 06c8ff7df15 ! 7: d4f1d209468 t1430: remove refs using test-tool
@@ Commit message
## t/t1430-bad-ref-name.sh ##
@@ t/t1430-bad-ref-name.sh: test_expect_success 'rev-parse skips symref pointing to broken name' '
git branch shadow one &&
- test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow &&
- test_when_finished "rm -f .git/refs/tags/shadow" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/tags/shadow" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" &&
git rev-parse --verify one >expect &&
git rev-parse --verify shadow >actual 2>err &&
test_cmp expect actual &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'for-each-ref emits warnings for broken names' '
- test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
- test_when_finished "rm -f .git/refs/heads/badname" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
- test_when_finished "rm -f .git/refs/heads/broken...symref" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git for-each-ref >output 2>error &&
! grep -e "broken\.\.\.ref" output &&
! grep -e "badname" output &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete symref to broken name'
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
- test_when_finished "rm -f .git/refs/heads/badname" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git update-ref --no-deref -d refs/heads/badname >output 2>error &&
test_path_is_missing .git/refs/heads/badname &&
test_must_be_empty output &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to broken name' '
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
- test_when_finished "rm -f .git/refs/heads/badname" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git branch -d badname >output 2>error &&
test_path_is_missing .git/refs/heads/badname &&
test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to bro
test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' '
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
- test_when_finished "rm -f .git/refs/heads/badname" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git update-ref --no-deref -d refs/heads/badname >output 2>error &&
test_path_is_missing .git/refs/heads/badname &&
test_must_be_empty output &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet
test_expect_success 'branch -d can delete dangling symref to broken name' '
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
- test_when_finished "rm -f .git/refs/heads/badname" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git branch -d badname >output 2>error &&
test_path_is_missing .git/refs/heads/badname &&
test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken name through symref' '
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
- test_when_finished "rm -f .git/refs/heads/badname" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git update-ref -d refs/heads/badname >output 2>error &&
test_path_is_missing .git/refs/heads/broken...ref &&
test_must_be_empty output &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken na
test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '
printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
- test_when_finished "rm -f .git/refs/heads/broken...symref" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
test_path_is_missing .git/refs/heads/broken...symref &&
test_must_be_empty output &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet
test_expect_success 'branch -d can delete symref with broken name' '
printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref &&
- test_when_finished "rm -f .git/refs/heads/broken...symref" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git branch -d broken...symref >output 2>error &&
test_path_is_missing .git/refs/heads/broken...symref &&
test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref with b
test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '
printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
- test_when_finished "rm -f .git/refs/heads/broken...symref" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&
test_path_is_missing .git/refs/heads/broken...symref &&
test_must_be_empty output &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet
test_expect_success 'branch -d can delete dangling symref with broken name' '
printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref &&
- test_when_finished "rm -f .git/refs/heads/broken...symref" &&
-+ test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...symref" &&
++ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" &&
git branch -d broken...symref >output 2>error &&
test_path_is_missing .git/refs/heads/broken...symref &&
test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output && 6: 3c100702fda ! 8: ca7f8aea3d9 t1430: create valid symrefs using test-helper
@@ Commit message
## t/t1430-bad-ref-name.sh ##
@@ t/t1430-bad-ref-name.sh: test_expect_success 'rev-parse skips symref pointing to broken name' '
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
git branch shadow one &&
- test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
- printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow &&
+ test-tool ref-store main create-symref refs/tags/shadow refs/heads/broken...ref msg &&
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/tags/shadow" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" &&
git rev-parse --verify one >expect &&
git rev-parse --verify shadow >actual 2>err &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete symref to broken name'
- test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
- printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git update-ref --no-deref -d refs/heads/badname >output 2>error &&
test_path_is_missing .git/refs/heads/badname &&
-@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to broken name' '
- test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
+@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete symref to broken name'
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
+ test_expect_success 'branch -d can delete symref to broken name' '
+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+-
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
- printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git branch -d badname >output 2>error &&
test_path_is_missing .git/refs/heads/badname &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to broken name' '
@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete symref to bro
test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' '
- printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git update-ref --no-deref -d refs/heads/badname >output 2>error &&
test_path_is_missing .git/refs/heads/badname &&
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok
@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref --no-deref -d can delet
test_expect_success 'branch -d can delete dangling symref to broken name' '
- printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git branch -d badname >output 2>error &&
test_path_is_missing .git/refs/heads/badname &&
-@@ t/t1430-bad-ref-name.sh: test_expect_success 'update-ref -d can delete broken name through symref' '
- test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID 2048 &&
+@@ t/t1430-bad-ref-name.sh: test_expect_success 'branch -d can delete dangling symref to broken name' '
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/broken...ref" &&
+ test_expect_success 'update-ref -d can delete broken name through symref' '
+ test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&
+-
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" &&
- printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname &&
+ test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg &&
- test_when_finished "test-tool ref-store main delete-refs 1 msg refs/heads/badname" &&
+ test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" &&
git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref &&
Signature · 1 line
gitgitgadget
PATCH v3 1/8 test-ref-store: remove force-create argument for create-reflog
From: Han-Wen Nienhuys <[email protected]>
Nobody uses force_create=0, so this flag is unnecessary.
--- t/helper/test-ref-store.c | 2 +- t/t1405-main-ref-store.sh | 5 ++--- t/t1406-submodule-ref-store.sh | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-)
t/helper/test-ref-store.c +1 -1
@@ -182,9 +182,9 @@ static int cmd_reflog_exists(struct ref_store *refs, const char **argv)static int cmd_create_reflog (struct ref_store *refs,const char **argv) {const char *refname =notnull (*argv++,"refname" ); -int force_create =arg_flags (*argv++,"force-create" );struct strbuf err =STRBUF_INIT ;int ret; +int force_create =1 ; ret =refs_create_reflog (refs, refname, force_create, &err);if (err.len)
t/t1405-main-ref-store.sh +2 -3
@@ -35,8 +35,7 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' ' git rev-parse FOO -- && git rev-parse refs/tags/new-tag -- && m=$(git rev-parse main) && - REF_NO_DEREF=1 && - $RUN delete-refs $REF_NO_DEREF nothing FOO refs/tags/new-tag && + $RUN delete-refs REF_NO_DEREF nothing FOO refs/tags/new-tag && test_must_fail git rev-parse --symbolic-full-name FOO && test_must_fail git rev-parse FOO -- && test_must_fail git rev-parse refs/tags/new-tag --
@@ -108,7 +107,7 @@ test_expect_success 'delete_reflog(HEAD)' ' ' test_expect_success 'create-reflog(HEAD)' ' - $RUN create-reflog HEAD 1 && + $RUN create-reflog HEAD && git reflog exists HEAD '
t/t1406-submodule-ref-store.sh +1 -1
@@ -92,7 +92,7 @@ test_expect_success 'delete_reflog() not allowed' ' ' test_expect_success 'create-reflog() not allowed' ' - test_must_fail $RUN create-reflog HEAD 1 + test_must_fail $RUN create-reflog HEAD ' test_done
Signature · 1 line
gitgitgadget
PATCH v3 2/8 test-ref-store: parse symbolic flag constants
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This lets tests use REF_XXXX constants instead of hardcoded integers. The flag names should be separated by a ','.
--- t/helper/test-ref-store.c | 69 +++++++++++++++++++++++++++++++++++---- t/t1405-main-ref-store.sh | 3 +- 2 files changed, 63 insertions(+), 9 deletions(-)
t/helper/test-ref-store.c +62 -7
@@ -5,6 +5,50 @@#include "object-store.h" #include "repository.h" +struct flag_definition { +const char *name; +uint64_t mask; +}; + +#define FLAG_DEF (x) \ + { \ +#x, (x) \ + } + +static unsigned int parse_flags (const char *str,struct flag_definition *defs) +{ +struct string_list masks =STRING_LIST_INIT_DUP ; +int i =0 ; +unsigned int result =0 ; + +if (!strcmp (str,"0" )) +return 0 ; + +string_list_split (&masks, str,',' ,64 ); +for (; i < masks.nr; i++) { +const char *name = masks.items[i].string; +struct flag_definition *def = defs; +int found =0 ; +while (def->name) { +if (!strcmp (def->name, name)) { + result |= def->mask; + found =1 ; +break ; + } + def++; + } +if (!found) +die ("unknown flag \"%s\"" , name); + } + +string_list_clear (&masks,0 ); +return result; +} + +static struct flag_definition empty_flags[] = { + {NULL ,0 }, +}; +static const char *notnull (const char *arg,const char *name) {if (!arg)
@@ -12,9 +56,10 @@ static const char *notnull(const char *arg, const char *name)return arg; } -static unsigned int arg_flags (const char *arg,const char *name) +static unsigned int arg_flags (const char *arg,const char *name, +struct flag_definition *defs) { -return atoi (notnull (arg, name)); +return parse_flags (notnull (arg, name), defs); }static const char **get_store (const char **argv,struct ref_store **refs)
@@ -64,10 +109,14 @@ static const char **get_store(const char **argv, struct ref_store **refs)return argv +1 ; } +static struct flag_definition pack_flags[] = { +FLAG_DEF (PACK_REFS_PRUNE ), +FLAG_DEF (PACK_REFS_ALL ), +};static int cmd_pack_refs (struct ref_store *refs,const char **argv) { -unsigned int flags =arg_flags (*argv++,"flags" ); +unsigned int flags =arg_flags (*argv++,"flags" , pack_flags);return refs_pack_refs (refs, flags); }
@@ -81,9 +130,15 @@ static int cmd_create_symref(struct ref_store *refs, const char **argv)return refs_create_symref (refs, refname, target, logmsg); } +static struct flag_definition transaction_flags[] = { +FLAG_DEF (REF_NO_DEREF ), +FLAG_DEF (REF_FORCE_CREATE_REFLOG ), + {NULL ,0 }, +}; +static int cmd_delete_refs (struct ref_store *refs,const char **argv) { -unsigned int flags =arg_flags (*argv++,"flags" ); +unsigned int flags =arg_flags (*argv++,"flags" , transaction_flags);const char *msg = *argv++;struct string_list refnames =STRING_LIST_INIT_NODUP ;
@@ -120,7 +175,7 @@ static int cmd_resolve_ref(struct ref_store *refs, const char **argv) {struct object_id oid = *null_oid ();const char *refname =notnull (*argv++,"refname" ); -int resolve_flags =arg_flags (*argv++,"resolve-flags" ); +int resolve_flags =arg_flags (*argv++,"resolve-flags" , empty_flags);int flags;const char *ref;int ignore_errno;
@@ -209,7 +264,7 @@ static int cmd_delete_ref(struct ref_store *refs, const char **argv)const char *msg =notnull (*argv++,"msg" );const char *refname =notnull (*argv++,"refname" );const char *sha1_buf =notnull (*argv++,"old-sha1" ); -unsigned int flags =arg_flags (*argv++,"flags" ); +unsigned int flags =arg_flags (*argv++,"flags" , transaction_flags);struct object_id old_oid;if (get_oid_hex (sha1_buf, &old_oid))
@@ -224,7 +279,7 @@ static int cmd_update_ref(struct ref_store *refs, const char **argv)const char *refname =notnull (*argv++,"refname" );const char *new_sha1_buf =notnull (*argv++,"new-sha1" );const char *old_sha1_buf =notnull (*argv++,"old-sha1" ); -unsigned int flags =arg_flags (*argv++,"flags" ); +unsigned int flags =arg_flags (*argv++,"flags" , transaction_flags);struct object_id old_oid;struct object_id new_oid;
t/t1405-main-ref-store.sh +1 -2
@@ -17,8 +17,7 @@ test_expect_success 'setup' ' test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' ' N=`find .git/refs -type f | wc -l` && test "$N" != 0 && - ALL_OR_PRUNE_FLAG=3 && - $RUN pack-refs ${ALL_OR_PRUNE_FLAG} && + $RUN pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL && N=`find .git/refs -type f` && test -z "$N" '
Signature · 1 line
gitgitgadget
Re: [PATCH v3 2/8] test-ref-store: parse symbolic flag constants
Quoted 4 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
This lets tests use
REF_XXXXconstants instead of hardcoded integers. The flag names should be separated by a ','.
So much nicer. Thank you for cleaning this up.
One small bug:
Quoted 12 lines
t/helper/test-ref-store.c +12
-0+static unsigned int parse_flags (const char *str,struct flag_definition *defs) +{ +struct string_list masks =STRING_LIST_INIT_DUP ; +int i =0 ; +unsigned int result =0 ; + +if (!strcmp (str,"0" )) +return 0 ; + +string_list_split (&masks, str,',' ,64 ); +for (; i < masks.nr; i++) { +const char *name = masks.items[i].string;
t/helper/test-ref-store.c line 39
Quoted 10 lines
t/helper/test-ref-store.c +10
-0+struct flag_definition *def = defs; +int found =0 ; +while (def->name) { +if (!strcmp (def->name, name)) { + result |= def->mask; + found =1 ; +break ; + } + def++; + }
We assume defs ends with a NULL entry here...
t/helper/test-ref-store.c line 50
Quoted 3 lines
t/helper/test-ref-store.c +3
-0+static struct flag_definition empty_flags[] = { + {NULL ,0 }, +};
...and this one does so...
t/helper/test-ref-store.c static const char **get_store(const char **argv, struct ref_store **refs) · line 115
Quoted 4 lines
t/helper/test-ref-store.c +4
-0+static struct flag_definition pack_flags[] = { +FLAG_DEF (PACK_REFS_PRUNE ), +FLAG_DEF (PACK_REFS_ALL ), +};
...but this one does not. So passing an unknown flag will result in us walking off the end of the list.
t/helper/test-ref-store.c static int cmd_create_symref(struct ref_store *refs, const char **argv) · line 137
Quoted 5 lines
t/helper/test-ref-store.c +5
-0+static struct flag_definition transaction_flags[] = { +FLAG_DEF (REF_NO_DEREF ), +FLAG_DEF (REF_FORCE_CREATE_REFLOG ), + {NULL ,0 }, +};
This one is good. We might want to drop the trailing comma on the NULL entries, to make it more clear that they have to come last.
The other option would be using ARRAY_SIZE() instead of a NULL terminator, but of course that has to happen in the caller. Which means either extra work there, or yet another macro. :)
-Peff
PATCH v3 3/8 test-ref-store: plug memory leak in cmd_delete_refs
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- t/helper/test-ref-store.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
t/helper/test-ref-store.c +4 -1
@@ -141,11 +141,14 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv)unsigned int flags =arg_flags (*argv++,"flags" , transaction_flags);const char *msg = *argv++;struct string_list refnames =STRING_LIST_INIT_NODUP ; +int result;while (*argv)string_list_append (&refnames, *argv++); -return refs_delete_refs (refs, msg, &refnames, flags); + result =refs_delete_refs (refs, msg, &refnames, flags); +string_list_clear (&refnames,0 ); +return result; }static int cmd_rename_ref (struct ref_store *refs,const char **argv)
Signature · 1 line
gitgitgadget
PATCH v3 4/8 refs: update comment.
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
REF_IS_PRUNING is right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 "refs: tidy up and adjust visibility of the ref_update flags").
--- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
refs/files-backend.c +1 -2
@@ -16,8 +16,7 @@* This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, -* REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in -* `ref_update::flags`. +* or REF_HAVE_OLD, which are also stored in `ref_update::flags`. */ /*
Signature · 1 line
gitgitgadget
PATCH v3 5/8 refs: introduce REF_SKIP_OID_VERIFICATION flag
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This lets the ref-store test helper write non-existent or unparsable objects into the ref storage.
Use this to make t1006 and t3800 independent of the files storage backend.
--- refs.h | 8 ++++++- refs/files-backend.c | 50 +++++++++++++++++++++++---------------- t/helper/test-ref-store.c | 1 + t/t1006-cat-file.sh | 5 ++-- t/t3800-mktag.sh | 6 +++-- 5 files changed, 43 insertions(+), 27 deletions(-)
refs.h +7 -1
@@ -615,12 +615,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);*/ #define REF_FORCE_CREATE_REFLOG (1 << 1) +/* +* Blindly write an object_id. This is useful for testing data corruption +* scenarios. +*/ +#define REF_SKIP_OID_VERIFICATION (1 << 10) +/* * Bitmask of all of the flags that are allowed to be passed in to * ref_transaction_update() and friends: */ #define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG) + (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION)/* * Add a reference update to transaction. `new_oid` is the value that
refs/files-backend.c +29 -21
91 lines not on this page · raw message
t/helper/test-ref-store.c +1 -0
@@ -133,6 +133,7 @@ static int cmd_create_symref(struct ref_store *refs, const char **argv)static struct flag_definition transaction_flags[] = {FLAG_DEF (REF_NO_DEREF ),FLAG_DEF (REF_FORCE_CREATE_REFLOG ), +FLAG_DEF (REF_SKIP_OID_VERIFICATION ), {NULL ,0 }, };
t/t1006-cat-file.sh +2 -3
@@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus # types. head=$(git rev-parse --verify HEAD) && - test_when_finished "rm -rf .git/refs/replace" && - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID REF_SKIP_OID_VERIFICATION && cat >expect <<-EOF && commit
t/t3800-mktag.sh +4 -2
@@ -72,7 +72,8 @@ check_verify_failure () { # Manually create the broken, we cannot do it with # update-ref - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && # Unlike fsck-ing unreachable content above, this # will always fail.
@@ -83,7 +84,8 @@ check_verify_failure () { # Make sure the earlier test created it for us git rev-parse "$bad_tag" && - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected && git -C bad-tag for-each-ref "$tag_ref" >actual &&
Signature · 1 line
gitgitgadget
PATCH v3 6/8 refs: introduce REF_SKIP_REFNAME_VERIFICATION flag
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
Use this flag with the test-helper in t1430, to avoid direct writes to the ref database.
--- refs.c | 7 +++-- refs.h | 10 +++++-- t/helper/test-ref-store.c | 1 + t/t1430-bad-ref-name.sh | 59 +++++++++++++++++++++------------------ 4 files changed, 45 insertions(+), 32 deletions(-)
refs.c +4 -3
@@ -1083,9 +1083,10 @@ int ref_transaction_update(struct ref_transaction *transaction, {assert (err); -if ((new_oid && !is_null_oid (new_oid)) ? -check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : - !refname_is_safe (refname)) { +if (!(flags &REF_SKIP_REFNAME_VERIFICATION ) && + ((new_oid && !is_null_oid (new_oid)) ? +check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : + !refname_is_safe (refname))) {strbuf_addf (err,_ ("refusing to update ref with bad name '%s'" ), refname);return -1 ;
refs.h +8 -2
@@ -621,12 +621,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);*/ #define REF_SKIP_OID_VERIFICATION (1 << 10) +/* +* Skip verifying refname. This is useful for testing data corruption scenarios. +*/ +#define REF_SKIP_REFNAME_VERIFICATION (1 << 11) +/* * Bitmask of all of the flags that are allowed to be passed in to * ref_transaction_update() and friends: */ -#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION) +#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ + (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION | \ + REF_SKIP_REFNAME_VERIFICATION)/* * Add a reference update to transaction. `new_oid` is the value that
t/helper/test-ref-store.c +1 -0
@@ -134,6 +134,7 @@ static struct flag_definition transaction_flags[] = {FLAG_DEF (REF_NO_DEREF ),FLAG_DEF (REF_FORCE_CREATE_REFLOG ),FLAG_DEF (REF_SKIP_OID_VERIFICATION ), +FLAG_DEF (REF_SKIP_REFNAME_VERIFICATION ), { NULL, 0 }, };
t/t1430-bad-ref-name.sh +32 -27
156 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v3 7/8 t1430: remove refs using test-tool
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- t/t1430-bad-ref-name.sh | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
t/t1430-bad-ref-name.sh +12 -12
@@ -154,7 +154,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' git branch shadow one && test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow && - test_when_finished "rm -f .git/refs/tags/shadow" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && test_cmp expect actual &&
@@ -165,9 +165,9 @@ test_expect_success 'for-each-ref emits warnings for broken names' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git for-each-ref >output 2>error && ! grep -e "broken\.\.\.ref" output && ! grep -e "badname" output &&
@@ -204,7 +204,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output &&
@@ -216,7 +216,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
@@ -225,7 +225,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output &&
@@ -234,7 +234,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok test_expect_success 'branch -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
@@ -246,7 +246,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && test_must_be_empty output &&
@@ -255,7 +255,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_expect_success 'update-ref --no-deref -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output &&
@@ -264,7 +264,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name test_expect_success 'branch -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output &&
@@ -273,7 +273,7 @@ test_expect_success 'branch -d can delete symref with broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output &&
@@ -282,7 +282,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br test_expect_success 'branch -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output &&
Signature · 1 line
gitgitgadget
PATCH v3 8/8 t1430: create valid symrefs using test-helper
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This still leaves some other direct filesystem access. Currently, the files backend does not allow invalidly named symrefs. Fixes for this are currently in the 'seen' branch
--- t/t1430-bad-ref-name.sh | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
t/t1430-bad-ref-name.sh +6 -8
@@ -153,7 +153,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && git branch shadow one && test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && - printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow && + test-tool ref-store main create-symref refs/tags/shadow refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err &&
@@ -203,7 +203,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -213,9 +213,8 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test_expect_success 'branch -d can delete symref to broken name' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && - test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -224,7 +223,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -233,7 +232,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok ' test_expect_success 'branch -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -243,9 +242,8 @@ test_expect_success 'branch -d can delete dangling symref to broken name' ' test_expect_success 'update-ref -d can delete broken name through symref' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && - test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref &&
Signature · 1 line
gitgitgadget
v4 cover Allow writing invalid OIDs into refs for testing purposes
In reply to Han-Wen Nienhuys via GitGitGadgetthis covers a few cases of direct filesystem access to the ref database in tests.
v4:
* fix stack OOB read due to missing array terminator.
Han-Wen Nienhuys (8):
test-ref-store: remove force-create argument for create-reflog
test-ref-store: parse symbolic flag constants
test-ref-store: plug memory leak in cmd_delete_refs
refs: update comment.
refs: introduce REF_SKIP_OID_VERIFICATION flag
refs: introduce REF_SKIP_REFNAME_VERIFICATION flag
t1430: remove refs using test-tool
t1430: create valid symrefs using test-helper
refs.c | 7 +-- refs.h | 16 +++++- refs/files-backend.c | 53 ++++++++++--------- t/helper/test-ref-store.c | 75 +++++++++++++++++++++++---- t/t1006-cat-file.sh | 5 +- t/t1405-main-ref-store.sh | 8 ++- t/t1406-submodule-ref-store.sh | 2 +- t/t1430-bad-ref-name.sh | 93 ++++++++++++++++++---------------- t/t3800-mktag.sh | 6 ++- 9 files changed, 172 insertions(+), 93 deletions(-)
base-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1147%2Fhanwen%2Fskip-verification-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1147/hanwen/skip-verification-v4
Pull-Request: https://github.com/git/git/pull/1147
Range-diff vs v3:
1: 3649ef6d0fa = 1: 3649ef6d0fa test-ref-store: remove force-create argument for create-reflog
2: 3cdebd2dbca ! 2: 865458bc0cb test-ref-store: parse symbolic flag constants
@@ t/helper/test-ref-store.c
+ return result;
+}
+
-+static struct flag_definition empty_flags[] = {
-+ { NULL, 0 },
-+};
++static struct flag_definition empty_flags[] = { { NULL, 0 } };
+
static const char *notnull(const char *arg, const char *name)
{
@@ t/helper/test-ref-store.c: static const char **get_store(const char **argv, stru
return argv + 1;
}
-+static struct flag_definition pack_flags[] = {
-+ FLAG_DEF(PACK_REFS_PRUNE),
-+ FLAG_DEF(PACK_REFS_ALL),
-+};
++static struct flag_definition pack_flags[] = { FLAG_DEF(PACK_REFS_PRUNE),
++ FLAG_DEF(PACK_REFS_ALL),
++ { NULL, 0 } };
static int cmd_pack_refs(struct ref_store *refs, const char **argv)
{
@@ t/helper/test-ref-store.c: static int cmd_create_symref(struct ref_store *refs,
+static struct flag_definition transaction_flags[] = {
+ FLAG_DEF(REF_NO_DEREF),
+ FLAG_DEF(REF_FORCE_CREATE_REFLOG),
-+ { NULL, 0 },
++ { NULL, 0 }
+};
+
static int cmd_delete_refs(struct ref_store *refs, const char **argv)
3: 536005d65a2 = 3: dcd4e9f5f26 test-ref-store: plug memory leak in cmd_delete_refs
4: 466b4451015 = 4: 67d22211ac4 refs: update comment.
5: 89d692a34b8 ! 5: a8713dd59ee refs: introduce REF_SKIP_OID_VERIFICATION flag @@ t/helper/test-ref-store.c: static int cmd_create_symref(struct ref_store *refs,
FLAG_DEF(REF_NO_DEREF),
FLAG_DEF(REF_FORCE_CREATE_REFLOG),
+ FLAG_DEF(REF_SKIP_OID_VERIFICATION),
- { NULL, 0 },
+ { NULL, 0 }
};
6: af9e94935bf ! 6: 577726dc9df refs: introduce REF_SKIP_REFNAME_VERIFICATION flag
@@ t/helper/test-ref-store.c: static struct flag_definition transaction_flags[] = {
FLAG_DEF(REF_FORCE_CREATE_REFLOG), FLAG_DEF(REF_SKIP_OID_VERIFICATION),
+ FLAG_DEF(REF_SKIP_REFNAME_VERIFICATION),
- { NULL, 0 },
+ { NULL, 0 }
};
7: d4f1d209468 = 7: d85cfa3fee1 t1430: remove refs using test-tool 8: ca7f8aea3d9 = 8: 7597620ec83 t1430: create valid symrefs using test-helper
Signature · 1 line
gitgitgadget
PATCH v4 1/8 test-ref-store: remove force-create argument for create-reflog
From: Han-Wen Nienhuys <[email protected]>
Nobody uses force_create=0, so this flag is unnecessary.
--- t/helper/test-ref-store.c | 2 +- t/t1405-main-ref-store.sh | 5 ++--- t/t1406-submodule-ref-store.sh | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-)
t/helper/test-ref-store.c +1 -1
@@ -182,9 +182,9 @@ static int cmd_reflog_exists(struct ref_store *refs, const char **argv)static int cmd_create_reflog (struct ref_store *refs,const char **argv) {const char *refname =notnull (*argv++,"refname" ); -int force_create =arg_flags (*argv++,"force-create" );struct strbuf err =STRBUF_INIT ;int ret; +int force_create =1 ; ret =refs_create_reflog (refs, refname, force_create, &err);if (err.len)
t/t1405-main-ref-store.sh +2 -3
@@ -35,8 +35,7 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' ' git rev-parse FOO -- && git rev-parse refs/tags/new-tag -- && m=$(git rev-parse main) && - REF_NO_DEREF=1 && - $RUN delete-refs $REF_NO_DEREF nothing FOO refs/tags/new-tag && + $RUN delete-refs REF_NO_DEREF nothing FOO refs/tags/new-tag && test_must_fail git rev-parse --symbolic-full-name FOO && test_must_fail git rev-parse FOO -- && test_must_fail git rev-parse refs/tags/new-tag --
@@ -108,7 +107,7 @@ test_expect_success 'delete_reflog(HEAD)' ' ' test_expect_success 'create-reflog(HEAD)' ' - $RUN create-reflog HEAD 1 && + $RUN create-reflog HEAD && git reflog exists HEAD '
t/t1406-submodule-ref-store.sh +1 -1
@@ -92,7 +92,7 @@ test_expect_success 'delete_reflog() not allowed' ' ' test_expect_success 'create-reflog() not allowed' ' - test_must_fail $RUN create-reflog HEAD 1 + test_must_fail $RUN create-reflog HEAD ' test_done
Signature · 1 line
gitgitgadget
PATCH v4 2/8 test-ref-store: parse symbolic flag constants
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This lets tests use REF_XXXX constants instead of hardcoded integers. The flag names should be separated by a ','.
--- t/helper/test-ref-store.c | 66 ++++++++++++++++++++++++++++++++++----- t/t1405-main-ref-store.sh | 3 +- 2 files changed, 60 insertions(+), 9 deletions(-)
t/helper/test-ref-store.c +59 -7
@@ -5,6 +5,48 @@#include "object-store.h" #include "repository.h" +struct flag_definition { +const char *name; +uint64_t mask; +}; + +#define FLAG_DEF (x) \ + { \ +#x, (x) \ + } + +static unsigned int parse_flags (const char *str,struct flag_definition *defs) +{ +struct string_list masks =STRING_LIST_INIT_DUP ; +int i =0 ; +unsigned int result =0 ; + +if (!strcmp (str,"0" )) +return 0 ; + +string_list_split (&masks, str,',' ,64 ); +for (; i < masks.nr; i++) { +const char *name = masks.items[i].string; +struct flag_definition *def = defs; +int found =0 ; +while (def->name) { +if (!strcmp (def->name, name)) { + result |= def->mask; + found =1 ; +break ; + } + def++; + } +if (!found) +die ("unknown flag \"%s\"" , name); + } + +string_list_clear (&masks,0 ); +return result; +} + +static struct flag_definition empty_flags[] = { {NULL ,0 } }; +static const char *notnull (const char *arg,const char *name) {if (!arg)
@@ -12,9 +54,10 @@ static const char *notnull(const char *arg, const char *name)return arg; } -static unsigned int arg_flags (const char *arg,const char *name) +static unsigned int arg_flags (const char *arg,const char *name, +struct flag_definition *defs) { -return atoi (notnull (arg, name)); +return parse_flags (notnull (arg, name), defs); }static const char **get_store (const char **argv,struct ref_store **refs)
@@ -64,10 +107,13 @@ static const char **get_store(const char **argv, struct ref_store **refs)return argv +1 ; } +static struct flag_definition pack_flags[] = {FLAG_DEF (PACK_REFS_PRUNE ), +FLAG_DEF (PACK_REFS_ALL ), + {NULL ,0 } };static int cmd_pack_refs (struct ref_store *refs,const char **argv) { -unsigned int flags =arg_flags (*argv++,"flags" ); +unsigned int flags =arg_flags (*argv++,"flags" , pack_flags);return refs_pack_refs (refs, flags); }
@@ -81,9 +127,15 @@ static int cmd_create_symref(struct ref_store *refs, const char **argv)return refs_create_symref (refs, refname, target, logmsg); } +static struct flag_definition transaction_flags[] = { +FLAG_DEF (REF_NO_DEREF ), +FLAG_DEF (REF_FORCE_CREATE_REFLOG ), + {NULL ,0 } +}; +static int cmd_delete_refs (struct ref_store *refs,const char **argv) { -unsigned int flags =arg_flags (*argv++,"flags" ); +unsigned int flags =arg_flags (*argv++,"flags" , transaction_flags);const char *msg = *argv++;struct string_list refnames =STRING_LIST_INIT_NODUP ;
@@ -120,7 +172,7 @@ static int cmd_resolve_ref(struct ref_store *refs, const char **argv) {struct object_id oid = *null_oid ();const char *refname =notnull (*argv++,"refname" ); -int resolve_flags =arg_flags (*argv++,"resolve-flags" ); +int resolve_flags =arg_flags (*argv++,"resolve-flags" , empty_flags);int flags;const char *ref;int ignore_errno;
@@ -209,7 +261,7 @@ static int cmd_delete_ref(struct ref_store *refs, const char **argv)const char *msg =notnull (*argv++,"msg" );const char *refname =notnull (*argv++,"refname" );const char *sha1_buf =notnull (*argv++,"old-sha1" ); -unsigned int flags =arg_flags (*argv++,"flags" ); +unsigned int flags =arg_flags (*argv++,"flags" , transaction_flags);struct object_id old_oid;if (get_oid_hex (sha1_buf, &old_oid))
@@ -224,7 +276,7 @@ static int cmd_update_ref(struct ref_store *refs, const char **argv)const char *refname =notnull (*argv++,"refname" );const char *new_sha1_buf =notnull (*argv++,"new-sha1" );const char *old_sha1_buf =notnull (*argv++,"old-sha1" ); -unsigned int flags =arg_flags (*argv++,"flags" ); +unsigned int flags =arg_flags (*argv++,"flags" , transaction_flags);struct object_id old_oid;struct object_id new_oid;
t/t1405-main-ref-store.sh +1 -2
@@ -17,8 +17,7 @@ test_expect_success 'setup' ' test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' ' N=`find .git/refs -type f | wc -l` && test "$N" != 0 && - ALL_OR_PRUNE_FLAG=3 && - $RUN pack-refs ${ALL_OR_PRUNE_FLAG} && + $RUN pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL && N=`find .git/refs -type f` && test -z "$N" '
Signature · 1 line
gitgitgadget
PATCH v4 3/8 test-ref-store: plug memory leak in cmd_delete_refs
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- t/helper/test-ref-store.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
t/helper/test-ref-store.c +4 -1
@@ -138,11 +138,14 @@ static int cmd_delete_refs(struct ref_store *refs, const char **argv)unsigned int flags =arg_flags (*argv++,"flags" , transaction_flags);const char *msg = *argv++;struct string_list refnames =STRING_LIST_INIT_NODUP ; +int result;while (*argv)string_list_append (&refnames, *argv++); -return refs_delete_refs (refs, msg, &refnames, flags); + result =refs_delete_refs (refs, msg, &refnames, flags); +string_list_clear (&refnames,0 ); +return result; }static int cmd_rename_ref (struct ref_store *refs,const char **argv)
Signature · 1 line
gitgitgadget
PATCH v4 4/8 refs: update comment.
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
REF_IS_PRUNING is right below this comment, so it clearly does not belong in this comment. This was apparently introduced in commit 5ac95fee (Nov 5, 2017 "refs: tidy up and adjust visibility of the ref_update flags").
--- refs/files-backend.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
refs/files-backend.c +1 -2
@@ -16,8 +16,7 @@* This backend uses the following flags in `ref_update::flags` for * internal bookkeeping purposes. Their numerical values must not * conflict with REF_NO_DEREF, REF_FORCE_CREATE_REFLOG, REF_HAVE_NEW, -* REF_HAVE_OLD, or REF_IS_PRUNING, which are also stored in -* `ref_update::flags`. +* or REF_HAVE_OLD, which are also stored in `ref_update::flags`. */ /*
Signature · 1 line
gitgitgadget
PATCH v4 5/8 refs: introduce REF_SKIP_OID_VERIFICATION flag
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This lets the ref-store test helper write non-existent or unparsable objects into the ref storage.
Use this to make t1006 and t3800 independent of the files storage backend.
--- refs.h | 8 ++++++- refs/files-backend.c | 50 +++++++++++++++++++++++---------------- t/helper/test-ref-store.c | 1 + t/t1006-cat-file.sh | 5 ++-- t/t3800-mktag.sh | 6 +++-- 5 files changed, 43 insertions(+), 27 deletions(-)
refs.h +7 -1
@@ -615,12 +615,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);*/ #define REF_FORCE_CREATE_REFLOG (1 << 1) +/* +* Blindly write an object_id. This is useful for testing data corruption +* scenarios. +*/ +#define REF_SKIP_OID_VERIFICATION (1 << 10) +/* * Bitmask of all of the flags that are allowed to be passed in to * ref_transaction_update() and friends: */ #define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG) + (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION)/* * Add a reference update to transaction. `new_oid` is the value that
refs/files-backend.c +29 -21
91 lines not on this page · raw message
t/helper/test-ref-store.c +1 -0
@@ -130,6 +130,7 @@ static int cmd_create_symref(struct ref_store *refs, const char **argv)static struct flag_definition transaction_flags[] = {FLAG_DEF (REF_NO_DEREF ),FLAG_DEF (REF_FORCE_CREATE_REFLOG ), +FLAG_DEF (REF_SKIP_OID_VERIFICATION ), {NULL ,0 } };
t/t1006-cat-file.sh +2 -3
@@ -452,9 +452,8 @@ test_expect_success 'the --allow-unknown-type option does not consider replaceme # Create it manually, as "git replace" will die on bogus # types. head=$(git rev-parse --verify HEAD) && - test_when_finished "rm -rf .git/refs/replace" && - mkdir -p .git/refs/replace && - echo $head >.git/refs/replace/$bogus_short_sha1 && + test_when_finished "test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_sha1" && + test-tool ref-store main update-ref msg "refs/replace/$bogus_short_sha1" $head $ZERO_OID REF_SKIP_OID_VERIFICATION && cat >expect <<-EOF && commit
t/t3800-mktag.sh +4 -2
@@ -72,7 +72,8 @@ check_verify_failure () { # Manually create the broken, we cannot do it with # update-ref - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && # Unlike fsck-ing unreachable content above, this # will always fail.
@@ -83,7 +84,8 @@ check_verify_failure () { # Make sure the earlier test created it for us git rev-parse "$bad_tag" && - echo "$bad_tag" >"bad-tag/$tag_ref" && + test-tool -C bad-tag ref-store main delete-refs 0 msg "$tag_ref" && + test-tool -C bad-tag ref-store main update-ref msg "$tag_ref" $bad_tag $ZERO_OID REF_SKIP_OID_VERIFICATION && printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected && git -C bad-tag for-each-ref "$tag_ref" >actual &&
Signature · 1 line
gitgitgadget
PATCH v4 6/8 refs: introduce REF_SKIP_REFNAME_VERIFICATION flag
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
Use this flag with the test-helper in t1430, to avoid direct writes to the ref database.
--- refs.c | 7 +++-- refs.h | 10 +++++-- t/helper/test-ref-store.c | 1 + t/t1430-bad-ref-name.sh | 59 +++++++++++++++++++++------------------ 4 files changed, 45 insertions(+), 32 deletions(-)
refs.c +4 -3
@@ -1083,9 +1083,10 @@ int ref_transaction_update(struct ref_transaction *transaction, {assert (err); -if ((new_oid && !is_null_oid (new_oid)) ? -check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : - !refname_is_safe (refname)) { +if (!(flags &REF_SKIP_REFNAME_VERIFICATION ) && + ((new_oid && !is_null_oid (new_oid)) ? +check_refname_format (refname,REFNAME_ALLOW_ONELEVEL ) : + !refname_is_safe (refname))) {strbuf_addf (err,_ ("refusing to update ref with bad name '%s'" ), refname);return -1 ;
refs.h +8 -2
@@ -621,12 +621,18 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);*/ #define REF_SKIP_OID_VERIFICATION (1 << 10) +/* +* Skip verifying refname. This is useful for testing data corruption scenarios. +*/ +#define REF_SKIP_REFNAME_VERIFICATION (1 << 11) +/* * Bitmask of all of the flags that are allowed to be passed in to * ref_transaction_update() and friends: */ -#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ - (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION) +#define REF_TRANSACTION_UPDATE_ALLOWED_FLAGS \ + (REF_NO_DEREF | REF_FORCE_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION | \ + REF_SKIP_REFNAME_VERIFICATION)/* * Add a reference update to transaction. `new_oid` is the value that
t/helper/test-ref-store.c +1 -0
@@ -131,6 +131,7 @@ static struct flag_definition transaction_flags[] = {FLAG_DEF (REF_NO_DEREF ),FLAG_DEF (REF_FORCE_CREATE_REFLOG ),FLAG_DEF (REF_SKIP_OID_VERIFICATION ), +FLAG_DEF (REF_SKIP_REFNAME_VERIFICATION ), { NULL, 0 } };
t/t1430-bad-ref-name.sh +32 -27
156 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v4 7/8 t1430: remove refs using test-tool
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- t/t1430-bad-ref-name.sh | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
t/t1430-bad-ref-name.sh +12 -12
@@ -154,7 +154,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' git branch shadow one && test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow && - test_when_finished "rm -f .git/refs/tags/shadow" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err && test_cmp expect actual &&
@@ -165,9 +165,9 @@ test_expect_success 'for-each-ref emits warnings for broken names' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git for-each-ref >output 2>error && ! grep -e "broken\.\.\.ref" output && ! grep -e "badname" output &&
@@ -204,7 +204,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output &&
@@ -216,7 +216,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
@@ -225,7 +225,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_must_be_empty output &&
@@ -234,7 +234,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok test_expect_success 'branch -d can delete dangling symref to broken name' ' printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname && test_i18ngrep "Deleted branch badname (was refs/heads/broken\.\.\.ref)" output &&
@@ -246,7 +246,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && - test_when_finished "rm -f .git/refs/heads/badname" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref && test_must_be_empty output &&
@@ -255,7 +255,7 @@ test_expect_success 'update-ref -d can delete broken name through symref' ' test_expect_success 'update-ref --no-deref -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output &&
@@ -264,7 +264,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref with broken name test_expect_success 'branch -d can delete symref with broken name' ' printf "ref: refs/heads/main\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/main)" output &&
@@ -273,7 +273,7 @@ test_expect_success 'branch -d can delete symref with broken name' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git update-ref --no-deref -d refs/heads/broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_must_be_empty output &&
@@ -282,7 +282,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref with br test_expect_success 'branch -d can delete dangling symref with broken name' ' printf "ref: refs/heads/idonotexist\n" >.git/refs/heads/broken...symref && - test_when_finished "rm -f .git/refs/heads/broken...symref" && + test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...symref" && git branch -d broken...symref >output 2>error && test_path_is_missing .git/refs/heads/broken...symref && test_i18ngrep "Deleted branch broken...symref (was refs/heads/idonotexist)" output &&
Signature · 1 line
gitgitgadget
PATCH v4 8/8 t1430: create valid symrefs using test-helper
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This still leaves some other direct filesystem access. Currently, the files backend does not allow invalidly named symrefs. Fixes for this are currently in the 'seen' branch
--- t/t1430-bad-ref-name.sh | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
t/t1430-bad-ref-name.sh +6 -8
@@ -153,7 +153,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' ' test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && git branch shadow one && test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && - printf "ref: refs/heads/broken...ref\n" >.git/refs/tags/shadow && + test-tool ref-store main create-symref refs/tags/shadow refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/tags/shadow" && git rev-parse --verify one >expect && git rev-parse --verify shadow >actual 2>err &&
@@ -203,7 +203,7 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -213,9 +213,8 @@ test_expect_success 'update-ref --no-deref -d can delete symref to broken name' test_expect_success 'branch -d can delete symref to broken name' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && - test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -224,7 +223,7 @@ test_expect_success 'branch -d can delete symref to broken name' ' ' test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref --no-deref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -233,7 +232,7 @@ test_expect_success 'update-ref --no-deref -d can delete dangling symref to brok ' test_expect_success 'branch -d can delete dangling symref to broken name' ' - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git branch -d badname >output 2>error && test_path_is_missing .git/refs/heads/badname &&
@@ -243,9 +242,8 @@ test_expect_success 'branch -d can delete dangling symref to broken name' ' test_expect_success 'update-ref -d can delete broken name through symref' ' test-tool ref-store main update-ref msg "refs/heads/broken...ref" $main_sha1 $ZERO_OID REF_SKIP_REFNAME_VERIFICATION && - test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/broken...ref" && - printf "ref: refs/heads/broken...ref\n" >.git/refs/heads/badname && + test-tool ref-store main create-symref refs/heads/badname refs/heads/broken...ref msg && test_when_finished "test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/badname" && git update-ref -d refs/heads/badname >output 2>error && test_path_is_missing .git/refs/heads/broken...ref &&
Signature · 1 line
gitgitgadget
Quoted 6 lines · Han-Wen Nienhuys via GitGitGadget
this covers a few cases of direct filesystem access to the ref database in tests.
v4:
* fix stack OOB read due to missing array terminator.
Looks good to me. I'll tentatively mark it for 'next' and will merge it by the end of this week unless problems are found in the series.
Thanks for a pleasant read.