* [PATCH v2 01/21] t4202: split testcase for invalid HEAD symref and HEAD hash
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:03 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 02/21] t/helper/ref-store: initialize oid in resolve-ref Han-Wen Nienhuys via GitGitGadget
` (21 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Reftable will prohibit invalid hashes at the storage level, but
git-symbolic-ref can still create branches ending in ".lock".
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t4202-log.sh | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 350cfa35936a..a8c5a00d012d 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1834,14 +1834,18 @@ test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
'
-test_expect_success 'log diagnoses bogus HEAD' '
+test_expect_success 'log diagnoses bogus HEAD hash' '
git init empty &&
test_must_fail git -C empty log 2>stderr &&
test_i18ngrep does.not.have.any.commits stderr &&
echo 1234abcd >empty/.git/refs/heads/main &&
test_must_fail git -C empty log 2>stderr &&
- test_i18ngrep broken stderr &&
- echo "ref: refs/heads/invalid.lock" >empty/.git/HEAD &&
+ test_i18ngrep broken stderr'
+
+test_expect_success 'log diagnoses bogus HEAD symref' '
+ rm -rf empty &&
+ git init empty &&
+ git --git-dir empty/.git symbolic-ref HEAD refs/heads/invalid.lock &&
test_must_fail git -C empty log 2>stderr &&
test_i18ngrep broken stderr &&
test_must_fail git -C empty log --default totally-bogus 2>stderr &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 01/21] t4202: split testcase for invalid HEAD symref and HEAD hash
2021-04-27 10:38 ` [PATCH v2 01/21] t4202: split testcase for invalid HEAD symref and HEAD hash Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:03 ` Ævar Arnfjörð Bjarmason
2021-05-31 13:54 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:03 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Reftable will prohibit invalid hashes at the storage level, but
> git-symbolic-ref can still create branches ending in ".lock".
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t4202-log.sh | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 350cfa35936a..a8c5a00d012d 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -1834,14 +1834,18 @@ test_expect_success 'log --graph --no-walk is forbidden' '
> test_must_fail git log --graph --no-walk
> '
>
> -test_expect_success 'log diagnoses bogus HEAD' '
> +test_expect_success 'log diagnoses bogus HEAD hash' '
> git init empty &&
> test_must_fail git -C empty log 2>stderr &&
> test_i18ngrep does.not.have.any.commits stderr &&
> echo 1234abcd >empty/.git/refs/heads/main &&
> test_must_fail git -C empty log 2>stderr &&
> - test_i18ngrep broken stderr &&
> - echo "ref: refs/heads/invalid.lock" >empty/.git/HEAD &&
> + test_i18ngrep broken stderr'
> +
> +test_expect_success 'log diagnoses bogus HEAD symref' '
> + rm -rf empty &&
> + git init empty &&
> + git --git-dir empty/.git symbolic-ref HEAD refs/heads/invalid.lock &&
> test_must_fail git -C empty log 2>stderr &&
> test_i18ngrep broken stderr &&
> test_must_fail git -C empty log --default totally-bogus 2>stderr &&
When splitting up tests like:
git init xyz &&
[do with xyz]
Let's do:
# test 1
test_when_finished "rm -rf xyz" &&
git init xyz &&
[do with xyz]
# test 2
test_when_finished "rm -rf xyz" &&
git init xyz &&
[do with xyz]
Instead of, as here:
# test 1
git init xyz &&
[do with xyz]
# test 2
rm -rf xyz &&
git init xyz &&
[do with xyz]
This isn't a logic error, but just a better idiom. Better to have the
thing creating stuff clean it up, than other tests being aware of their
order and (possibly redundantly, if we skip tests), clean up after
earlier tests.
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 01/21] t4202: split testcase for invalid HEAD symref and HEAD hash
2021-05-20 15:03 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 13:54 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 13:54 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:05 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> When splitting up tests like:
>
> git init xyz &&
> [do with xyz]
>
> Let's do:
>
> # test 1
> test_when_finished "rm -rf xyz" &&
Done.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 02/21] t/helper/ref-store: initialize oid in resolve-ref
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 01/21] t4202: split testcase for invalid HEAD symref and HEAD hash Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:06 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 03/21] t9300: check ref existence using test-helper rather than a file system check Han-Wen Nienhuys via GitGitGadget
` (20 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
This will print $ZERO_OID when asking for a non-existent ref from the
test-helper.
Since resolve-ref provides direct access to refs_resolve_ref_unsafe(), it
provides a reliable mechanism for accessing REFNAME, while avoiding the implicit
resolution to refs/heads/REFNAME.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/helper/test-ref-store.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index bba5f841c6ab..01d8f3285dc8 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -118,7 +118,7 @@ static int cmd_for_each_ref(struct ref_store *refs, const char **argv)
static int cmd_resolve_ref(struct ref_store *refs, const char **argv)
{
- struct object_id oid;
+ struct object_id oid = { 0 };
const char *refname = notnull(*argv++, "refname");
int resolve_flags = arg_flags(*argv++, "resolve-flags");
int flags;
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 02/21] t/helper/ref-store: initialize oid in resolve-ref
2021-04-27 10:38 ` [PATCH v2 02/21] t/helper/ref-store: initialize oid in resolve-ref Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:06 ` Ævar Arnfjörð Bjarmason
2021-05-31 14:48 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:06 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> This will print $ZERO_OID when asking for a non-existent ref from the
> test-helper.
>
> Since resolve-ref provides direct access to refs_resolve_ref_unsafe(), it
> provides a reliable mechanism for accessing REFNAME, while avoiding the implicit
> resolution to refs/heads/REFNAME.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/helper/test-ref-store.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
> index bba5f841c6ab..01d8f3285dc8 100644
> --- a/t/helper/test-ref-store.c
> +++ b/t/helper/test-ref-store.c
> @@ -118,7 +118,7 @@ static int cmd_for_each_ref(struct ref_store *refs, const char **argv)
>
> static int cmd_resolve_ref(struct ref_store *refs, const char **argv)
> {
> - struct object_id oid;
> + struct object_id oid = { 0 };
> const char *refname = notnull(*argv++, "refname");
> int resolve_flags = arg_flags(*argv++, "resolve-flags");
> int flags;
This feels a bit magical, later we have this:
printf("%s %s 0x%x\n", oid_to_hex(&oid), ref ? ref : "(null)", flags);
Isn't ref always going to be NULL in that case too? Wouldn't it make
more sense to not zero this out and instead do:
if (ref)
/* current code, mostly */
else
use zero_oid()
That seems more straightforward to me than this implicit proxy for
zero_oid(). Also, isn't the point of zero_oid() to not make this
particular assumption, i.e. the recent discussion (haven't followed
where it went) of the "oid" having some sort of "hash type" member,
which surely would do the wrong thing here under either SHA-1 or
SHA-256, or maybe I mis(understood|remember) that discussion...
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 02/21] t/helper/ref-store: initialize oid in resolve-ref
2021-05-20 15:06 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 14:48 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 14:48 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:09 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
>
> > From: Han-Wen Nienhuys <hanwen@google.com>
> >
> > This will print $ZERO_OID when asking for a non-existent ref from the
> > test-helper.
> >
> > Since resolve-ref provides direct access to refs_resolve_ref_unsafe(), it
> > provides a reliable mechanism for accessing REFNAME, while avoiding the implicit
> > resolution to refs/heads/REFNAME.
> >
> > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> > ---
> > t/helper/test-ref-store.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
> > index bba5f841c6ab..01d8f3285dc8 100644
> > --- a/t/helper/test-ref-store.c
> > +++ b/t/helper/test-ref-store.c
> > @@ -118,7 +118,7 @@ static int cmd_for_each_ref(struct ref_store *refs, const char **argv)
> >
> > static int cmd_resolve_ref(struct ref_store *refs, const char **argv)
> > {
> > - struct object_id oid;
> > + struct object_id oid = { 0 };
> > const char *refname = notnull(*argv++, "refname");
> > int resolve_flags = arg_flags(*argv++, "resolve-flags");
> > int flags;
>
> This feels a bit magical, later we have this:
>
> printf("%s %s 0x%x\n", oid_to_hex(&oid), ref ? ref : "(null)", flags);
>
> Isn't ref always going to be NULL in that case too? Wouldn't it make
> more sense to not zero this out and instead do:
>
> if (ref)
> /* current code, mostly */
> else
> use zero_oid()
for programmatic access, the if will actually make it harder to parse
out what is happening, so I think this solution is better. Note that
the function already handles ref == null, so changing the printf
format in this way can only serve to break other tests.
> That seems more straightforward to me than this implicit proxy for
> zero_oid(). Also, isn't the point of zero_oid() to not make this
used null_oid() instead (zero_oid() doesn't exist in C).
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 03/21] t9300: check ref existence using test-helper rather than a file system check
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 01/21] t4202: split testcase for invalid HEAD symref and HEAD hash Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 02/21] t/helper/ref-store: initialize oid in resolve-ref Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:11 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 04/21] t5601: read HEAD using rev-parse Han-Wen Nienhuys via GitGitGadget
` (19 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t9300-fast-import.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 5c47ac4465cb..1aea943bef72 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -392,7 +392,7 @@ test_expect_success 'B: accept branch name "TEMP_TAG"' '
git gc
git prune" &&
git fast-import <input &&
- test -f .git/TEMP_TAG &&
+ test $(test-tool ref-store main resolve-ref TEMP_TAG 0 | cut -f1 -d " " ) != "$ZERO_OID" &&
test $(git rev-parse main) = $(git rev-parse TEMP_TAG^)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 03/21] t9300: check ref existence using test-helper rather than a file system check
2021-04-27 10:38 ` [PATCH v2 03/21] t9300: check ref existence using test-helper rather than a file system check Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:11 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:11 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t9300-fast-import.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index 5c47ac4465cb..1aea943bef72 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -392,7 +392,7 @@ test_expect_success 'B: accept branch name "TEMP_TAG"' '
> git gc
> git prune" &&
> git fast-import <input &&
> - test -f .git/TEMP_TAG &&
> + test $(test-tool ref-store main resolve-ref TEMP_TAG 0 | cut -f1 -d " " ) != "$ZERO_OID" &&
> test $(git rev-parse main) = $(git rev-parse TEMP_TAG^)
> '
We have resolve-ref and verify-ref in the helper, why not a few-line
"exists-ref" or similar instead of needing to parse this out with
shell/cut/test?
I haven't tested, but in resolve-ref we do this:
printf("%s %s 0x%x\n", oid_to_hex(&oid), ref ? ref : "(null)", flags);
return ref ? 0 : 1;
So re my earlier question about "ref" being NULL or not, isn't this
going to be reflected by the exit code of resolve-ref already? I.e. any
"exists-ref" would just be a convenience wrapper equivalent to a
"--quiet" flag for resolve-ref, no?
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 04/21] t5601: read HEAD using rev-parse
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (2 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 03/21] t9300: check ref existence using test-helper rather than a file system check Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 05/21] t1401-symbolic-ref: avoid direct filesystem access Han-Wen Nienhuys via GitGitGadget
` (18 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t5601-clone.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 329ae599fd3c..7223372c7660 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -305,7 +305,8 @@ test_expect_success 'clone from original with relative alternate' '
test_expect_success 'clone checking out a tag' '
git clone --branch=some-tag src dst.tag &&
GIT_DIR=src/.git git rev-parse some-tag >expected &&
- test_cmp expected dst.tag/.git/HEAD &&
+ GIT_DIR=dst.tag/.git git rev-parse HEAD >actual &&
+ test_cmp expected actual &&
GIT_DIR=dst.tag/.git git config remote.origin.fetch >fetch.actual &&
echo "+refs/heads/*:refs/remotes/origin/*" >fetch.expected &&
test_cmp fetch.expected fetch.actual
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v2 05/21] t1401-symbolic-ref: avoid direct filesystem access
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (3 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 04/21] t5601: read HEAD using rev-parse Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:20 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 06/21] t1413: use tar to save and restore entire .git directory Han-Wen Nienhuys via GitGitGadget
` (17 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t1401-symbolic-ref.sh | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index a4ebb0b65fec..315a62f78019 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -3,21 +3,20 @@
test_description='basic symbolic-ref tests'
. ./test-lib.sh
-# If the tests munging HEAD fail, they can break detection of
-# the git repo, meaning that further tests will operate on
-# the surrounding git repo instead of the trash directory.
-reset_to_sane() {
- echo ref: refs/heads/foo >.git/HEAD
-}
-
-test_expect_success 'symbolic-ref writes HEAD' '
+test_expect_success 'setup' '
git symbolic-ref HEAD refs/heads/foo &&
- echo ref: refs/heads/foo >expect &&
- test_cmp expect .git/HEAD
+ test_commit file &&
+ "$TAR" cf .git.tar .git/
'
-test_expect_success 'symbolic-ref reads HEAD' '
- echo refs/heads/foo >expect &&
+reset_to_sane() {
+ rm -rf .git &&
+ "$TAR" xf .git.tar
+}
+
+test_expect_success 'symbolic-ref read/write roundtrip' '
+ git symbolic-ref HEAD refs/heads/read-write-roundtrip &&
+ echo refs/heads/read-write-roundtrip >expect &&
git symbolic-ref HEAD >actual &&
test_cmp expect actual
'
@@ -25,12 +24,13 @@ test_expect_success 'symbolic-ref reads HEAD' '
test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
test_must_fail git symbolic-ref HEAD foo
'
+
reset_to_sane
test_expect_success 'symbolic-ref refuses bare sha1' '
- echo content >file && git add file && git commit -m one &&
test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD)
'
+
reset_to_sane
test_expect_success 'HEAD cannot be removed' '
@@ -42,16 +42,16 @@ reset_to_sane
test_expect_success 'symbolic-ref can be deleted' '
git symbolic-ref NOTHEAD refs/heads/foo &&
git symbolic-ref -d NOTHEAD &&
- test_path_is_file .git/refs/heads/foo &&
- test_path_is_missing .git/NOTHEAD
+ git rev-parse refs/heads/foo &&
+ test_must_fail git symbolic-ref NOTHEAD
'
reset_to_sane
test_expect_success 'symbolic-ref can delete dangling symref' '
git symbolic-ref NOTHEAD refs/heads/missing &&
git symbolic-ref -d NOTHEAD &&
- test_path_is_missing .git/refs/heads/missing &&
- test_path_is_missing .git/NOTHEAD
+ test_must_fail git rev-parse refs/heads/missing &&
+ test_must_fail git symbolic-ref NOTHEAD
'
reset_to_sane
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 05/21] t1401-symbolic-ref: avoid direct filesystem access
2021-04-27 10:38 ` [PATCH v2 05/21] t1401-symbolic-ref: avoid direct filesystem access Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:20 ` Ævar Arnfjörð Bjarmason
2021-05-31 13:40 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:20 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t1401-symbolic-ref.sh | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
> index a4ebb0b65fec..315a62f78019 100755
> --- a/t/t1401-symbolic-ref.sh
> +++ b/t/t1401-symbolic-ref.sh
> @@ -3,21 +3,20 @@
> test_description='basic symbolic-ref tests'
> . ./test-lib.sh
>
> -# If the tests munging HEAD fail, they can break detection of
> -# the git repo, meaning that further tests will operate on
> -# the surrounding git repo instead of the trash directory.
> -reset_to_sane() {
> - echo ref: refs/heads/foo >.git/HEAD
> -}
> -
> -test_expect_success 'symbolic-ref writes HEAD' '
> +test_expect_success 'setup' '
> git symbolic-ref HEAD refs/heads/foo &&
> - echo ref: refs/heads/foo >expect &&
> - test_cmp expect .git/HEAD
> + test_commit file &&
> + "$TAR" cf .git.tar .git/
> '
>
> -test_expect_success 'symbolic-ref reads HEAD' '
> - echo refs/heads/foo >expect &&
> +reset_to_sane() {
> + rm -rf .git &&
> + "$TAR" xf .git.tar
> +}
> +
> +test_expect_success 'symbolic-ref read/write roundtrip' '
> + git symbolic-ref HEAD refs/heads/read-write-roundtrip &&
> + echo refs/heads/read-write-roundtrip >expect &&
> git symbolic-ref HEAD >actual &&
> test_cmp expect actual
> '
> @@ -25,12 +24,13 @@ test_expect_success 'symbolic-ref reads HEAD' '
> test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
> test_must_fail git symbolic-ref HEAD foo
> '
> +
> reset_to_sane
>
> test_expect_success 'symbolic-ref refuses bare sha1' '
> - echo content >file && git add file && git commit -m one &&
> test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD)
> '
> +
> reset_to_sane
>
> test_expect_success 'HEAD cannot be removed' '
> @@ -42,16 +42,16 @@ reset_to_sane
> test_expect_success 'symbolic-ref can be deleted' '
> git symbolic-ref NOTHEAD refs/heads/foo &&
> git symbolic-ref -d NOTHEAD &&
> - test_path_is_file .git/refs/heads/foo &&
> - test_path_is_missing .git/NOTHEAD
> + git rev-parse refs/heads/foo &&
> + test_must_fail git symbolic-ref NOTHEAD
> '
> reset_to_sane
>
> test_expect_success 'symbolic-ref can delete dangling symref' '
> git symbolic-ref NOTHEAD refs/heads/missing &&
> git symbolic-ref -d NOTHEAD &&
> - test_path_is_missing .git/refs/heads/missing &&
> - test_path_is_missing .git/NOTHEAD
> + test_must_fail git rev-parse refs/heads/missing &&
> + test_must_fail git symbolic-ref NOTHEAD
> '
> reset_to_sane
You do end up needing to refactor some rather nasty patterns in this
series, so this isn't on you initially...
But since we're encountering this "reset_to_sane" pattern, can't we just
as easily fix it up with something more obvious than replacing an echo
to a specific ref with a tarring up and untarring of the whole .git each
time?
I.e. something like:
# setup the .git initially
Then:
test_when_finished "rm -rf copy" &&
git clone . copy &&
# munge the repo and use "git -C copy" for the tests"
?
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 05/21] t1401-symbolic-ref: avoid direct filesystem access
2021-05-20 15:20 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 13:40 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 13:40 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:21 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> But since we're encountering this "reset_to_sane" pattern, can't we just
> as easily fix it up with something more obvious than replacing an echo
> to a specific ref with a tarring up and untarring of the whole .git each
> time?
>
> I.e. something like:
>
> # setup the .git initially
>
> Then:
>
> test_when_finished "rm -rf copy" &&
> git clone . copy &&
> # munge the repo and use "git -C copy" for the tests"
I think this is actually worse than tarring up the tree. By using
clone, your initial state will be affected by any behavior that
git-clone does that makes sense when the user is a human. For example,
refs/heads/x is renamed to refs/remotes/origin/x. Also, all the
timestamps will be off, which might be a problem if there is
infrastructure (index refreshes?) that uses timestamps. By contrast,
using tar (re)creates a precise snapshot of the initial state.
--
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
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 06/21] t1413: use tar to save and restore entire .git directory
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (4 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 05/21] t1401-symbolic-ref: avoid direct filesystem access Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:22 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 07/21] t1301: fix typo in error message Han-Wen Nienhuys via GitGitGadget
` (16 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
This makes the test independent of the particulars of the storage formats.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t1413-reflog-detach.sh | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh
index bde05208ae6a..934688a1ee82 100755
--- a/t/t1413-reflog-detach.sh
+++ b/t/t1413-reflog-detach.sh
@@ -7,8 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
reset_state () {
- git checkout main &&
- cp saved_reflog .git/logs/HEAD
+ rm -rf .git && "$TAR" xf .git-saved.tar
}
test_expect_success setup '
@@ -17,7 +16,7 @@ test_expect_success setup '
git branch side &&
test_tick &&
git commit --allow-empty -m second &&
- cat .git/logs/HEAD >saved_reflog
+ "$TAR" cf .git-saved.tar .git
'
test_expect_success baseline '
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 06/21] t1413: use tar to save and restore entire .git directory
2021-04-27 10:38 ` [PATCH v2 06/21] t1413: use tar to save and restore entire .git directory Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:22 ` Ævar Arnfjörð Bjarmason
2021-05-31 15:16 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:22 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> This makes the test independent of the particulars of the storage formats.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t1413-reflog-detach.sh | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh
> index bde05208ae6a..934688a1ee82 100755
> --- a/t/t1413-reflog-detach.sh
> +++ b/t/t1413-reflog-detach.sh
> @@ -7,8 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> . ./test-lib.sh
>
> reset_state () {
> - git checkout main &&
> - cp saved_reflog .git/logs/HEAD
> + rm -rf .git && "$TAR" xf .git-saved.tar
> }
>
> test_expect_success setup '
> @@ -17,7 +16,7 @@ test_expect_success setup '
> git branch side &&
> test_tick &&
> git commit --allow-empty -m second &&
> - cat .git/logs/HEAD >saved_reflog
> + "$TAR" cf .git-saved.tar .git
> '
>
> test_expect_success baseline '
So what I said in 05, but also didn't the commit messages of 05 and 06
get mixed up / bad fixup in a WIP version? I.e. the first commit says
"use symbolic ref", but it's mostly about introducing the use of this
tar/untar pattern.
Whereas this continues the tarring pattern, and doesn't start using it,
and (presumably) is the mis-squashed commit that should have added this
whole tar thing after the first commit does the isolated
s/echo/symbolic-ref/ fix.
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 06/21] t1413: use tar to save and restore entire .git directory
2021-05-20 15:22 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 15:16 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 15:16 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:23 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> > test_expect_success baseline '
>
> So what I said in 05, but also didn't the commit messages of 05 and 06
> get mixed up / bad fixup in a WIP version? I.e. the first commit says
> "use symbolic ref", but it's mostly about introducing the use of this
> tar/untar pattern.
>
> Whereas this continues the tarring pattern, and doesn't start using it,
> and (presumably) is the mis-squashed commit that should have added this
> whole tar thing after the first commit does the isolated
> s/echo/symbolic-ref/ fix.
I've split the commits by test file. I added a separate commit for the
tar use here.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 07/21] t1301: fix typo in error message
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (5 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 06/21] t1413: use tar to save and restore entire .git directory Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 08/21] t5000: reformat indentation to the latest fashion Han-Wen Nienhuys via GitGitGadget
` (15 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t1301-shared-repo.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index ac947bff9fcf..84bf1970d8bf 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -124,7 +124,7 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
: happy
;;
*)
- echo Ooops, .git/logs/refs/heads/main is not 0662 [$actual]
+ echo Ooops, .git/logs/refs/heads/main is not 066x [$actual]
false
;;
esac
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v2 08/21] t5000: reformat indentation to the latest fashion
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (6 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 07/21] t1301: fix typo in error message Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:24 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 09/21] t5000: inspect HEAD using git-rev-parse Han-Wen Nienhuys via GitGitGadget
` (14 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t5000-tar-tree.sh | 104 ++++++++++++++++++++++----------------------
1 file changed, 53 insertions(+), 51 deletions(-)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 7204799a0b52..b6734cba7e65 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -111,25 +111,25 @@ test_expect_success 'setup' '
EOF
'
-test_expect_success \
- 'populate workdir' \
- 'mkdir a &&
- echo simple textfile >a/a &&
- ten=0123456789 && hundred=$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten &&
- echo long filename >a/four$hundred &&
- mkdir a/bin &&
- test-tool genrandom "frotz" 500000 >a/bin/sh &&
- printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
- printf "A not substituted O" >a/substfile2 &&
- if test_have_prereq SYMLINKS; then
- ln -s a a/l1
- else
- printf %s a > a/l1
- fi &&
- (p=long_path_to_a_file && cd a &&
- for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
- echo text >file_with_long_path) &&
- (cd a && find .) | sort >a.lst'
+test_expect_success 'populate workdir' '
+ mkdir a &&
+ echo simple textfile >a/a &&
+ ten=0123456789 && hundred=$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten &&
+ echo long filename >a/four$hundred &&
+ mkdir a/bin &&
+ test-tool genrandom "frotz" 500000 >a/bin/sh &&
+ printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
+ printf "A not substituted O" >a/substfile2 &&
+ if test_have_prereq SYMLINKS; then
+ ln -s a a/l1
+ else
+ printf %s a >a/l1
+ fi &&
+ (p=long_path_to_a_file && cd a &&
+ for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
+ echo text >file_with_long_path) &&
+ (cd a && find .) | sort >a.lst
+'
test_expect_success \
'add ignored file' \
@@ -147,18 +147,18 @@ test_expect_success 'setup export-subst' '
>a/substfile1
'
-test_expect_success \
- 'create bare clone' \
- 'git clone --bare . bare.git &&
- cp .git/info/attributes bare.git/info/attributes'
+test_expect_success 'create bare clone' '
+ git clone --bare . bare.git &&
+ cp .git/info/attributes bare.git/info/attributes
+'
-test_expect_success \
- 'remove ignored file' \
- 'rm a/ignored'
+test_expect_success 'remove ignored file' '
+ rm a/ignored
+'
-test_expect_success \
- 'git archive' \
- 'git archive HEAD >b.tar'
+test_expect_success 'git archive' '
+ git archive HEAD >b.tar
+'
check_tar b
@@ -194,26 +194,28 @@ check_added with_untracked2 untracked one/untracked
check_added with_untracked2 untracked two/untracked
test_expect_success 'git archive on large files' '
- test_config core.bigfilethreshold 1 &&
- git archive HEAD >b3.tar &&
- test_cmp_bin b.tar b3.tar
+ test_config core.bigfilethreshold 1 &&
+ git archive HEAD >b3.tar &&
+ test_cmp_bin b.tar b3.tar
'
-test_expect_success \
- 'git archive in a bare repo' \
- '(cd bare.git && git archive HEAD) >b3.tar'
+test_expect_success 'git archive in a bare repo' '
+ (cd bare.git && git archive HEAD) >b3.tar
+'
-test_expect_success \
- 'git archive vs. the same in a bare repo' \
- 'test_cmp_bin b.tar b3.tar'
+test_expect_success 'git archive vs. the same in a bare repo' '
+ test_cmp_bin b.tar b3.tar
+'
-test_expect_success 'git archive with --output' \
- 'git archive --output=b4.tar HEAD &&
- test_cmp_bin b.tar b4.tar'
+test_expect_success 'git archive with --output' '
+ git archive --output=b4.tar HEAD &&
+ test_cmp_bin b.tar b4.tar
+'
-test_expect_success 'git archive --remote' \
- 'git archive --remote=. HEAD >b5.tar &&
- test_cmp_bin b.tar b5.tar'
+test_expect_success 'git archive --remote' '
+ git archive --remote=. HEAD >b5.tar &&
+ test_cmp_bin b.tar b5.tar
+'
test_expect_success 'git archive --remote with configured remote' '
git config remote.foo.url . &&
@@ -224,13 +226,13 @@ test_expect_success 'git archive --remote with configured remote' '
test_cmp_bin b.tar b5-nick.tar
'
-test_expect_success \
- 'validate file modification time' \
- 'mkdir extract &&
- "$TAR" xf b.tar -C extract a/a &&
- test-tool chmtime --get extract/a/a >b.mtime &&
- echo "1117231200" >expected.mtime &&
- test_cmp expected.mtime b.mtime'
+test_expect_success 'validate file modification time' '
+ mkdir extract &&
+ "$TAR" xf b.tar -C extract a/a &&
+ test-tool chmtime --get extract/a/a >b.mtime &&
+ echo "1117231200" >expected.mtime &&
+ test_cmp expected.mtime b.mtime
+'
test_expect_success \
'git get-tar-commit-id' \
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 08/21] t5000: reformat indentation to the latest fashion
2021-04-27 10:38 ` [PATCH v2 08/21] t5000: reformat indentation to the latest fashion Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:24 ` Ævar Arnfjörð Bjarmason
2021-05-31 13:56 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:24 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t5000-tar-tree.sh | 104 ++++++++++++++++++++++----------------------
> 1 file changed, 53 insertions(+), 51 deletions(-)
>
> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
> index 7204799a0b52..b6734cba7e65 100755
> --- a/t/t5000-tar-tree.sh
> +++ b/t/t5000-tar-tree.sh
> @@ -111,25 +111,25 @@ test_expect_success 'setup' '
> EOF
> '
>
> -test_expect_success \
> - 'populate workdir' \
> - 'mkdir a &&
> - echo simple textfile >a/a &&
> - ten=0123456789 && hundred=$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten &&
> - echo long filename >a/four$hundred &&
> - mkdir a/bin &&
> - test-tool genrandom "frotz" 500000 >a/bin/sh &&
> - printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
> - printf "A not substituted O" >a/substfile2 &&
> - if test_have_prereq SYMLINKS; then
> - ln -s a a/l1
> - else
> - printf %s a > a/l1
> - fi &&
> - (p=long_path_to_a_file && cd a &&
> - for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
> - echo text >file_with_long_path) &&
> - (cd a && find .) | sort >a.lst'
> +test_expect_success 'populate workdir' '
> + mkdir a &&
> + echo simple textfile >a/a &&
> + ten=0123456789 && hundred=$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten &&
> + echo long filename >a/four$hundred &&
> + mkdir a/bin &&
> + test-tool genrandom "frotz" 500000 >a/bin/sh &&
> + printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
> + printf "A not substituted O" >a/substfile2 &&
> + if test_have_prereq SYMLINKS; then
> + ln -s a a/l1
> + else
> + printf %s a >a/l1
> + fi &&
> + (p=long_path_to_a_file && cd a &&
> + for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
> + echo text >file_with_long_path) &&
> + (cd a && find .) | sort >a.lst
> +'
Since we're refactoring this anyway, let's:
* split up ten=... hundred=... into two lines
* put "; then" on its own line, if ...\nthen\n...
* split up that sub-shell into (\np=...&& ..., i.e. our usual style, some with the one-line for-loop
> test_expect_success \
> 'add ignored file' \
> @@ -147,18 +147,18 @@ test_expect_success 'setup export-subst' '
> >a/substfile1
> '
>
> -test_expect_success \
> - 'create bare clone' \
> - 'git clone --bare . bare.git &&
> - cp .git/info/attributes bare.git/info/attributes'
> +test_expect_success 'create bare clone' '
> + git clone --bare . bare.git &&
> + cp .git/info/attributes bare.git/info/attributes
> +'
>
> -test_expect_success \
> - 'remove ignored file' \
> - 'rm a/ignored'
> +test_expect_success 'remove ignored file' '
> + rm a/ignored
> +'
>
> -test_expect_success \
> - 'git archive' \
> - 'git archive HEAD >b.tar'
> +test_expect_success 'git archive' '
> + git archive HEAD >b.tar
> +'
>
> check_tar b
>
> @@ -194,26 +194,28 @@ check_added with_untracked2 untracked one/untracked
> check_added with_untracked2 untracked two/untracked
>
> test_expect_success 'git archive on large files' '
> - test_config core.bigfilethreshold 1 &&
> - git archive HEAD >b3.tar &&
> - test_cmp_bin b.tar b3.tar
> + test_config core.bigfilethreshold 1 &&
> + git archive HEAD >b3.tar &&
> + test_cmp_bin b.tar b3.tar
> '
>
> -test_expect_success \
> - 'git archive in a bare repo' \
> - '(cd bare.git && git archive HEAD) >b3.tar'
> +test_expect_success 'git archive in a bare repo' '
> + (cd bare.git && git archive HEAD) >b3.tar
> +'
Ditto subshell etc. on one line, can't this be git -C bare.git archive
HEAD >b3.tar?
>
> -test_expect_success \
> - 'git archive vs. the same in a bare repo' \
> - 'test_cmp_bin b.tar b3.tar'
> +test_expect_success 'git archive vs. the same in a bare repo' '
> + test_cmp_bin b.tar b3.tar
> +'
>
> -test_expect_success 'git archive with --output' \
> - 'git archive --output=b4.tar HEAD &&
> - test_cmp_bin b.tar b4.tar'
> +test_expect_success 'git archive with --output' '
> + git archive --output=b4.tar HEAD &&
> + test_cmp_bin b.tar b4.tar
> +'
>
> -test_expect_success 'git archive --remote' \
> - 'git archive --remote=. HEAD >b5.tar &&
> - test_cmp_bin b.tar b5.tar'
> +test_expect_success 'git archive --remote' '
> + git archive --remote=. HEAD >b5.tar &&
> + test_cmp_bin b.tar b5.tar
> +'
>
> test_expect_success 'git archive --remote with configured remote' '
> git config remote.foo.url . &&
> @@ -224,13 +226,13 @@ test_expect_success 'git archive --remote with configured remote' '
> test_cmp_bin b.tar b5-nick.tar
> '
>
> -test_expect_success \
> - 'validate file modification time' \
> - 'mkdir extract &&
> - "$TAR" xf b.tar -C extract a/a &&
> - test-tool chmtime --get extract/a/a >b.mtime &&
> - echo "1117231200" >expected.mtime &&
> - test_cmp expected.mtime b.mtime'
> +test_expect_success 'validate file modification time' '
> + mkdir extract &&
> + "$TAR" xf b.tar -C extract a/a &&
> + test-tool chmtime --get extract/a/a >b.mtime &&
> + echo "1117231200" >expected.mtime &&
> + test_cmp expected.mtime b.mtime
> +'
>
> test_expect_success \
> 'git get-tar-commit-id' \
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 08/21] t5000: reformat indentation to the latest fashion
2021-05-20 15:24 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 13:56 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 13:56 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:26 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Ditto subshell etc. on one line, can't this be git -C bare.git archive
> HEAD >b3.tar?
yup, both done.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 09/21] t5000: inspect HEAD using git-rev-parse
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (7 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 08/21] t5000: reformat indentation to the latest fashion Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 10/21] t7003: use rev-parse rather than FS inspection Han-Wen Nienhuys via GitGitGadget
` (13 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t5000-tar-tree.sh | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index b6734cba7e65..153f8400035a 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -234,10 +234,11 @@ test_expect_success 'validate file modification time' '
test_cmp expected.mtime b.mtime
'
-test_expect_success \
- 'git get-tar-commit-id' \
- 'git get-tar-commit-id <b.tar >b.commitid &&
- test_cmp .git/$(git symbolic-ref HEAD) b.commitid'
+test_expect_success 'git get-tar-commit-id' '
+ git get-tar-commit-id <b.tar >actual &&
+ git rev-parse HEAD >expect &&
+ test_cmp expect actual
+'
test_expect_success 'git archive with --output, override inferred format' '
git archive --format=tar --output=d4.zip HEAD &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v2 10/21] t7003: use rev-parse rather than FS inspection
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (8 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 09/21] t5000: inspect HEAD using git-rev-parse Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 11/21] t5304: restyle: trim empty lines, drop ':' before > Han-Wen Nienhuys via GitGitGadget
` (12 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t7003-filter-branch.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 1349e5b2321c..cf30055c88dd 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -395,7 +395,7 @@ test_expect_success '--prune-empty is able to prune root commit' '
test_expect_success '--prune-empty is able to prune entire branch' '
git branch prune-entire B &&
git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire &&
- test_path_is_missing .git/refs/heads/prune-entire &&
+ test_must_fail git rev-parse refs/heads/prune-entire &&
test_must_fail git reflog exists refs/heads/prune-entire
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v2 11/21] t5304: restyle: trim empty lines, drop ':' before >
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (9 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 10/21] t7003: use rev-parse rather than FS inspection Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 12/21] t5304: use "reflog expire --all" to clear the reflog Han-Wen Nienhuys via GitGitGadget
` (11 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t5304-prune.sh | 74 +++++++++++++-----------------------------------
1 file changed, 20 insertions(+), 54 deletions(-)
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index b447ce56a9b2..7f47f13c78e8 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -22,30 +22,25 @@ add_blob() {
}
test_expect_success setup '
-
- : > file &&
+ >file &&
git add file &&
test_tick &&
git commit -m initial &&
git gc
-
'
test_expect_success 'prune stale packs' '
-
orig_pack=$(echo .git/objects/pack/*.pack) &&
- : > .git/objects/tmp_1.pack &&
- : > .git/objects/tmp_2.pack &&
+ >.git/objects/tmp_1.pack &&
+ >.git/objects/tmp_2.pack &&
test-tool chmtime =-86501 .git/objects/tmp_1.pack &&
git prune --expire 1.day &&
test_path_is_file $orig_pack &&
test_path_is_file .git/objects/tmp_2.pack &&
test_path_is_missing .git/objects/tmp_1.pack
-
'
test_expect_success 'prune --expire' '
-
add_blob &&
git prune --expire=1.hour.ago &&
verbose test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
@@ -54,11 +49,9 @@ test_expect_success 'prune --expire' '
git prune --expire 1.day &&
verbose test $before = $(git count-objects | sed "s/ .*//") &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc: implicit prune --expire' '
-
add_blob &&
test-tool chmtime =-$((2*$week-30)) $BLOB_FILE &&
git gc &&
@@ -68,33 +61,25 @@ test_expect_success 'gc: implicit prune --expire' '
git gc &&
verbose test $before = $(git count-objects | sed "s/ .*//") &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc: refuse to start with invalid gc.pruneExpire' '
-
git config gc.pruneExpire invalid &&
test_must_fail git gc
-
'
test_expect_success 'gc: start with ok gc.pruneExpire' '
-
git config gc.pruneExpire 2.days.ago &&
git gc
-
'
test_expect_success 'prune: prune nonsense parameters' '
-
test_must_fail git prune garbage &&
test_must_fail git prune --- &&
test_must_fail git prune --no-such-option
-
'
test_expect_success 'prune: prune unreachable heads' '
-
git config core.logAllRefUpdates false &&
mv .git/logs .git/logs.old &&
: > file2 &&
@@ -104,11 +89,9 @@ test_expect_success 'prune: prune unreachable heads' '
git reset HEAD^ &&
git prune &&
test_must_fail git reset $tmp_head --
-
'
test_expect_success 'prune: do not prune detached HEAD with no reflog' '
-
git checkout --detach --quiet &&
git commit --allow-empty -m "detached commit" &&
# verify that there is no reflogs
@@ -116,75 +99,61 @@ test_expect_success 'prune: do not prune detached HEAD with no reflog' '
test_path_is_missing .git/logs &&
git prune -n >prune_actual &&
test_must_be_empty prune_actual
-
'
test_expect_success 'prune: prune former HEAD after checking out branch' '
-
head_oid=$(git rev-parse HEAD) &&
git checkout --quiet main &&
git prune -v >prune_actual &&
grep "$head_oid" prune_actual
-
'
test_expect_success 'prune: do not prune heads listed as an argument' '
-
- : > file2 &&
+ >file2 &&
git add file2 &&
git commit -m temporary &&
tmp_head=$(git rev-list -1 HEAD) &&
git reset HEAD^ &&
git prune -- $tmp_head &&
git reset $tmp_head --
-
'
test_expect_success 'gc --no-prune' '
-
add_blob &&
test-tool chmtime =-$((5001*$day)) $BLOB_FILE &&
git config gc.pruneExpire 2.days.ago &&
git gc --no-prune &&
verbose test 1 = $(git count-objects | sed "s/ .*//") &&
test_path_is_file $BLOB_FILE
-
'
test_expect_success 'gc respects gc.pruneExpire' '
-
git config gc.pruneExpire 5002.days.ago &&
git gc &&
test_path_is_file $BLOB_FILE &&
git config gc.pruneExpire 5000.days.ago &&
git gc &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc --prune=<date>' '
-
add_blob &&
test-tool chmtime =-$((5001*$day)) $BLOB_FILE &&
git gc --prune=5002.days.ago &&
test_path_is_file $BLOB_FILE &&
git gc --prune=5000.days.ago &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc --prune=never' '
-
add_blob &&
git gc --prune=never &&
test_path_is_file $BLOB_FILE &&
git gc --prune=now &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc respects gc.pruneExpire=never' '
-
git config gc.pruneExpire never &&
add_blob &&
git gc &&
@@ -192,17 +161,14 @@ test_expect_success 'gc respects gc.pruneExpire=never' '
git config gc.pruneExpire now &&
git gc &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'prune --expire=never' '
-
add_blob &&
git prune --expire=never &&
test_path_is_file $BLOB_FILE &&
git prune &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc: prune old objects after local clone' '
@@ -222,16 +188,16 @@ test_expect_success 'gc: prune old objects after local clone' '
test_expect_success 'garbage report in count-objects -v' '
test_when_finished "rm -f .git/objects/pack/fake*" &&
test_when_finished "rm -f .git/objects/pack/foo*" &&
- : >.git/objects/pack/foo &&
- : >.git/objects/pack/foo.bar &&
- : >.git/objects/pack/foo.keep &&
- : >.git/objects/pack/foo.pack &&
- : >.git/objects/pack/fake.bar &&
- : >.git/objects/pack/fake.keep &&
- : >.git/objects/pack/fake.pack &&
- : >.git/objects/pack/fake.idx &&
- : >.git/objects/pack/fake2.keep &&
- : >.git/objects/pack/fake3.idx &&
+ >.git/objects/pack/foo &&
+ >.git/objects/pack/foo.bar &&
+ >.git/objects/pack/foo.keep &&
+ >.git/objects/pack/foo.pack &&
+ >.git/objects/pack/fake.bar &&
+ >.git/objects/pack/fake.keep &&
+ >.git/objects/pack/fake.pack &&
+ >.git/objects/pack/fake.idx &&
+ >.git/objects/pack/fake2.keep &&
+ >.git/objects/pack/fake3.idx &&
git count-objects -v 2>stderr &&
grep "index file .git/objects/pack/fake.idx is too small" stderr &&
grep "^warning:" stderr | sort >actual &&
@@ -250,12 +216,12 @@ EOF
test_expect_success 'clean pack garbage with gc' '
test_when_finished "rm -f .git/objects/pack/fake*" &&
test_when_finished "rm -f .git/objects/pack/foo*" &&
- : >.git/objects/pack/foo.keep &&
- : >.git/objects/pack/foo.pack &&
- : >.git/objects/pack/fake.idx &&
- : >.git/objects/pack/fake2.keep &&
- : >.git/objects/pack/fake2.idx &&
- : >.git/objects/pack/fake3.keep &&
+ >.git/objects/pack/foo.keep &&
+ >.git/objects/pack/foo.pack &&
+ >.git/objects/pack/fake.idx &&
+ >.git/objects/pack/fake2.keep &&
+ >.git/objects/pack/fake2.idx &&
+ >.git/objects/pack/fake3.keep &&
git gc &&
git count-objects -v 2>stderr &&
grep "^warning:" stderr | sort >actual &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v2 12/21] t5304: use "reflog expire --all" to clear the reflog
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (10 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 11/21] t5304: restyle: trim empty lines, drop ':' before > Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:27 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 13/21] test-lib: provide test prereq REFFILES Han-Wen Nienhuys via GitGitGadget
` (10 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
This is more explicit, and reduces the depency between test functions. It also
is more amenable to use with reftable, which has no concept of (non)existence of
a reflog
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t5304-prune.sh | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 7f47f13c78e8..7b850ae26171 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -81,12 +81,12 @@ test_expect_success 'prune: prune nonsense parameters' '
test_expect_success 'prune: prune unreachable heads' '
git config core.logAllRefUpdates false &&
- mv .git/logs .git/logs.old &&
- : > file2 &&
+ >file2 &&
git add file2 &&
git commit -m temporary &&
tmp_head=$(git rev-list -1 HEAD) &&
git reset HEAD^ &&
+ git reflog expire --all &&
git prune &&
test_must_fail git reset $tmp_head --
'
@@ -94,9 +94,7 @@ test_expect_success 'prune: prune unreachable heads' '
test_expect_success 'prune: do not prune detached HEAD with no reflog' '
git checkout --detach --quiet &&
git commit --allow-empty -m "detached commit" &&
- # verify that there is no reflogs
- # (should be removed and disabled by previous test)
- test_path_is_missing .git/logs &&
+ git reflog expire --all &&
git prune -n >prune_actual &&
test_must_be_empty prune_actual
'
@@ -104,6 +102,7 @@ test_expect_success 'prune: do not prune detached HEAD with no reflog' '
test_expect_success 'prune: prune former HEAD after checking out branch' '
head_oid=$(git rev-parse HEAD) &&
git checkout --quiet main &&
+ git reflog expire --all &&
git prune -v >prune_actual &&
grep "$head_oid" prune_actual
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 12/21] t5304: use "reflog expire --all" to clear the reflog
2021-04-27 10:38 ` [PATCH v2 12/21] t5304: use "reflog expire --all" to clear the reflog Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:27 ` Ævar Arnfjörð Bjarmason
2021-05-31 14:04 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:27 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> This is more explicit, and reduces the depency between test functions. It also
> is more amenable to use with reftable, which has no concept of (non)existence of
> a reflog
Sounds good in principle, but:
> @@ -94,9 +94,7 @@ test_expect_success 'prune: prune unreachable heads' '
> test_expect_success 'prune: do not prune detached HEAD with no reflog' '
> git checkout --detach --quiet &&
> git commit --allow-empty -m "detached commit" &&
> - # verify that there is no reflogs
> - # (should be removed and disabled by previous test)
> - test_path_is_missing .git/logs &&
> + git reflog expire --all &&
> git prune -n >prune_actual &&
> test_must_be_empty prune_actual
> '
Isn't the point of the existing test to check that there isn't an
existing reflog, not to just expire it if we find it, or does expire
--all return non-zero if none was found ?
> @@ -104,6 +102,7 @@ test_expect_success 'prune: do not prune detached HEAD with no reflog' '
> test_expect_success 'prune: prune former HEAD after checking out branch' '
> head_oid=$(git rev-parse HEAD) &&
> git checkout --quiet main &&
> + git reflog expire --all &&
> git prune -v >prune_actual &&
> grep "$head_oid" prune_actual
> '
Just skimming this I'm perplexed why this needs expiring now, as opposed
to being s/file/command/ changes like the rest...
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 12/21] t5304: use "reflog expire --all" to clear the reflog
2021-05-20 15:27 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 14:04 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 14:04 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:28 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
>
> > From: Han-Wen Nienhuys <hanwen@google.com>
> >
> > This is more explicit, and reduces the depency between test functions. It also
> > is more amenable to use with reftable, which has no concept of (non)existence of
> > a reflog
>
> Sounds good in principle, but:
>
> > @@ -94,9 +94,7 @@ test_expect_success 'prune: prune unreachable heads' '
> > test_expect_success 'prune: do not prune detached HEAD with no reflog' '
> > git checkout --detach --quiet &&
> > git commit --allow-empty -m "detached commit" &&
> > - # verify that there is no reflogs
> > - # (should be removed and disabled by previous test)
> > - test_path_is_missing .git/logs &&
> > + git reflog expire --all &&
> > git prune -n >prune_actual &&
> > test_must_be_empty prune_actual
> > '
>
> Isn't the point of the existing test to check that there isn't an
> existing reflog, not to just expire it if we find it, or does expire
> --all return non-zero if none was found ?
no. it's verifying that the cross-test behavior is working as
intended. The real objective is to make sure there is no reflog.
Clarified commit msg.
> > @@ -104,6 +102,7 @@ test_expect_success 'prune: do not prune detached HEAD with no reflog' '
> > test_expect_success 'prune: prune former HEAD after checking out branch' '
> > head_oid=$(git rev-parse HEAD) &&
> > git checkout --quiet main &&
> > + git reflog expire --all &&
> > git prune -v >prune_actual &&
> > grep "$head_oid" prune_actual
> > '
>
> Just skimming this I'm perplexed why this needs expiring now, as opposed
> to being s/file/command/ changes like the rest...
explained in the commit msg.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 13/21] test-lib: provide test prereq REFFILES
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (11 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 12/21] t5304: use "reflog expire --all" to clear the reflog Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 14/21] t1407: require REFFILES for for_each_reflog test Han-Wen Nienhuys via GitGitGadget
` (9 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
REFFILES can be used to mark tests that are specific to the packed/loose ref
storage format and its limitations. Marking such tests is a preparation for
introducing the reftable storage backend.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/README | 6 ++++++
t/test-lib.sh | 2 ++
2 files changed, 8 insertions(+)
diff --git a/t/README b/t/README
index fd9375b146d1..723bd3387fb7 100644
--- a/t/README
+++ b/t/README
@@ -1114,6 +1114,12 @@ use these, and "test_set_prereq" for how to define your own.
Git wasn't compiled with NO_PTHREADS=YesPlease.
+ - REFFILES
+
+ Test is specific to packed/loose ref storage, and should be
+ disabled for other ref storage backends
+
+
Tips for Writing Tests
----------------------
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 3dec266221cd..4a0c08e81e12 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1483,6 +1483,8 @@ parisc* | hppa*)
;;
esac
+test_set_prereq REFFILES
+
( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
test -z "$NO_PERL" && test_set_prereq PERL
test -z "$NO_PTHREADS" && test_set_prereq PTHREADS
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v2 14/21] t1407: require REFFILES for for_each_reflog test
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (12 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 13/21] test-lib: provide test prereq REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 15/21] t1414: mark corruption test with REFFILES Han-Wen Nienhuys via GitGitGadget
` (8 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Add extensive comment why this test needs a REFFILES annotation.
I tried forcing universal reflog creation with core.logAllRefUpdates=true, but
that apparently also doesn't cause reflogs to be created for pseudorefs
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t1407-worktree-ref-store.sh | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index d3fe77751122..ad8006c81397 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -52,7 +52,14 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' '
test_cmp expected actual
'
-test_expect_success 'for_each_reflog()' '
+# Some refs (refs/bisect/*, pseudorefs) are kept per worktree, so they should
+# only appear in the for-each-reflog output if it is called from the correct
+# worktree, which is exercised in this test. This test is poorly written (and
+# therefore marked REFFILES) for mulitple reasons: 1) it creates invalidly
+# formatted log entres. 2) it uses direct FS access for creating the reflogs. 3)
+# PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is
+# not testing a realistic scenario.
+test_expect_success REFFILES 'for_each_reflog()' '
echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
mkdir -p .git/logs/refs/bisect &&
echo $ZERO_OID > .git/logs/refs/bisect/random &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v2 15/21] t1414: mark corruption test with REFFILES
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (13 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 14/21] t1407: require REFFILES for for_each_reflog test Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:31 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 16/21] t2017: mark --orphan/logAllRefUpdates=false test as REFFILES Han-Wen Nienhuys via GitGitGadget
` (7 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
The reftable format guarantees that reflog entries are well-formed
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t1414-reflog-walk.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t1414-reflog-walk.sh b/t/t1414-reflog-walk.sh
index 80d94704d012..72a5ac61a520 100755
--- a/t/t1414-reflog-walk.sh
+++ b/t/t1414-reflog-walk.sh
@@ -119,7 +119,7 @@ test_expect_success 'min/max age uses entry date to limit' '
test_cmp expect actual
'
-test_expect_success 'walk prefers reflog to ref tip' '
+test_expect_success REFFILES 'walk prefers reflog to ref tip' '
head=$(git rev-parse HEAD) &&
one=$(git rev-parse one) &&
ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 15/21] t1414: mark corruption test with REFFILES
2021-04-27 10:38 ` [PATCH v2 15/21] t1414: mark corruption test with REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:31 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:31 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> The reftable format guarantees that reflog entries are well-formed
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t1414-reflog-walk.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t1414-reflog-walk.sh b/t/t1414-reflog-walk.sh
> index 80d94704d012..72a5ac61a520 100755
> --- a/t/t1414-reflog-walk.sh
> +++ b/t/t1414-reflog-walk.sh
> @@ -119,7 +119,7 @@ test_expect_success 'min/max age uses entry date to limit' '
> test_cmp expect actual
> '
>
> -test_expect_success 'walk prefers reflog to ref tip' '
> +test_expect_success REFFILES 'walk prefers reflog to ref tip' '
> head=$(git rev-parse HEAD) &&
> one=$(git rev-parse one) &&
> ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
Isn't the point of this test added in 7cf686b9a8 (t1414: document some
reflog-walk oddities, 2017-07-07) to test what we do not when the reflog
isn't well formed, but when the reflog and branch history disagree about
what the latest tip is?
Perhaps reftable is going to guarantee that that'll never happen via
some transaction mechanism (I'm not sure), but if I'm right then I think
it's worth at least calling out that difference in the commit message
and/or modified test description.
Not a big deal, but this also makes me somewhat question the name
"REFFILES" as a prereq if we really mean (as we are in the first test
that uses this) "we don't need this under reftable, specifically".
Maybe I'm overthinking things and it's stupid to split these up, as we
have no other planned ref backend, but for any such future test backend
/ readability I'd find it much easier to grok if we clearly
deliniate/use REFFILES for tests that really are testing the ref files
backend specifically, v.s. say a !REFTABLE for things that we're
skipping because REFTABLE in particular makes certain guarantees we can
count on. Say that the reflog and branch is never out of sync.
Of course this also uses ref .. files, anyway, just food for thought.
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 16/21] t2017: mark --orphan/logAllRefUpdates=false test as REFFILES
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (14 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 15/21] t1414: mark corruption test with REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:38 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 17/21] t1404: mark tests that muck with .git directly " Han-Wen Nienhuys via GitGitGadget
` (6 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
In reftable, there is no notion of a per-ref 'existence' of a reflog. Each
reflog entry has its own key, so it is not possible to distinguish between
{reflog doesn't exist,reflog exists but is empty}. This makes the logic
in log_ref_setup() (file refs/files-backend.c), which depends on the existence
of the reflog file infeasible.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t2017-checkout-orphan.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
index c7adbdd39ab9..88d6992a5e1f 100755
--- a/t/t2017-checkout-orphan.sh
+++ b/t/t2017-checkout-orphan.sh
@@ -76,7 +76,7 @@ test_expect_success '--orphan makes reflog by default' '
git rev-parse --verify delta@{0}
'
-test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '
+test_expect_success REFFILES '--orphan does not make reflog when core.logAllRefUpdates = false' '
git checkout main &&
git config core.logAllRefUpdates false &&
git checkout --orphan epsilon &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 16/21] t2017: mark --orphan/logAllRefUpdates=false test as REFFILES
2021-04-27 10:38 ` [PATCH v2 16/21] t2017: mark --orphan/logAllRefUpdates=false test as REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:38 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:38 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> In reftable, there is no notion of a per-ref 'existence' of a reflog. Each
> reflog entry has its own key, so it is not possible to distinguish between
> {reflog doesn't exist,reflog exists but is empty}. This makes the logic
> in log_ref_setup() (file refs/files-backend.c), which depends on the existence
> of the reflog file infeasible.
Okey...
> -test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '
> +test_expect_success REFFILES '--orphan does not make reflog when core.logAllRefUpdates = false' '
> git checkout main &&
> git config core.logAllRefUpdates false &&
> git checkout --orphan epsilon &&
But let's not simply skip the test, but positively test for the reftable
behavior here, or maybe that's planned for a later series.
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 17/21] t1404: mark tests that muck with .git directly as REFFILES.
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (15 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 16/21] t2017: mark --orphan/logAllRefUpdates=false test as REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-04-27 10:38 ` [PATCH v2 18/21] t7900: mark pack-refs tests " Han-Wen Nienhuys via GitGitGadget
` (5 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
The packed/loose ref storage is an overlay combination of packed-refs (refs and
tags in a single file) and one-file-per-ref. This creates all kinds of edge
cases related to directory/file conflicts, (non-)empty directories, and the
locking scheme, none of which applies to reftable.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t1404-update-ref-errors.sh | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index 8b51c4efc135..b729c1f48030 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -189,7 +189,7 @@ test_expect_success 'one new ref is a simple prefix of another' '
'
-test_expect_success 'empty directory should not fool rev-parse' '
+test_expect_success REFFILES 'empty directory should not fool rev-parse' '
prefix=refs/e-rev-parse &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -199,7 +199,7 @@ test_expect_success 'empty directory should not fool rev-parse' '
test_cmp expected actual
'
-test_expect_success 'empty directory should not fool for-each-ref' '
+test_expect_success REFFILES 'empty directory should not fool for-each-ref' '
prefix=refs/e-for-each-ref &&
git update-ref $prefix/foo $C &&
git for-each-ref $prefix >expected &&
@@ -209,14 +209,14 @@ test_expect_success 'empty directory should not fool for-each-ref' '
test_cmp expected actual
'
-test_expect_success 'empty directory should not fool create' '
+test_expect_success REFFILES 'empty directory should not fool create' '
prefix=refs/e-create &&
mkdir -p .git/$prefix/foo/bar/baz &&
printf "create %s $C\n" $prefix/foo |
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool verify' '
+test_expect_success REFFILES 'empty directory should not fool verify' '
prefix=refs/e-verify &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -225,7 +225,7 @@ test_expect_success 'empty directory should not fool verify' '
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool 1-arg update' '
+test_expect_success REFFILES 'empty directory should not fool 1-arg update' '
prefix=refs/e-update-1 &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -234,7 +234,7 @@ test_expect_success 'empty directory should not fool 1-arg update' '
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool 2-arg update' '
+test_expect_success REFFILES 'empty directory should not fool 2-arg update' '
prefix=refs/e-update-2 &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -243,7 +243,7 @@ test_expect_success 'empty directory should not fool 2-arg update' '
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool 0-arg delete' '
+test_expect_success REFFILES 'empty directory should not fool 0-arg delete' '
prefix=refs/e-delete-0 &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -252,7 +252,7 @@ test_expect_success 'empty directory should not fool 0-arg delete' '
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool 1-arg delete' '
+test_expect_success REFFILES 'empty directory should not fool 1-arg delete' '
prefix=refs/e-delete-1 &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -466,7 +466,7 @@ test_expect_success 'incorrect old value blocks indirect no-deref delete' '
test_cmp expected output.err
'
-test_expect_success 'non-empty directory blocks create' '
+test_expect_success REFFILES 'non-empty directory blocks create' '
prefix=refs/ne-create &&
mkdir -p .git/$prefix/foo/bar &&
: >.git/$prefix/foo/bar/baz.lock &&
@@ -485,7 +485,7 @@ test_expect_success 'non-empty directory blocks create' '
test_cmp expected output.err
'
-test_expect_success 'broken reference blocks create' '
+test_expect_success REFFILES 'broken reference blocks create' '
prefix=refs/broken-create &&
mkdir -p .git/$prefix &&
echo "gobbledigook" >.git/$prefix/foo &&
@@ -504,7 +504,7 @@ test_expect_success 'broken reference blocks create' '
test_cmp expected output.err
'
-test_expect_success 'non-empty directory blocks indirect create' '
+test_expect_success REFFILES 'non-empty directory blocks indirect create' '
prefix=refs/ne-indirect-create &&
git symbolic-ref $prefix/symref $prefix/foo &&
mkdir -p .git/$prefix/foo/bar &&
@@ -524,7 +524,7 @@ test_expect_success 'non-empty directory blocks indirect create' '
test_cmp expected output.err
'
-test_expect_success 'broken reference blocks indirect create' '
+test_expect_success REFFILES 'broken reference blocks indirect create' '
prefix=refs/broken-indirect-create &&
git symbolic-ref $prefix/symref $prefix/foo &&
echo "gobbledigook" >.git/$prefix/foo &&
@@ -543,7 +543,7 @@ test_expect_success 'broken reference blocks indirect create' '
test_cmp expected output.err
'
-test_expect_success 'no bogus intermediate values during delete' '
+test_expect_success REFFILES 'no bogus intermediate values during delete' '
prefix=refs/slow-transaction &&
# Set up a reference with differing loose and packed versions:
git update-ref $prefix/foo $C &&
@@ -600,7 +600,7 @@ test_expect_success 'no bogus intermediate values during delete' '
test_must_fail git rev-parse --verify --quiet $prefix/foo
'
-test_expect_success 'delete fails cleanly if packed-refs file is locked' '
+test_expect_success REFFILES 'delete fails cleanly if packed-refs file is locked' '
prefix=refs/locked-packed-refs &&
# Set up a reference with differing loose and packed versions:
git update-ref $prefix/foo $C &&
@@ -616,7 +616,7 @@ test_expect_success 'delete fails cleanly if packed-refs file is locked' '
test_cmp unchanged actual
'
-test_expect_success 'delete fails cleanly if packed-refs.new write fails' '
+test_expect_success REFFILES 'delete fails cleanly if packed-refs.new write fails' '
# Setup and expectations are similar to the test above.
prefix=refs/failed-packed-refs &&
git update-ref $prefix/foo $C &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v2 18/21] t7900: mark pack-refs tests as REFFILES
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (16 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 17/21] t1404: mark tests that muck with .git directly " Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:40 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 19/21] t7003: check reflog existence only for REFFILES Han-Wen Nienhuys via GitGitGadget
` (4 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Reftable automatically compacts tables on writes. The functionality of
incremental compaction is unittested in reftable/stack_test.c
(test_reftable_stack_auto_compaction)
In addition, pack-refs triggers a full compaction of the entire stack. This is
exercised in t0031-reftable.sh.
Given that git-maintenance simply calls out git-pack-refs, it seems superfluous
to test this further for reftable.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t7900-maintenance.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 2412d8c5c006..6f2f55a6c51d 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -343,7 +343,7 @@ test_expect_success 'maintenance.incremental-repack.auto' '
test_subcommand git multi-pack-index write --no-progress <trace-B
'
-test_expect_success 'pack-refs task' '
+test_expect_success REFFILES 'pack-refs task' '
for n in $(test_seq 1 5)
do
git branch -f to-pack/$n HEAD || return 1
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 18/21] t7900: mark pack-refs tests as REFFILES
2021-04-27 10:38 ` [PATCH v2 18/21] t7900: mark pack-refs tests " Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:40 ` Ævar Arnfjörð Bjarmason
2021-05-31 14:08 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:40 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Reftable automatically compacts tables on writes. The functionality of
> incremental compaction is unittested in reftable/stack_test.c
> (test_reftable_stack_auto_compaction)
>
> In addition, pack-refs triggers a full compaction of the entire stack. This is
> exercised in t0031-reftable.sh.
>
> Given that git-maintenance simply calls out git-pack-refs, it seems superfluous
> to test this further for reftable.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t7900-maintenance.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index 2412d8c5c006..6f2f55a6c51d 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -343,7 +343,7 @@ test_expect_success 'maintenance.incremental-repack.auto' '
> test_subcommand git multi-pack-index write --no-progress <trace-B
> '
>
> -test_expect_success 'pack-refs task' '
> +test_expect_success REFFILES 'pack-refs task' '
> for n in $(test_seq 1 5)
> do
> git branch -f to-pack/$n HEAD || return 1
I don't think it's superfluous to test what "git maintenance" does
anyway, i.e. the test ends with:
test_subcommand git pack-refs --all --prune
Shouldn't we test that we .. don't run that, abort, warn, whatever?
Anyway, as with another earlier comment of mine we're going to have
chicken & egg problems with amending these tests and then actually
introducing reftable support, so maybe "not yet", but I worry in general
about the loss of coverage...
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 18/21] t7900: mark pack-refs tests as REFFILES
2021-05-20 15:40 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 14:08 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 14:08 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:41 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> > -test_expect_success 'pack-refs task' '
> > +test_expect_success REFFILES 'pack-refs task' '
> > for n in $(test_seq 1 5)
> > do
> > git branch -f to-pack/$n HEAD || return 1
>
> I don't think it's superfluous to test what "git maintenance" does
> anyway, i.e. the test ends with:
>
> test_subcommand git pack-refs --all --prune
>
> Shouldn't we test that we .. don't run that, abort, warn, whatever?
Sure. I've removed the check on the side effect for pack-refs instead.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 19/21] t7003: check reflog existence only for REFFILES
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (17 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 18/21] t7900: mark pack-refs tests " Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:41 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 20/21] t4202: mark bogus head hash test with REFFILES Han-Wen Nienhuys via GitGitGadget
` (3 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t7003-filter-branch.sh | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index cf30055c88dd..e18a21895238 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -396,7 +396,10 @@ test_expect_success '--prune-empty is able to prune entire branch' '
git branch prune-entire B &&
git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire &&
test_must_fail git rev-parse refs/heads/prune-entire &&
- test_must_fail git reflog exists refs/heads/prune-entire
+ if test_have_prereq REFFILES
+ then
+ test_must_fail git reflog exists refs/heads/prune-entire
+ fi
'
test_expect_success '--remap-to-ancestor with filename filters' '
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 19/21] t7003: check reflog existence only for REFFILES
2021-04-27 10:38 ` [PATCH v2 19/21] t7003: check reflog existence only for REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:41 ` Ævar Arnfjörð Bjarmason
2021-05-31 14:27 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:41 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t7003-filter-branch.sh | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> index cf30055c88dd..e18a21895238 100755
> --- a/t/t7003-filter-branch.sh
> +++ b/t/t7003-filter-branch.sh
> @@ -396,7 +396,10 @@ test_expect_success '--prune-empty is able to prune entire branch' '
> git branch prune-entire B &&
> git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire &&
> test_must_fail git rev-parse refs/heads/prune-entire &&
> - test_must_fail git reflog exists refs/heads/prune-entire
> + if test_have_prereq REFFILES
> + then
> + test_must_fail git reflog exists refs/heads/prune-entire
> + fi
> '
>
> test_expect_success '--remap-to-ancestor with filename filters' '
Ditto chicken and egg, but isn't this conflating "we always write logs"
v.s. reftable just behaving differently, i.e. shouldn't this be
positively asserting that we have the log *for that branch* still after
its deletion?
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 19/21] t7003: check reflog existence only for REFFILES
2021-05-20 15:41 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 14:27 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 14:27 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:42 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> > - test_must_fail git reflog exists refs/heads/prune-entire
> > + if test_have_prereq REFFILES
> > + then
> > + test_must_fail git reflog exists refs/heads/prune-entire
> > + fi
> > '
> >
> > test_expect_success '--remap-to-ancestor with filename filters' '
>
> Ditto chicken and egg, but isn't this conflating "we always write logs"
> v.s. reftable just behaving differently, i.e. shouldn't this be
> positively asserting that we have the log *for that branch* still after
> its deletion?
That is a great topic to discuss on the reftable series.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 20/21] t4202: mark bogus head hash test with REFFILES
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (18 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 19/21] t7003: check reflog existence only for REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:43 ` Ævar Arnfjörð Bjarmason
2021-04-27 10:38 ` [PATCH v2 21/21] t1415: set REFFILES for test specific to storage format Han-Wen Nienhuys via GitGitGadget
` (2 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
In reftable, hashes are correctly formed by design
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t4202-log.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index a8c5a00d012d..3f969b01508c 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1834,7 +1834,7 @@ test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
'
-test_expect_success 'log diagnoses bogus HEAD hash' '
+test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
git init empty &&
test_must_fail git -C empty log 2>stderr &&
test_i18ngrep does.not.have.any.commits stderr &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 20/21] t4202: mark bogus head hash test with REFFILES
2021-04-27 10:38 ` [PATCH v2 20/21] t4202: mark bogus head hash test with REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:43 ` Ævar Arnfjörð Bjarmason
2021-05-31 14:21 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:43 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> In reftable, hashes are correctly formed by design
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t4202-log.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index a8c5a00d012d..3f969b01508c 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -1834,7 +1834,7 @@ test_expect_success 'log --graph --no-walk is forbidden' '
> test_must_fail git log --graph --no-walk
> '
>
> -test_expect_success 'log diagnoses bogus HEAD hash' '
> +test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
> git init empty &&
> test_must_fail git -C empty log 2>stderr &&
> test_i18ngrep does.not.have.any.commits stderr &&
Okey, they're correctly formed by design, but the first test here is
just checking what happens when we do a "git log" on a repo with no
commits. What does that have to do with reflog's guarantees that we have
a valid-looking SHA in some entry in its database?
Surely we also want to test for the same thing, the ref backend doesn't
change that we have no commits or refs yet, no?
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 20/21] t4202: mark bogus head hash test with REFFILES
2021-05-20 15:43 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 14:21 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 14:21 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:44 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> > -test_expect_success 'log diagnoses bogus HEAD hash' '
> > +test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
> > git init empty &&
> > test_must_fail git -C empty log 2>stderr &&
> > test_i18ngrep does.not.have.any.commits stderr &&
>
> Okey, they're correctly formed by design, but the first test here is
> just checking what happens when we do a "git log" on a repo with no
> commits. What does that have to do with reflog's guarantees that we have
> a valid-looking SHA in some entry in its database?
>
> Surely we also want to test for the same thing, the ref backend doesn't
> change that we have no commits or refs yet, no?
Done.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v2 21/21] t1415: set REFFILES for test specific to storage format
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (19 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 20/21] t4202: mark bogus head hash test with REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-04-27 10:38 ` Han-Wen Nienhuys via GitGitGadget
2021-05-20 15:50 ` Ævar Arnfjörð Bjarmason
2021-05-20 16:28 ` [PATCH v2 00/21] Prepare tests for reftable backend Ævar Arnfjörð Bjarmason
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-04-27 10:38 UTC (permalink / raw)
To: git; +Cc: Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Packing refs (and therefore checking that certain refs are not packed) is a
property of the packed/loose ref storage. Add a comment to explain what the test
checks.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t1415-worktree-refs.sh | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 7ab91241ab7c..d6e6e523bbba 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -16,7 +16,10 @@ test_expect_success 'setup' '
git -C wt2 update-ref refs/worktree/foo HEAD
'
-test_expect_success 'refs/worktree must not be packed' '
+# The 'packed-refs' files is stored directly in .git/. This means it is global
+# to the repository, and can only contain refs that are shared across all
+# worktrees.
+test_expect_success REFFILES 'refs/worktree must not be packed' '
git pack-refs --all &&
test_path_is_missing .git/refs/tags/wt1 &&
test_path_is_file .git/refs/worktree/foo &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v2 21/21] t1415: set REFFILES for test specific to storage format
2021-04-27 10:38 ` [PATCH v2 21/21] t1415: set REFFILES for test specific to storage format Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 15:50 ` Ævar Arnfjörð Bjarmason
2021-05-31 14:16 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 15:50 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Packing refs (and therefore checking that certain refs are not packed) is a
> property of the packed/loose ref storage. Add a comment to explain what the test
> checks.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
> t/t1415-worktree-refs.sh | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
> index 7ab91241ab7c..d6e6e523bbba 100755
> --- a/t/t1415-worktree-refs.sh
> +++ b/t/t1415-worktree-refs.sh
> @@ -16,7 +16,10 @@ test_expect_success 'setup' '
> git -C wt2 update-ref refs/worktree/foo HEAD
> '
>
> -test_expect_success 'refs/worktree must not be packed' '
> +# The 'packed-refs' files is stored directly in .git/. This means it is global
> +# to the repository, and can only contain refs that are shared across all
> +# worktrees.
> +test_expect_success REFFILES 'refs/worktree must not be packed' '
> git pack-refs --all &&
> test_path_is_missing .git/refs/tags/wt1 &&
> test_path_is_file .git/refs/worktree/foo &&
Nit but also chicken & egg: Let's keep the "pack-refs --all" though
under reftable in its own test, and do the test_path_* assertions just
under REFFILES, i.e. does/should pack-refs warn/error under reftable as
redundant, succeed silently?
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 21/21] t1415: set REFFILES for test specific to storage format
2021-05-20 15:50 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 14:16 ` Han-Wen Nienhuys
2021-05-31 23:36 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 14:16 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 5:51 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> > -test_expect_success 'refs/worktree must not be packed' '
> > +# The 'packed-refs' files is stored directly in .git/. This means it is global
> > +# to the repository, and can only contain refs that are shared across all
> > +# worktrees.
> > +test_expect_success REFFILES 'refs/worktree must not be packed' '
> > git pack-refs --all &&
> > test_path_is_missing .git/refs/tags/wt1 &&
> > test_path_is_file .git/refs/worktree/foo &&
>
> Nit but also chicken & egg: Let's keep the "pack-refs --all" though
> under reftable in its own test, and do the test_path_* assertions just
> under REFFILES, i.e. does/should pack-refs warn/error under reftable as
> redundant, succeed silently?
I don't understand your comment. This test is checking constraints on
worktrees that are only relevant for loose/packed storage. In fact,
under reftable, there is no such thing as a "packed ref" storage
class. This test is just not relevant for reftable, and I tried
explaining in the comment why this is so. Happy to hear how to further
clarify this comment.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 21/21] t1415: set REFFILES for test specific to storage format
2021-05-31 14:16 ` Han-Wen Nienhuys
@ 2021-05-31 23:36 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-31 23:36 UTC (permalink / raw)
To: Han-Wen Nienhuys; +Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Mon, May 31 2021, Han-Wen Nienhuys wrote:
> On Thu, May 20, 2021 at 5:51 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> > -test_expect_success 'refs/worktree must not be packed' '
>> > +# The 'packed-refs' files is stored directly in .git/. This means it is global
>> > +# to the repository, and can only contain refs that are shared across all
>> > +# worktrees.
>> > +test_expect_success REFFILES 'refs/worktree must not be packed' '
>> > git pack-refs --all &&
>> > test_path_is_missing .git/refs/tags/wt1 &&
>> > test_path_is_file .git/refs/worktree/foo &&
>>
>> Nit but also chicken & egg: Let's keep the "pack-refs --all" though
>> under reftable in its own test, and do the test_path_* assertions just
>> under REFFILES, i.e. does/should pack-refs warn/error under reftable as
>> redundant, succeed silently?
>
> I don't understand your comment. This test is checking constraints on
> worktrees that are only relevant for loose/packed storage. In fact,
> under reftable, there is no such thing as a "packed ref" storage
> class. This test is just not relevant for reftable, and I tried
> explaining in the comment why this is so. Happy to hear how to further
> clarify this comment.
The chicken & egg part is that reftable isn't landed in-tree yet, so
it's probably pointless to discuss at this point.
But what I was getting at is that while the test when written was trying
to test that the file backend is doing some particular thing, when we go
through our tests to find those cases we should at least make a mental
note (hence my E-Mail) to eventually positively assert the new behavior.
I.e. if pack-refs is a noop shouldn't it warn or something, and then the
tests should check if reftable warn, else do_old_stuff().
In this case one can imagine that e.g. a naïve buggy implementation
would check if .git/reftable or whatever exists, which of course with
".git" being a plain file in worktrees won't work, so testing this
particular case somewhere in the worktree tests is probably prudent...
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 00/21] Prepare tests for reftable backend
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (20 preceding siblings ...)
2021-04-27 10:38 ` [PATCH v2 21/21] t1415: set REFFILES for test specific to storage format Han-Wen Nienhuys via GitGitGadget
@ 2021-05-20 16:28 ` Ævar Arnfjörð Bjarmason
2021-05-26 9:23 ` Han-Wen Nienhuys
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
22 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-20 16:28 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget; +Cc: git, Han-Wen Nienhuys, Han-Wen Nienhuys
On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> Rewrites some tests to avoid direct filesystem access.
>
> Introduces the test prereq REFFILES to mark other tests that depend on
> specifics of the files ref backend.
>
> changes in v2 (relative to v1 from Apr 19)
>
> * t9300: use ref-store test-helper to read toplevel tag.
> * t1401: use $TAR for restoring.
> * t1407: clarify what test is doing.
> * t1417: "$TAR" rather than tar
> * t1415: clarify test objective.
> * t7003: formatting.
> * README entry for REFFILES.
>
> cc: SZEDER Gábor szeder.dev@gmail.com cc: Ævar Arnfjörð Bjarmason
> avarab@gmail.com cc: Han-Wen Nienhuys hanwen@google.com
>
> cc: Han-Wen Nienhuys hanwen@google.com
>
> Han-Wen Nienhuys (21):
> t4202: split testcase for invalid HEAD symref and HEAD hash
> t/helper/ref-store: initialize oid in resolve-ref
> t9300: check ref existence using test-helper rather than a file system
> check
> t5601: read HEAD using rev-parse
> t1401-symbolic-ref: avoid direct filesystem access
> t1413: use tar to save and restore entire .git directory
> t1301: fix typo in error message
> t5000: reformat indentation to the latest fashion
> t5000: inspect HEAD using git-rev-parse
> t7003: use rev-parse rather than FS inspection
> t5304: restyle: trim empty lines, drop ':' before >
> t5304: use "reflog expire --all" to clear the reflog
> test-lib: provide test prereq REFFILES
> t1407: require REFFILES for for_each_reflog test
> t1414: mark corruption test with REFFILES
> t2017: mark --orphan/logAllRefUpdates=false test as REFFILES
> t1404: mark tests that muck with .git directly as REFFILES.
> t7900: mark pack-refs tests as REFFILES
> t7003: check reflog existence only for REFFILES
> t4202: mark bogus head hash test with REFFILES
> t1415: set REFFILES for test specific to storage format
>
> t/README | 6 ++
> t/helper/test-ref-store.c | 2 +-
> t/t1301-shared-repo.sh | 2 +-
> t/t1401-symbolic-ref.sh | 34 +++++-----
> t/t1404-update-ref-errors.sh | 30 ++++-----
> t/t1407-worktree-ref-store.sh | 9 ++-
> t/t1413-reflog-detach.sh | 5 +-
> t/t1414-reflog-walk.sh | 2 +-
> t/t1415-worktree-refs.sh | 5 +-
> t/t2017-checkout-orphan.sh | 2 +-
> t/t4202-log.sh | 10 ++-
> t/t5000-tar-tree.sh | 113 +++++++++++++++++-----------------
> t/t5304-prune.sh | 83 ++++++++-----------------
> t/t5601-clone.sh | 3 +-
> t/t7003-filter-branch.sh | 7 ++-
> t/t7900-maintenance.sh | 2 +-
> t/t9300-fast-import.sh | 2 +-
> t/test-lib.sh | 2 +
> 18 files changed, 156 insertions(+), 163 deletions(-)
>
>
> base-commit: 311531c9de557d25ac087c1637818bd2aad6eb3a
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1008%2Fhanwen%2Freffiles-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1008/hanwen/reffiles-v2
> Pull-Request: https://github.com/git/git/pull/1008
>
> Range-diff vs v1:
>
> 1: 91ef012cbcc9 ! 1: 8ad4a35cda70 t4202: split testcase for invalid HEAD symref and HEAD hash
> @@ t/t4202-log.sh: test_expect_success 'log --graph --no-walk is forbidden' '
> + test_i18ngrep broken stderr'
> +
> +test_expect_success 'log diagnoses bogus HEAD symref' '
> ++ rm -rf empty &&
> + git init empty &&
> + git --git-dir empty/.git symbolic-ref HEAD refs/heads/invalid.lock &&
> test_must_fail git -C empty log 2>stderr &&
> -: ------------ > 2: e6222944a3eb t/helper/ref-store: initialize oid in resolve-ref
> 2: ccc26a8950be ! 3: c5855b0caa73 t9300: check ref existence using git-rev-parse rather than FS check
> @@ Metadata
> Author: Han-Wen Nienhuys <hanwen@google.com>
>
> ## Commit message ##
> - t9300: check ref existence using git-rev-parse rather than FS check
> + t9300: check ref existence using test-helper rather than a file system check
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
>
> @@ t/t9300-fast-import.sh: test_expect_success 'B: accept branch name "TEMP_TAG"' '
> git prune" &&
> git fast-import <input &&
> - test -f .git/TEMP_TAG &&
> -+ git rev-parse TEMP_TAG &&
> ++ test $(test-tool ref-store main resolve-ref TEMP_TAG 0 | cut -f1 -d " " ) != "$ZERO_OID" &&
> test $(git rev-parse main) = $(git rev-parse TEMP_TAG^)
> '
>
> 3: 47b5ec56a383 = 4: 369c968ab837 t5601: read HEAD using rev-parse
> 4: 53cf1069552b ! 5: 248d9ffe7927 t1401-symbolic-ref: avoid direct filesystem access
> @@ Commit message
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
>
> ## t/t1401-symbolic-ref.sh ##
> -@@ t/t1401-symbolic-ref.sh: test_description='basic symbolic-ref tests'
> - # the git repo, meaning that further tests will operate on
> - # the surrounding git repo instead of the trash directory.
> - reset_to_sane() {
> -- echo ref: refs/heads/foo >.git/HEAD
> -+ git --git-dir .git symbolic-ref HEAD refs/heads/foo
> - }
> +@@
> + test_description='basic symbolic-ref tests'
> + . ./test-lib.sh
>
> +-# If the tests munging HEAD fail, they can break detection of
> +-# the git repo, meaning that further tests will operate on
> +-# the surrounding git repo instead of the trash directory.
> +-reset_to_sane() {
> +- echo ref: refs/heads/foo >.git/HEAD
> +-}
> +-
> -test_expect_success 'symbolic-ref writes HEAD' '
> -- git symbolic-ref HEAD refs/heads/foo &&
> ++test_expect_success 'setup' '
> + git symbolic-ref HEAD refs/heads/foo &&
> - echo ref: refs/heads/foo >expect &&
> - test_cmp expect .git/HEAD
> --'
> --
> ++ test_commit file &&
> ++ "$TAR" cf .git.tar .git/
> + '
> +
> -test_expect_success 'symbolic-ref reads HEAD' '
> - echo refs/heads/foo >expect &&
> -- git symbolic-ref HEAD >actual &&
> ++reset_to_sane() {
> ++ rm -rf .git &&
> ++ "$TAR" xf .git.tar
> ++}
> ++
> +test_expect_success 'symbolic-ref read/write roundtrip' '
> + git symbolic-ref HEAD refs/heads/read-write-roundtrip &&
> -+ echo refs/heads/read-write-roundtrip > expect &&
> -+ git symbolic-ref HEAD > actual &&
> ++ echo refs/heads/read-write-roundtrip >expect &&
> + git symbolic-ref HEAD >actual &&
> test_cmp expect actual
> '
> +@@ t/t1401-symbolic-ref.sh: test_expect_success 'symbolic-ref reads HEAD' '
> + test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
> + test_must_fail git symbolic-ref HEAD foo
> + '
> ++
> + reset_to_sane
> +
> + test_expect_success 'symbolic-ref refuses bare sha1' '
> +- echo content >file && git add file && git commit -m one &&
> + test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD)
> + '
> ++
> + reset_to_sane
>
> + test_expect_success 'HEAD cannot be removed' '
> @@ t/t1401-symbolic-ref.sh: reset_to_sane
> test_expect_success 'symbolic-ref can be deleted' '
> git symbolic-ref NOTHEAD refs/heads/foo &&
> @@ t/t1401-symbolic-ref.sh: reset_to_sane
> - test_path_is_file .git/refs/heads/foo &&
> - test_path_is_missing .git/NOTHEAD
> + git rev-parse refs/heads/foo &&
> -+ ! git symbolic-ref NOTHEAD
> ++ test_must_fail git symbolic-ref NOTHEAD
> '
> reset_to_sane
>
> @@ t/t1401-symbolic-ref.sh: reset_to_sane
> git symbolic-ref -d NOTHEAD &&
> - test_path_is_missing .git/refs/heads/missing &&
> - test_path_is_missing .git/NOTHEAD
> -+ ! git rev-parse refs/heads/missing &&
> -+ ! git symbolic-ref NOTHEAD
> ++ test_must_fail git rev-parse refs/heads/missing &&
> ++ test_must_fail git symbolic-ref NOTHEAD
> '
> reset_to_sane
>
> 5: 223583594c00 ! 6: e4e8fc1d4b4f t1413: use tar to save and restore entire .git directory
> @@ t/t1413-reflog-detach.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> reset_state () {
> - git checkout main &&
> - cp saved_reflog .git/logs/HEAD
> -+ rm -rf .git && tar -xf .git-saved.tar
> ++ rm -rf .git && "$TAR" xf .git-saved.tar
> }
>
> test_expect_success setup '
> @@ t/t1413-reflog-detach.sh: test_expect_success setup '
> test_tick &&
> git commit --allow-empty -m second &&
> - cat .git/logs/HEAD >saved_reflog
> -+ tar -cf .git-saved.tar .git
> ++ "$TAR" cf .git-saved.tar .git
> '
>
> test_expect_success baseline '
> 6: 70da8f5631d0 = 7: 89cc215c6014 t1301: fix typo in error message
> -: ------------ > 8: e67b90847c4e t5000: reformat indentation to the latest fashion
> 7: 79843c0d5727 ! 9: d6072a70ae7d t5000: inspect HEAD using git-rev-parse
> @@ Commit message
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
>
> ## t/t5000-tar-tree.sh ##
> -@@ t/t5000-tar-tree.sh: test_expect_success \
> - test_cmp expected.mtime b.mtime'
> +@@ t/t5000-tar-tree.sh: test_expect_success 'validate file modification time' '
> + test_cmp expected.mtime b.mtime
> + '
>
> - test_expect_success \
> +-test_expect_success \
> - 'git get-tar-commit-id' \
> - 'git get-tar-commit-id <b.tar >b.commitid &&
> - test_cmp .git/$(git symbolic-ref HEAD) b.commitid'
> -+ 'git get-tar-commit-id' \
> -+ 'git get-tar-commit-id <b.tar >actual &&
> -+ git rev-parse HEAD > expect &&
> -+ test_cmp expect actual'
> ++test_expect_success 'git get-tar-commit-id' '
> ++ git get-tar-commit-id <b.tar >actual &&
> ++ git rev-parse HEAD >expect &&
> ++ test_cmp expect actual
> ++'
>
> test_expect_success 'git archive with --output, override inferred format' '
> git archive --format=tar --output=d4.zip HEAD &&
> 8: dbb81b5b89d8 = 10: 4bf1bf16bca3 t7003: use rev-parse rather than FS inspection
> -: ------------ > 11: 6f15c15573af t5304: restyle: trim empty lines, drop ':' before >
> 9: ba575839e422 ! 12: d8e80d83b6f8 t5304: use "reflog expire --all" to clear the reflog
> @@ Commit message
>
> ## t/t5304-prune.sh ##
> @@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' '
> - '
>
> test_expect_success 'prune: prune unreachable heads' '
> --
> git config core.logAllRefUpdates false &&
> - mv .git/logs .git/logs.old &&
> - : > file2 &&
> +- : > file2 &&
> ++ >file2 &&
> git add file2 &&
> git commit -m temporary &&
> tmp_head=$(git rev-list -1 HEAD) &&
> @@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' '
> + git reflog expire --all &&
> git prune &&
> test_must_fail git reset $tmp_head --
> --
> '
> -
> +@@ t/t5304-prune.sh: test_expect_success 'prune: prune unreachable heads' '
> test_expect_success 'prune: do not prune detached HEAD with no reflog' '
> -
> git checkout --detach --quiet &&
> git commit --allow-empty -m "detached commit" &&
> - # verify that there is no reflogs
> @@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' '
> + git reflog expire --all &&
> git prune -n >prune_actual &&
> test_must_be_empty prune_actual
> -
> -@@ t/t5304-prune.sh: test_expect_success 'prune: prune former HEAD after checking out branch' '
> -
> + '
> +@@ t/t5304-prune.sh: test_expect_success 'prune: do not prune detached HEAD with no reflog' '
> + test_expect_success 'prune: prune former HEAD after checking out branch' '
> head_oid=$(git rev-parse HEAD) &&
> git checkout --quiet main &&
> + git reflog expire --all &&
> git prune -v >prune_actual &&
> grep "$head_oid" prune_actual
> -
> + '
> 10: 3d3b733c3127 ! 13: 180847f4db14 test-lib: provide test prereq REFFILES
> @@ Commit message
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
>
> + ## t/README ##
> +@@ t/README: use these, and "test_set_prereq" for how to define your own.
> +
> + Git wasn't compiled with NO_PTHREADS=YesPlease.
> +
> ++ - REFFILES
> ++
> ++ Test is specific to packed/loose ref storage, and should be
> ++ disabled for other ref storage backends
> ++
> ++
> + Tips for Writing Tests
> + ----------------------
> +
> +
> ## t/test-lib.sh ##
> @@ t/test-lib.sh: parisc* | hppa*)
> ;;
> 11: dd1f6969c28d ! 14: f3307b62bfd7 t1407: require REFFILES for for_each_reflog test
> @@ Metadata
> ## Commit message ##
> t1407: require REFFILES for for_each_reflog test
>
> - It tries to setup a reflog by catting to .git/logs/
> + Add extensive comment why this test needs a REFFILES annotation.
> +
> + I tried forcing universal reflog creation with core.logAllRefUpdates=true, but
> + that apparently also doesn't cause reflogs to be created for pseudorefs
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
>
> @@ t/t1407-worktree-ref-store.sh: test_expect_success 'create_symref(FOO, refs/head
> '
>
> -test_expect_success 'for_each_reflog()' '
> ++# Some refs (refs/bisect/*, pseudorefs) are kept per worktree, so they should
> ++# only appear in the for-each-reflog output if it is called from the correct
> ++# worktree, which is exercised in this test. This test is poorly written (and
> ++# therefore marked REFFILES) for mulitple reasons: 1) it creates invalidly
> ++# formatted log entres. 2) it uses direct FS access for creating the reflogs. 3)
> ++# PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is
> ++# not testing a realistic scenario.
> +test_expect_success REFFILES 'for_each_reflog()' '
> echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
> mkdir -p .git/logs/refs/bisect &&
> 12: 86951eb39cb6 = 15: 0d3b18cd3542 t1414: mark corruption test with REFFILES
> 13: 1ce545043846 = 16: b64e3e7ade15 t2017: mark --orphan/logAllRefUpdates=false test as REFFILES
> 14: a3abc4f70e7d ! 17: fcc2b714dd50 t1404: mark tests that muck with .git directly as REFFILES.
> @@ Metadata
> ## Commit message ##
> t1404: mark tests that muck with .git directly as REFFILES.
>
> + The packed/loose ref storage is an overlay combination of packed-refs (refs and
> + tags in a single file) and one-file-per-ref. This creates all kinds of edge
> + cases related to directory/file conflicts, (non-)empty directories, and the
> + locking scheme, none of which applies to reftable.
> +
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
>
> ## t/t1404-update-ref-errors.sh ##
> 15: 2b3021c4ba62 ! 18: ff3b67c84c41 t7900: mark pack-refs tests as REFFILES
> @@ Metadata
> ## Commit message ##
> t7900: mark pack-refs tests as REFFILES
>
> + Reftable automatically compacts tables on writes. The functionality of
> + incremental compaction is unittested in reftable/stack_test.c
> + (test_reftable_stack_auto_compaction)
> +
> + In addition, pack-refs triggers a full compaction of the entire stack. This is
> + exercised in t0031-reftable.sh.
> +
> + Given that git-maintenance simply calls out git-pack-refs, it seems superfluous
> + to test this further for reftable.
> +
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
>
> ## t/t7900-maintenance.sh ##
> 16: a5b9439192db ! 19: 24dcf05d8fa6 t7003: check reflog existence only for REFFILES
> @@ t/t7003-filter-branch.sh: test_expect_success '--prune-empty is able to prune en
> git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire &&
> test_must_fail git rev-parse refs/heads/prune-entire &&
> - test_must_fail git reflog exists refs/heads/prune-entire
> -+ if test_have_prereq REFFILES ; then
> ++ if test_have_prereq REFFILES
> ++ then
> + test_must_fail git reflog exists refs/heads/prune-entire
> + fi
> '
> 17: a2cce772d44f = 20: a33cdfda74ff t4202: mark bogus head hash test with REFFILES
> 18: 0665edb1308b ! 21: d7e5de8dba46 t1415: set REFFILES for test specific to storage format
> @@ Metadata
> ## Commit message ##
> t1415: set REFFILES for test specific to storage format
>
> + Packing refs (and therefore checking that certain refs are not packed) is a
> + property of the packed/loose ref storage. Add a comment to explain what the test
> + checks.
> +
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
>
> ## t/t1415-worktree-refs.sh ##
> @@ t/t1415-worktree-refs.sh: test_expect_success 'setup' '
> '
>
> -test_expect_success 'refs/worktree must not be packed' '
> ++# The 'packed-refs' files is stored directly in .git/. This means it is global
> ++# to the repository, and can only contain refs that are shared across all
> ++# worktrees.
> +test_expect_success REFFILES 'refs/worktree must not be packed' '
> git pack-refs --all &&
> test_path_is_missing .git/refs/tags/wt1 &&
I looked this all over and left some nits, suspect/probably/definite bug
comments on specific patches, the ones with no comments from me LGTM.
I intentionally didn't review my own earlier feedback on v1 to look at
this with fresh eyes, I'd forgetten what points I raised, aside from the
general "let's not skip tests but test the new behavior" mantra I think
I either mentioned there or in related discussions.
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 00/21] Prepare tests for reftable backend
2021-05-20 16:28 ` [PATCH v2 00/21] Prepare tests for reftable backend Ævar Arnfjörð Bjarmason
@ 2021-05-26 9:23 ` Han-Wen Nienhuys
2021-05-26 9:52 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-26 9:23 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Thu, May 20, 2021 at 6:35 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> I looked this all over and left some nits, suspect/probably/definite bug
> comments on specific patches, the ones with no comments from me LGTM.
Awesome. Thanks for reviewing. I'll try to go over your comments this week.
> I intentionally didn't review my own earlier feedback on v1 to look at
> this with fresh eyes, I'd forgetten what points I raised, aside from the
> general "let's not skip tests but test the new behavior" mantra I think
> I either mentioned there or in related discussions.
"test new behavior" is troublesome, because it requires merging the
reftable support first, which in itself is a tough job, and predicated
on getting reviews for that code. This is why I split this series off,
because it can be merged early without impacting coverage of the
existing loose/packed backend.
How about I document more clearly, for each test marked REFFILES, what
is going on, and what should be done for coverage with reftable? We
can go over it as part of the reftable series. "REFFILES" will be an
easy term to grep for.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 00/21] Prepare tests for reftable backend
2021-05-26 9:23 ` Han-Wen Nienhuys
@ 2021-05-26 9:52 ` Ævar Arnfjörð Bjarmason
2021-05-31 14:37 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-26 9:52 UTC (permalink / raw)
To: Han-Wen Nienhuys; +Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys
On Wed, May 26 2021, Han-Wen Nienhuys wrote:
> On Thu, May 20, 2021 at 6:35 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>
>>
>> I looked this all over and left some nits, suspect/probably/definite bug
>> comments on specific patches, the ones with no comments from me LGTM.
>
> Awesome. Thanks for reviewing. I'll try to go over your comments this week.
>
>> I intentionally didn't review my own earlier feedback on v1 to look at
>> this with fresh eyes, I'd forgetten what points I raised, aside from the
>> general "let's not skip tests but test the new behavior" mantra I think
>> I either mentioned there or in related discussions.
>
>
> "test new behavior" is troublesome, because it requires merging the
> reftable support first, which in itself is a tough job, and predicated
> on getting reviews for that code. This is why I split this series off,
> because it can be merged early without impacting coverage of the
> existing loose/packed backend.
>
> How about I document more clearly, for each test marked REFFILES, what
> is going on, and what should be done for coverage with reftable? We
> can go over it as part of the reftable series. "REFFILES" will be an
> easy term to grep for.
Indeed, as I pointed out in some (particularly later patches) v2
commentary there's that chicken & egg problem.
Having re-skimmed my v2 reviews I think these would help:
A. Just leaning on the side of splitting up tests, e.g in my comment on
20/21[1], there's a test with file backend specific logic there, but
also a general assertion. Should just be two tests.
B. My suggestion in 15/21[2] to split up maybe make these two
prerequisites, i.e. REFFILES and REFTABLE.
One one hand it's a bit silly to have two prerequisites required for
the one reftable mode we don't have yet, and as long as we just have
two backends it'll be an informed guess at best.
But I'm leaning on the side of it being a good idea, to clearly
document those cases that are really files-backend.c specific, such
as low-level test to check that a .git/HEAD exists along with a "git
rev-parse HEAD" for that backend.
And then REFTABLE (and !REFTABLE) to test/skip things that are
specific to the behavior of reftable, but not in a way that has to
do with the "ref files" v.s. "a ref database" distinction. E.g. a
maintenance command calling pack-refs is skipped under REFTABLE, but
one could imagine us e.g. having a PostgreSQL backend where that
would be mapped to "VACUUM refs" or whatever. I.e. it's
testing/skipping based on reftable-specific features.
Even if strictly speaking we'd make no use of the distinction now in
that you could losslessly map REFFILES = !REFTABLE and REFTABLE =
!REFFILES.
Or maybe it's useless and we'd want to eventually do a second pass
over both REFFILES and REFTABLE to see if we need to change them to
positively assert the behavior of the "other" backend in both
cases. Just a thought...
1. https://lore.kernel.org/git/87eee1pfzk.fsf@evledraar.gmail.com/
2. http://lore.kernel.org/git/87pmxlpgb1.fsf@evledraar.gmail.com
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v2 00/21] Prepare tests for reftable backend
2021-05-26 9:52 ` Ævar Arnfjörð Bjarmason
@ 2021-05-31 14:37 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-05-31 14:37 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Han-Wen Nienhuys via GitGitGadget, git, Han-Wen Nienhuys,
Junio C Hamano
On Wed, May 26, 2021 at 12:07 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> >> I looked this all over and left some nits, suspect/probably/definite bug
> >> comments on specific patches, the ones with no comments from me LGTM.
> >
> > Awesome. Thanks for reviewing. I'll try to go over your comments this week.
I'm sending out the series shortly, with a Reviewed-By footer added to
the commits.
Jun, is this OK to schedule for 'next' ?
> >> I intentionally didn't review my own earlier feedback on v1 to look at
> >> this with fresh eyes, I'd forgetten what points I raised, aside from the
> >> general "let's not skip tests but test the new behavior" mantra I think
> >> I either mentioned there or in related discussions.
> >
> > "test new behavior" is troublesome, because it requires merging the
> > reftable support first, which in itself is a tough job, and predicated
> > on getting reviews for that code. This is why I split this series off,
> > because it can be merged early without impacting coverage of the
> > existing loose/packed backend.
> >
> > How about I document more clearly, for each test marked REFFILES, what
> > is going on, and what should be done for coverage with reftable? We
> > can go over it as part of the reftable series. "REFFILES" will be an
> > easy term to grep for.
>
> Indeed, as I pointed out in some (particularly later patches) v2
> commentary there's that chicken & egg problem.
>
> Having re-skimmed my v2 reviews I think these would help:
>
> A. Just leaning on the side of splitting up tests, e.g in my comment on
> 20/21[1], there's a test with file backend specific logic there, but
> also a general assertion. Should just be two tests.
I split a few tests.
> B. My suggestion in 15/21[2] to split up maybe make these two
> prerequisites, i.e. REFFILES and REFTABLE.
Until reftable is actually merged that will introduce a lot of dead
code in the tests. Besides requiring time for me to write that code,
it also introduces extra reviewer effort. I think that is best done as
part of the reftable series, when it is close to being merged.
> One one hand it's a bit silly to have two prerequisites required for
> the one reftable mode we don't have yet, and as long as we just have
> two backends it'll be an informed guess at best.
>
> But I'm leaning on the side of it being a good idea, to clearly
> document those cases that are really files-backend.c specific, such
> as low-level test to check that a .git/HEAD exists along with a "git
> rev-parse HEAD" for that backend.
>
> And then REFTABLE (and !REFTABLE) to test/skip things that are
> specific to the behavior of reftable, but not in a way that has to
> do with the "ref files" v.s. "a ref database" distinction. E.g. a
> maintenance command calling pack-refs is skipped under REFTABLE, but
> one could imagine us e.g. having a PostgreSQL backend where that
Let's first see the actual PostgreSQL backend materialize before
making investments to support it.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v3 00/22] Prepare tests for reftable backend
2021-04-27 10:38 ` [PATCH v2 00/21] Prepare tests for reftable backend Han-Wen Nienhuys via GitGitGadget
` (21 preceding siblings ...)
2021-05-20 16:28 ` [PATCH v2 00/21] Prepare tests for reftable backend Ævar Arnfjörð Bjarmason
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 01/22] t4202: split testcase for invalid HEAD symref and HEAD hash Han-Wen Nienhuys via GitGitGadget
` (22 more replies)
22 siblings, 23 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys
Rewrites some tests to avoid direct filesystem access.
Introduces the test prereq REFFILES to mark other tests that depend on
specifics of the files ref backend.
changes in v3 (relative to v2 from Apr 27)
* address avarab's feedback.
Han-Wen Nienhuys (22):
t4202: split testcase for invalid HEAD symref and HEAD hash
t/helper/ref-store: initialize oid in resolve-ref
t9300: check ref existence using test-helper rather than a file system
check
t5601: read HEAD using rev-parse
t1401: use tar to snapshot and restore repo state
t1401-symbolic-ref: avoid direct filesystem access
t1413: use tar to save and restore entire .git directory
t1301: fix typo in error message
t5000: reformat indentation to the latest fashion
t5000: inspect HEAD using git-rev-parse
t7003: use rev-parse rather than FS inspection
t5304: restyle: trim empty lines, drop ':' before >
t5304: use "reflog expire --all" to clear the reflog
test-lib: provide test prereq REFFILES
t1407: require REFFILES for for_each_reflog test
t1414: mark corruption test with REFFILES
t2017: mark --orphan/logAllRefUpdates=false test as REFFILES
t1404: mark tests that muck with .git directly as REFFILES.
t7900: stop checking for loose refs
t7003: check reflog existence only for REFFILES
t4202: mark bogus head hash test with REFFILES
t1415: set REFFILES for test specific to storage format
t/README | 6 ++
t/helper/test-ref-store.c | 2 +-
t/t1301-shared-repo.sh | 2 +-
t/t1401-symbolic-ref.sh | 25 ++++---
t/t1404-update-ref-errors.sh | 30 ++++-----
t/t1407-worktree-ref-store.sh | 9 ++-
t/t1413-reflog-detach.sh | 5 +-
t/t1414-reflog-walk.sh | 4 +-
t/t1415-worktree-refs.sh | 5 +-
t/t2017-checkout-orphan.sh | 2 +-
t/t4202-log.sh | 18 +++--
t/t5000-tar-tree.sh | 122 +++++++++++++++++++---------------
t/t5304-prune.sh | 83 +++++++----------------
t/t5601-clone.sh | 3 +-
t/t7003-filter-branch.sh | 7 +-
t/t7900-maintenance.sh | 2 -
t/t9300-fast-import.sh | 2 +-
t/test-lib.sh | 2 +
18 files changed, 170 insertions(+), 159 deletions(-)
base-commit: 4e42405f00ecbbee412846f48cb0253efeebe726
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1008%2Fhanwen%2Freffiles-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1008/hanwen/reffiles-v3
Pull-Request: https://github.com/git/git/pull/1008
Range-diff vs v2:
1: 8ad4a35cda70 ! 1: 6d875c6d7579 t4202: split testcase for invalid HEAD symref and HEAD hash
@@ t/t4202-log.sh: test_expect_success 'log --graph --no-walk is forbidden' '
-test_expect_success 'log diagnoses bogus HEAD' '
+test_expect_success 'log diagnoses bogus HEAD hash' '
git init empty &&
++ test_when_finished "rm -rf empty" &&
test_must_fail git -C empty log 2>stderr &&
test_i18ngrep does.not.have.any.commits stderr &&
echo 1234abcd >empty/.git/refs/heads/main &&
@@ t/t4202-log.sh: test_expect_success 'log --graph --no-walk is forbidden' '
+ test_i18ngrep broken stderr'
+
+test_expect_success 'log diagnoses bogus HEAD symref' '
-+ rm -rf empty &&
+ git init empty &&
+ git --git-dir empty/.git symbolic-ref HEAD refs/heads/invalid.lock &&
test_must_fail git -C empty log 2>stderr &&
2: e6222944a3eb ! 2: 7c6ef1dcadfc t/helper/ref-store: initialize oid in resolve-ref
@@ Commit message
provides a reliable mechanism for accessing REFNAME, while avoiding the implicit
resolution to refs/heads/REFNAME.
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
## t/helper/test-ref-store.c ##
@@ t/helper/test-ref-store.c: static int cmd_for_each_ref(struct ref_store *refs, c
static int cmd_resolve_ref(struct ref_store *refs, const char **argv)
{
- struct object_id oid;
-+ struct object_id oid = { 0 };
++ struct object_id oid = *null_oid();
const char *refname = notnull(*argv++, "refname");
int resolve_flags = arg_flags(*argv++, "resolve-flags");
int flags;
3: c5855b0caa73 ! 3: 130099d30aba t9300: check ref existence using test-helper rather than a file system check
@@ Commit message
t9300: check ref existence using test-helper rather than a file system check
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t9300-fast-import.sh ##
@@ t/t9300-fast-import.sh: test_expect_success 'B: accept branch name "TEMP_TAG"' '
4: 369c968ab837 ! 4: c898982255c1 t5601: read HEAD using rev-parse
@@ Commit message
t5601: read HEAD using rev-parse
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t5601-clone.sh ##
@@ t/t5601-clone.sh: test_expect_success 'clone from original with relative alternate' '
-: ------------ > 5: 12d43ff6a9e5 t1401: use tar to snapshot and restore repo state
5: 248d9ffe7927 ! 6: f05817d80565 t1401-symbolic-ref: avoid direct filesystem access
@@ Metadata
## Commit message ##
t1401-symbolic-ref: avoid direct filesystem access
+ Use symbolic-ref and rev-parse to inspect refs.
+
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t1401-symbolic-ref.sh ##
-@@
- test_description='basic symbolic-ref tests'
- . ./test-lib.sh
+@@ t/t1401-symbolic-ref.sh: test_expect_success 'setup' '
+ "$TAR" cf .git.tar .git/
+ '
--# If the tests munging HEAD fail, they can break detection of
--# the git repo, meaning that further tests will operate on
--# the surrounding git repo instead of the trash directory.
--reset_to_sane() {
-- echo ref: refs/heads/foo >.git/HEAD
--}
--
-test_expect_success 'symbolic-ref writes HEAD' '
-+test_expect_success 'setup' '
- git symbolic-ref HEAD refs/heads/foo &&
+- git symbolic-ref HEAD refs/heads/foo &&
- echo ref: refs/heads/foo >expect &&
- test_cmp expect .git/HEAD
-+ test_commit file &&
-+ "$TAR" cf .git.tar .git/
- '
-
+-'
+-
-test_expect_success 'symbolic-ref reads HEAD' '
- echo refs/heads/foo >expect &&
-+reset_to_sane() {
-+ rm -rf .git &&
-+ "$TAR" xf .git.tar
-+}
-+
+test_expect_success 'symbolic-ref read/write roundtrip' '
+ git symbolic-ref HEAD refs/heads/read-write-roundtrip &&
+ echo refs/heads/read-write-roundtrip >expect &&
6: e4e8fc1d4b4f ! 7: f6ab40c4e659 t1413: use tar to save and restore entire .git directory
@@ Commit message
This makes the test independent of the particulars of the storage formats.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t1413-reflog-detach.sh ##
@@ t/t1413-reflog-detach.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7: 89cc215c6014 ! 8: 852bc0f3055d t1301: fix typo in error message
@@ Commit message
t1301: fix typo in error message
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t1301-shared-repo.sh ##
@@ t/t1301-shared-repo.sh: test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
8: e67b90847c4e ! 9: e76c1e71bcb0 t5000: reformat indentation to the latest fashion
@@ Commit message
t5000: reformat indentation to the latest fashion
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t5000-tar-tree.sh ##
@@ t/t5000-tar-tree.sh: test_expect_success 'setup' '
@@ t/t5000-tar-tree.sh: test_expect_success 'setup' '
+test_expect_success 'populate workdir' '
+ mkdir a &&
+ echo simple textfile >a/a &&
-+ ten=0123456789 && hundred=$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten &&
-+ echo long filename >a/four$hundred &&
++ ten=0123456789 &&
++ hundred="$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten" &&
++ echo long filename >"a/four$hundred" &&
+ mkdir a/bin &&
+ test-tool genrandom "frotz" 500000 >a/bin/sh &&
+ printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
+ printf "A not substituted O" >a/substfile2 &&
-+ if test_have_prereq SYMLINKS; then
++ if test_have_prereq SYMLINKS
++ then
+ ln -s a a/l1
+ else
+ printf %s a >a/l1
+ fi &&
-+ (p=long_path_to_a_file && cd a &&
-+ for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
-+ echo text >file_with_long_path) &&
++ (
++ p=long_path_to_a_file &&
++ cd a &&
++ for depth in 1 2 3 4 5
++ do
++ mkdir $p &&
++ cd $p
++ done &&
++ echo text >file_with_long_path
++ ) &&
+ (cd a && find .) | sort >a.lst
+'
@@ t/t5000-tar-tree.sh: check_added with_untracked2 untracked one/untracked
- 'git archive in a bare repo' \
- '(cd bare.git && git archive HEAD) >b3.tar'
+test_expect_success 'git archive in a bare repo' '
-+ (cd bare.git && git archive HEAD) >b3.tar
++ git --git-dir bare.git archive HEAD >b3.tar
+'
-test_expect_success \
9: d6072a70ae7d ! 10: 3e748285876b t5000: inspect HEAD using git-rev-parse
@@ Commit message
t5000: inspect HEAD using git-rev-parse
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t5000-tar-tree.sh ##
@@ t/t5000-tar-tree.sh: test_expect_success 'validate file modification time' '
10: 4bf1bf16bca3 ! 11: a0605387d153 t7003: use rev-parse rather than FS inspection
@@ Commit message
t7003: use rev-parse rather than FS inspection
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t7003-filter-branch.sh ##
@@ t/t7003-filter-branch.sh: test_expect_success '--prune-empty is able to prune root commit' '
11: 6f15c15573af ! 12: c12406ac9655 t5304: restyle: trim empty lines, drop ':' before >
@@ Commit message
t5304: restyle: trim empty lines, drop ':' before >
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t5304-prune.sh ##
@@ t/t5304-prune.sh: add_blob() {
12: d8e80d83b6f8 ! 13: 9ede1b73d523 t5304: use "reflog expire --all" to clear the reflog
@@ Metadata
## Commit message ##
t5304: use "reflog expire --all" to clear the reflog
- This is more explicit, and reduces the depency between test functions. It also
- is more amenable to use with reftable, which has no concept of (non)existence of
- a reflog
+ This test checks that unreachable objects are really removed. For the test to
+ work, it has to ensure that no reflog retain any reachable objects.
+
+ Previously, it did this by manipulating the file system to remove reflog in the
+ first test, and relying on git not updating the reflog if the relevant logfile
+ doesn't exist in follow-up tests.
+
+ Now, explicitly clear the reflog using 'reflog expire'. This reduces the
+ dependency between test functions. It also is more amenable to use with
+ reftable, which has no concept of (non)-existence of a reflog
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t5304-prune.sh ##
@@ t/t5304-prune.sh: test_expect_success 'prune: prune nonsense parameters' '
13: 180847f4db14 ! 14: 8c552699fdbc test-lib: provide test prereq REFFILES
@@ Commit message
introducing the reftable storage backend.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/README ##
@@ t/README: use these, and "test_set_prereq" for how to define your own.
14: f3307b62bfd7 ! 15: 57fcd175fa72 t1407: require REFFILES for for_each_reflog test
@@ Commit message
that apparently also doesn't cause reflogs to be created for pseudorefs
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t1407-worktree-ref-store.sh ##
@@ t/t1407-worktree-ref-store.sh: test_expect_success 'create_symref(FOO, refs/heads/main)' '
15: 0d3b18cd3542 ! 16: 5fe2dc0efce9 t1414: mark corruption test with REFFILES
@@ Metadata
## Commit message ##
t1414: mark corruption test with REFFILES
- The reftable format guarantees that reflog entries are well-formed
+ The test checks what happens if reflog and ref database disagree on the state of
+ the latest commit. This seems to require accessing reflog storage directly.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t1414-reflog-walk.sh ##
@@ t/t1414-reflog-walk.sh: test_expect_success 'min/max age uses entry date to limit' '
@@ t/t1414-reflog-walk.sh: test_expect_success 'min/max age uses entry date to limi
'
-test_expect_success 'walk prefers reflog to ref tip' '
++# Create a situation where the reflog and ref database disagree about the latest
++# state of HEAD.
+test_expect_success REFFILES 'walk prefers reflog to ref tip' '
head=$(git rev-parse HEAD) &&
one=$(git rev-parse one) &&
16: b64e3e7ade15 ! 17: 496796d4e084 t2017: mark --orphan/logAllRefUpdates=false test as REFFILES
@@ Commit message
of the reflog file infeasible.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t2017-checkout-orphan.sh ##
@@ t/t2017-checkout-orphan.sh: test_expect_success '--orphan makes reflog by default' '
17: fcc2b714dd50 ! 18: c9d199b84499 t1404: mark tests that muck with .git directly as REFFILES.
@@ Commit message
locking scheme, none of which applies to reftable.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t1404-update-ref-errors.sh ##
@@ t/t1404-update-ref-errors.sh: test_expect_success 'one new ref is a simple prefix of another' '
18: ff3b67c84c41 < -: ------------ t7900: mark pack-refs tests as REFFILES
-: ------------ > 19: 6919c15e5f98 t7900: stop checking for loose refs
19: 24dcf05d8fa6 ! 20: 73f89faa3b0a t7003: check reflog existence only for REFFILES
@@ Commit message
t7003: check reflog existence only for REFFILES
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t7003-filter-branch.sh ##
@@ t/t7003-filter-branch.sh: test_expect_success '--prune-empty is able to prune entire branch' '
20: a33cdfda74ff ! 21: ff86cf916943 t4202: mark bogus head hash test with REFFILES
@@ Metadata
## Commit message ##
t4202: mark bogus head hash test with REFFILES
- In reftable, hashes are correctly formed by design
+ In reftable, hashes are correctly formed by design.
+
+ Split off test for git-log in empty repo.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t4202-log.sh ##
@@ t/t4202-log.sh: test_expect_success 'log --graph --no-walk is forbidden' '
@@ t/t4202-log.sh: test_expect_success 'log --graph --no-walk is forbidden' '
'
-test_expect_success 'log diagnoses bogus HEAD hash' '
-+test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
++test_expect_success 'log on empty repo fails' '
git init empty &&
+ test_when_finished "rm -rf empty" &&
+ test_must_fail git -C empty log 2>stderr &&
+- test_i18ngrep does.not.have.any.commits stderr &&
++ test_i18ngrep does.not.have.any.commits stderr
++'
++
++test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
++ git init empty &&
++ test_when_finished "rm -rf empty" &&
+ echo 1234abcd >empty/.git/refs/heads/main &&
test_must_fail git -C empty log 2>stderr &&
- test_i18ngrep does.not.have.any.commits stderr &&
+- test_i18ngrep broken stderr'
++ test_i18ngrep broken stderr
++'
+
+ test_expect_success 'log diagnoses bogus HEAD symref' '
+ git init empty &&
21: d7e5de8dba46 ! 22: cbcbb2d78fc9 t1415: set REFFILES for test specific to storage format
@@ Metadata
## Commit message ##
t1415: set REFFILES for test specific to storage format
- Packing refs (and therefore checking that certain refs are not packed) is a
- property of the packed/loose ref storage. Add a comment to explain what the test
- checks.
+ Packing refs (and therefore checking that certain refs are not packed)
+ is a property of the packed/loose ref storage. Add a comment to explain
+ what the test checks.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
+ Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## t/t1415-worktree-refs.sh ##
@@ t/t1415-worktree-refs.sh: test_expect_success 'setup' '
@@ t/t1415-worktree-refs.sh: test_expect_success 'setup' '
'
-test_expect_success 'refs/worktree must not be packed' '
-+# The 'packed-refs' files is stored directly in .git/. This means it is global
++# The 'packed-refs' file is stored directly in .git/. This means it is global
+# to the repository, and can only contain refs that are shared across all
+# worktrees.
+test_expect_success REFFILES 'refs/worktree must not be packed' '
--
gitgitgadget
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v3 01/22] t4202: split testcase for invalid HEAD symref and HEAD hash
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 02/22] t/helper/ref-store: initialize oid in resolve-ref Han-Wen Nienhuys via GitGitGadget
` (21 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Reftable will prohibit invalid hashes at the storage level, but
git-symbolic-ref can still create branches ending in ".lock".
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t4202-log.sh | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 350cfa35936a..327991fcea49 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1834,14 +1834,18 @@ test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
'
-test_expect_success 'log diagnoses bogus HEAD' '
+test_expect_success 'log diagnoses bogus HEAD hash' '
git init empty &&
+ test_when_finished "rm -rf empty" &&
test_must_fail git -C empty log 2>stderr &&
test_i18ngrep does.not.have.any.commits stderr &&
echo 1234abcd >empty/.git/refs/heads/main &&
test_must_fail git -C empty log 2>stderr &&
- test_i18ngrep broken stderr &&
- echo "ref: refs/heads/invalid.lock" >empty/.git/HEAD &&
+ test_i18ngrep broken stderr'
+
+test_expect_success 'log diagnoses bogus HEAD symref' '
+ git init empty &&
+ git --git-dir empty/.git symbolic-ref HEAD refs/heads/invalid.lock &&
test_must_fail git -C empty log 2>stderr &&
test_i18ngrep broken stderr &&
test_must_fail git -C empty log --default totally-bogus 2>stderr &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 02/22] t/helper/ref-store: initialize oid in resolve-ref
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 01/22] t4202: split testcase for invalid HEAD symref and HEAD hash Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 03/22] t9300: check ref existence using test-helper rather than a file system check Han-Wen Nienhuys via GitGitGadget
` (20 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
This will print $ZERO_OID when asking for a non-existent ref from the
test-helper.
Since resolve-ref provides direct access to refs_resolve_ref_unsafe(), it
provides a reliable mechanism for accessing REFNAME, while avoiding the implicit
resolution to refs/heads/REFNAME.
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/helper/test-ref-store.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index bba5f841c6ab..b314b81a45b2 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -118,7 +118,7 @@ static int cmd_for_each_ref(struct ref_store *refs, const char **argv)
static int cmd_resolve_ref(struct ref_store *refs, const char **argv)
{
- struct object_id oid;
+ struct object_id oid = *null_oid();
const char *refname = notnull(*argv++, "refname");
int resolve_flags = arg_flags(*argv++, "resolve-flags");
int flags;
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 03/22] t9300: check ref existence using test-helper rather than a file system check
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 01/22] t4202: split testcase for invalid HEAD symref and HEAD hash Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 02/22] t/helper/ref-store: initialize oid in resolve-ref Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 04/22] t5601: read HEAD using rev-parse Han-Wen Nienhuys via GitGitGadget
` (19 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t9300-fast-import.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 5c47ac4465cb..1aea943bef72 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -392,7 +392,7 @@ test_expect_success 'B: accept branch name "TEMP_TAG"' '
git gc
git prune" &&
git fast-import <input &&
- test -f .git/TEMP_TAG &&
+ test $(test-tool ref-store main resolve-ref TEMP_TAG 0 | cut -f1 -d " " ) != "$ZERO_OID" &&
test $(git rev-parse main) = $(git rev-parse TEMP_TAG^)
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 04/22] t5601: read HEAD using rev-parse
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (2 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 03/22] t9300: check ref existence using test-helper rather than a file system check Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 05/22] t1401: use tar to snapshot and restore repo state Han-Wen Nienhuys via GitGitGadget
` (18 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t5601-clone.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index c0688467e7c0..83c24fc97a7b 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -305,7 +305,8 @@ test_expect_success 'clone from original with relative alternate' '
test_expect_success 'clone checking out a tag' '
git clone --branch=some-tag src dst.tag &&
GIT_DIR=src/.git git rev-parse some-tag >expected &&
- test_cmp expected dst.tag/.git/HEAD &&
+ GIT_DIR=dst.tag/.git git rev-parse HEAD >actual &&
+ test_cmp expected actual &&
GIT_DIR=dst.tag/.git git config remote.origin.fetch >fetch.actual &&
echo "+refs/heads/*:refs/remotes/origin/*" >fetch.expected &&
test_cmp fetch.expected fetch.actual
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 05/22] t1401: use tar to snapshot and restore repo state
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (3 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 04/22] t5601: read HEAD using rev-parse Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 06/22] t1401-symbolic-ref: avoid direct filesystem access Han-Wen Nienhuys via GitGitGadget
` (17 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
This is agnostic to the ref storage format
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
t/t1401-symbolic-ref.sh | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index a4ebb0b65fec..7a9629fb9f0e 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -7,9 +7,16 @@ test_description='basic symbolic-ref tests'
# the git repo, meaning that further tests will operate on
# the surrounding git repo instead of the trash directory.
reset_to_sane() {
- echo ref: refs/heads/foo >.git/HEAD
+ rm -rf .git &&
+ "$TAR" xf .git.tar
}
+test_expect_success 'setup' '
+ git symbolic-ref HEAD refs/heads/foo &&
+ test_commit file &&
+ "$TAR" cf .git.tar .git/
+'
+
test_expect_success 'symbolic-ref writes HEAD' '
git symbolic-ref HEAD refs/heads/foo &&
echo ref: refs/heads/foo >expect &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 06/22] t1401-symbolic-ref: avoid direct filesystem access
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (4 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 05/22] t1401: use tar to snapshot and restore repo state Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 07/22] t1413: use tar to save and restore entire .git directory Han-Wen Nienhuys via GitGitGadget
` (16 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Use symbolic-ref and rev-parse to inspect refs.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1401-symbolic-ref.sh | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index 7a9629fb9f0e..132a1b885acb 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -17,14 +17,9 @@ test_expect_success 'setup' '
"$TAR" cf .git.tar .git/
'
-test_expect_success 'symbolic-ref writes HEAD' '
- git symbolic-ref HEAD refs/heads/foo &&
- echo ref: refs/heads/foo >expect &&
- test_cmp expect .git/HEAD
-'
-
-test_expect_success 'symbolic-ref reads HEAD' '
- echo refs/heads/foo >expect &&
+test_expect_success 'symbolic-ref read/write roundtrip' '
+ git symbolic-ref HEAD refs/heads/read-write-roundtrip &&
+ echo refs/heads/read-write-roundtrip >expect &&
git symbolic-ref HEAD >actual &&
test_cmp expect actual
'
@@ -32,12 +27,13 @@ test_expect_success 'symbolic-ref reads HEAD' '
test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
test_must_fail git symbolic-ref HEAD foo
'
+
reset_to_sane
test_expect_success 'symbolic-ref refuses bare sha1' '
- echo content >file && git add file && git commit -m one &&
test_must_fail git symbolic-ref HEAD $(git rev-parse HEAD)
'
+
reset_to_sane
test_expect_success 'HEAD cannot be removed' '
@@ -49,16 +45,16 @@ reset_to_sane
test_expect_success 'symbolic-ref can be deleted' '
git symbolic-ref NOTHEAD refs/heads/foo &&
git symbolic-ref -d NOTHEAD &&
- test_path_is_file .git/refs/heads/foo &&
- test_path_is_missing .git/NOTHEAD
+ git rev-parse refs/heads/foo &&
+ test_must_fail git symbolic-ref NOTHEAD
'
reset_to_sane
test_expect_success 'symbolic-ref can delete dangling symref' '
git symbolic-ref NOTHEAD refs/heads/missing &&
git symbolic-ref -d NOTHEAD &&
- test_path_is_missing .git/refs/heads/missing &&
- test_path_is_missing .git/NOTHEAD
+ test_must_fail git rev-parse refs/heads/missing &&
+ test_must_fail git symbolic-ref NOTHEAD
'
reset_to_sane
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 07/22] t1413: use tar to save and restore entire .git directory
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (5 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 06/22] t1401-symbolic-ref: avoid direct filesystem access Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-06-01 4:55 ` Bagas Sanjaya
2021-05-31 16:56 ` [PATCH v3 08/22] t1301: fix typo in error message Han-Wen Nienhuys via GitGitGadget
` (15 subsequent siblings)
22 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
This makes the test independent of the particulars of the storage formats.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1413-reflog-detach.sh | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh
index bde05208ae6a..934688a1ee82 100755
--- a/t/t1413-reflog-detach.sh
+++ b/t/t1413-reflog-detach.sh
@@ -7,8 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
reset_state () {
- git checkout main &&
- cp saved_reflog .git/logs/HEAD
+ rm -rf .git && "$TAR" xf .git-saved.tar
}
test_expect_success setup '
@@ -17,7 +16,7 @@ test_expect_success setup '
git branch side &&
test_tick &&
git commit --allow-empty -m second &&
- cat .git/logs/HEAD >saved_reflog
+ "$TAR" cf .git-saved.tar .git
'
test_expect_success baseline '
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v3 07/22] t1413: use tar to save and restore entire .git directory
2021-05-31 16:56 ` [PATCH v3 07/22] t1413: use tar to save and restore entire .git directory Han-Wen Nienhuys via GitGitGadget
@ 2021-06-01 4:55 ` Bagas Sanjaya
2021-06-01 9:23 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Bagas Sanjaya @ 2021-06-01 4:55 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget, git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys
Hi,
On 31/05/21 23.56, Han-Wen Nienhuys via GitGitGadget wrote:
> diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh
> index bde05208ae6a..934688a1ee82 100755
> --- a/t/t1413-reflog-detach.sh
> +++ b/t/t1413-reflog-detach.sh
> @@ -7,8 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> . ./test-lib.sh
>
> reset_state () {
> - git checkout main &&
> - cp saved_reflog .git/logs/HEAD
> + rm -rf .git && "$TAR" xf .git-saved.tar
> }
>
Why do you do rm -rf git directory then extract tar archive to reset?
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v3 07/22] t1413: use tar to save and restore entire .git directory
2021-06-01 4:55 ` Bagas Sanjaya
@ 2021-06-01 9:23 ` Han-Wen Nienhuys
2021-06-01 20:44 ` Junio C Hamano
0 siblings, 1 reply; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-06-01 9:23 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: Han-Wen Nienhuys via GitGitGadget, git, SZEDER Gábor,
Ævar Arnfjörð Bjarmason, Han-Wen Nienhuys
On Tue, Jun 1, 2021 at 6:55 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> On 31/05/21 23.56, Han-Wen Nienhuys via GitGitGadget wrote:
> > diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh
> > index bde05208ae6a..934688a1ee82 100755
> > --- a/t/t1413-reflog-detach.sh
> > +++ b/t/t1413-reflog-detach.sh
> > @@ -7,8 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> > . ./test-lib.sh
> >
> > reset_state () {
> > - git checkout main &&
> > - cp saved_reflog .git/logs/HEAD
> > + rm -rf .git && "$TAR" xf .git-saved.tar
> > }
> >
>
> Why do you do rm -rf git directory then extract tar archive to reset?
I'm not sure I understand your question. Are you asking why we have to
do a reset, or why we'd use rm + tar? The rm + tar restores the former
state reliably, so we can be sure it is correct. It's also independent
of the storage format details.
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v3 07/22] t1413: use tar to save and restore entire .git directory
2021-06-01 9:23 ` Han-Wen Nienhuys
@ 2021-06-01 20:44 ` Junio C Hamano
2021-06-02 7:49 ` Han-Wen Nienhuys
0 siblings, 1 reply; 138+ messages in thread
From: Junio C Hamano @ 2021-06-01 20:44 UTC (permalink / raw)
To: Han-Wen Nienhuys
Cc: Bagas Sanjaya, Han-Wen Nienhuys via GitGitGadget, git,
SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys
Han-Wen Nienhuys <hanwen@google.com> writes:
> On Tue, Jun 1, 2021 at 6:55 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>>
>> On 31/05/21 23.56, Han-Wen Nienhuys via GitGitGadget wrote:
>> > diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh
>> > index bde05208ae6a..934688a1ee82 100755
>> > --- a/t/t1413-reflog-detach.sh
>> > +++ b/t/t1413-reflog-detach.sh
>> > @@ -7,8 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>> > . ./test-lib.sh
>> >
>> > reset_state () {
>> > - git checkout main &&
>> > - cp saved_reflog .git/logs/HEAD
>> > + rm -rf .git && "$TAR" xf .git-saved.tar
>> > }
>> >
>>
>> Why do you do rm -rf git directory then extract tar archive to reset?
>
> I'm not sure I understand your question. Are you asking why we have to
> do a reset, or why we'd use rm + tar? The rm + tar restores the former
> state reliably, so we can be sure it is correct. It's also independent
> of the storage format details.
I think a short answer is "without rm -rf .git, a stale file in that
directory will stay there when .git-saved.tar gets extracted", but
the whole arrangement makes me worried what would happen if somebody
manages to interrupt "rm -rf" without killing the whole test
framework (or letting the when-finished handlers run). The test
framework thinks it is working in a throw-away repository but the
$TRASH_DIRECTORY that was supposed to be removed and extracted but
failed to do so due to interruption in the middle may not look like
a git repository, in which case it may try to do the usual repository
discovery and trash the git project repository instead.
^ permalink raw reply [flat|nested] 138+ messages in thread
* Re: [PATCH v3 07/22] t1413: use tar to save and restore entire .git directory
2021-06-01 20:44 ` Junio C Hamano
@ 2021-06-02 7:49 ` Han-Wen Nienhuys
0 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys @ 2021-06-02 7:49 UTC (permalink / raw)
To: Junio C Hamano
Cc: Bagas Sanjaya, Han-Wen Nienhuys via GitGitGadget, git,
SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys
On Tue, Jun 1, 2021 at 10:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Han-Wen Nienhuys <hanwen@google.com> writes:
>
> > On Tue, Jun 1, 2021 at 6:55 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> >>
> >> On 31/05/21 23.56, Han-Wen Nienhuys via GitGitGadget wrote:
> >> > diff --git a/t/t1413-reflog-detach.sh b/t/t1413-reflog-detach.sh
> >> > index bde05208ae6a..934688a1ee82 100755
> >> > --- a/t/t1413-reflog-detach.sh
> >> > +++ b/t/t1413-reflog-detach.sh
> >> > @@ -7,8 +7,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> >> > . ./test-lib.sh
> >> >
> >> > reset_state () {
> >> > - git checkout main &&
> >> > - cp saved_reflog .git/logs/HEAD
> >> > + rm -rf .git && "$TAR" xf .git-saved.tar
> >> > }
> >> >
> >>
> >> Why do you do rm -rf git directory then extract tar archive to reset?
> >
> > I'm not sure I understand your question. Are you asking why we have to
> > do a reset, or why we'd use rm + tar? The rm + tar restores the former
> > state reliably, so we can be sure it is correct. It's also independent
> > of the storage format details.
>
> I think a short answer is "without rm -rf .git, a stale file in that
> directory will stay there when .git-saved.tar gets extracted", but
> the whole arrangement makes me worried what would happen if somebody
> manages to interrupt "rm -rf" without killing the whole test
> framework (or letting the when-finished handlers run). The test
> framework thinks it is working in a throw-away repository but the
> $TRASH_DIRECTORY that was supposed to be removed and extracted but
> failed to do so due to interruption in the middle may not look like
> a git repository, in which case it may try to do the usual repository
> discovery and trash the git project repository instead.
Similar problems could occur if a developer is trying out a change to
the Git source code and makes an error. It's also easy to mess up the
check-out by doing a "cd .." too many in a shell test under
development.
I don't understand why the temp directories for tests are inside the
source tree. Every other project that I've worked on uses mktemp -d
for temporary directories instead.
For the problem you describe here, using atomic rename won't work,
because we want to swap out a complete directory. So we'd need a
command that replicates a source into a destination tree, using atomic
rename. Maybe rsync --delete might do the trick, but I don't think
we'd want that as a dependency?
--
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
^ permalink raw reply [flat|nested] 138+ messages in thread
* [PATCH v3 08/22] t1301: fix typo in error message
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (6 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 07/22] t1413: use tar to save and restore entire .git directory Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 09/22] t5000: reformat indentation to the latest fashion Han-Wen Nienhuys via GitGitGadget
` (14 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1301-shared-repo.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index ac947bff9fcf..84bf1970d8bf 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -124,7 +124,7 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
: happy
;;
*)
- echo Ooops, .git/logs/refs/heads/main is not 0662 [$actual]
+ echo Ooops, .git/logs/refs/heads/main is not 066x [$actual]
false
;;
esac
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 09/22] t5000: reformat indentation to the latest fashion
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (7 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 08/22] t1301: fix typo in error message Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 10/22] t5000: inspect HEAD using git-rev-parse Han-Wen Nienhuys via GitGitGadget
` (13 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t5000-tar-tree.sh | 113 ++++++++++++++++++++++++--------------------
1 file changed, 62 insertions(+), 51 deletions(-)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 7204799a0b52..8c5867b6c8ae 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -111,25 +111,34 @@ test_expect_success 'setup' '
EOF
'
-test_expect_success \
- 'populate workdir' \
- 'mkdir a &&
- echo simple textfile >a/a &&
- ten=0123456789 && hundred=$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten &&
- echo long filename >a/four$hundred &&
- mkdir a/bin &&
- test-tool genrandom "frotz" 500000 >a/bin/sh &&
- printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
- printf "A not substituted O" >a/substfile2 &&
- if test_have_prereq SYMLINKS; then
- ln -s a a/l1
- else
- printf %s a > a/l1
- fi &&
- (p=long_path_to_a_file && cd a &&
- for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&
- echo text >file_with_long_path) &&
- (cd a && find .) | sort >a.lst'
+test_expect_success 'populate workdir' '
+ mkdir a &&
+ echo simple textfile >a/a &&
+ ten=0123456789 &&
+ hundred="$ten$ten$ten$ten$ten$ten$ten$ten$ten$ten" &&
+ echo long filename >"a/four$hundred" &&
+ mkdir a/bin &&
+ test-tool genrandom "frotz" 500000 >a/bin/sh &&
+ printf "A\$Format:%s\$O" "$SUBSTFORMAT" >a/substfile1 &&
+ printf "A not substituted O" >a/substfile2 &&
+ if test_have_prereq SYMLINKS
+ then
+ ln -s a a/l1
+ else
+ printf %s a >a/l1
+ fi &&
+ (
+ p=long_path_to_a_file &&
+ cd a &&
+ for depth in 1 2 3 4 5
+ do
+ mkdir $p &&
+ cd $p
+ done &&
+ echo text >file_with_long_path
+ ) &&
+ (cd a && find .) | sort >a.lst
+'
test_expect_success \
'add ignored file' \
@@ -147,18 +156,18 @@ test_expect_success 'setup export-subst' '
>a/substfile1
'
-test_expect_success \
- 'create bare clone' \
- 'git clone --bare . bare.git &&
- cp .git/info/attributes bare.git/info/attributes'
+test_expect_success 'create bare clone' '
+ git clone --bare . bare.git &&
+ cp .git/info/attributes bare.git/info/attributes
+'
-test_expect_success \
- 'remove ignored file' \
- 'rm a/ignored'
+test_expect_success 'remove ignored file' '
+ rm a/ignored
+'
-test_expect_success \
- 'git archive' \
- 'git archive HEAD >b.tar'
+test_expect_success 'git archive' '
+ git archive HEAD >b.tar
+'
check_tar b
@@ -194,26 +203,28 @@ check_added with_untracked2 untracked one/untracked
check_added with_untracked2 untracked two/untracked
test_expect_success 'git archive on large files' '
- test_config core.bigfilethreshold 1 &&
- git archive HEAD >b3.tar &&
- test_cmp_bin b.tar b3.tar
+ test_config core.bigfilethreshold 1 &&
+ git archive HEAD >b3.tar &&
+ test_cmp_bin b.tar b3.tar
'
-test_expect_success \
- 'git archive in a bare repo' \
- '(cd bare.git && git archive HEAD) >b3.tar'
+test_expect_success 'git archive in a bare repo' '
+ git --git-dir bare.git archive HEAD >b3.tar
+'
-test_expect_success \
- 'git archive vs. the same in a bare repo' \
- 'test_cmp_bin b.tar b3.tar'
+test_expect_success 'git archive vs. the same in a bare repo' '
+ test_cmp_bin b.tar b3.tar
+'
-test_expect_success 'git archive with --output' \
- 'git archive --output=b4.tar HEAD &&
- test_cmp_bin b.tar b4.tar'
+test_expect_success 'git archive with --output' '
+ git archive --output=b4.tar HEAD &&
+ test_cmp_bin b.tar b4.tar
+'
-test_expect_success 'git archive --remote' \
- 'git archive --remote=. HEAD >b5.tar &&
- test_cmp_bin b.tar b5.tar'
+test_expect_success 'git archive --remote' '
+ git archive --remote=. HEAD >b5.tar &&
+ test_cmp_bin b.tar b5.tar
+'
test_expect_success 'git archive --remote with configured remote' '
git config remote.foo.url . &&
@@ -224,13 +235,13 @@ test_expect_success 'git archive --remote with configured remote' '
test_cmp_bin b.tar b5-nick.tar
'
-test_expect_success \
- 'validate file modification time' \
- 'mkdir extract &&
- "$TAR" xf b.tar -C extract a/a &&
- test-tool chmtime --get extract/a/a >b.mtime &&
- echo "1117231200" >expected.mtime &&
- test_cmp expected.mtime b.mtime'
+test_expect_success 'validate file modification time' '
+ mkdir extract &&
+ "$TAR" xf b.tar -C extract a/a &&
+ test-tool chmtime --get extract/a/a >b.mtime &&
+ echo "1117231200" >expected.mtime &&
+ test_cmp expected.mtime b.mtime
+'
test_expect_success \
'git get-tar-commit-id' \
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 10/22] t5000: inspect HEAD using git-rev-parse
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (8 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 09/22] t5000: reformat indentation to the latest fashion Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 11/22] t7003: use rev-parse rather than FS inspection Han-Wen Nienhuys via GitGitGadget
` (12 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t5000-tar-tree.sh | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 8c5867b6c8ae..2c88d1c15962 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -243,10 +243,11 @@ test_expect_success 'validate file modification time' '
test_cmp expected.mtime b.mtime
'
-test_expect_success \
- 'git get-tar-commit-id' \
- 'git get-tar-commit-id <b.tar >b.commitid &&
- test_cmp .git/$(git symbolic-ref HEAD) b.commitid'
+test_expect_success 'git get-tar-commit-id' '
+ git get-tar-commit-id <b.tar >actual &&
+ git rev-parse HEAD >expect &&
+ test_cmp expect actual
+'
test_expect_success 'git archive with --output, override inferred format' '
git archive --format=tar --output=d4.zip HEAD &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 11/22] t7003: use rev-parse rather than FS inspection
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (9 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 10/22] t5000: inspect HEAD using git-rev-parse Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 12/22] t5304: restyle: trim empty lines, drop ':' before > Han-Wen Nienhuys via GitGitGadget
` (11 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t7003-filter-branch.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 1349e5b2321c..cf30055c88dd 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -395,7 +395,7 @@ test_expect_success '--prune-empty is able to prune root commit' '
test_expect_success '--prune-empty is able to prune entire branch' '
git branch prune-entire B &&
git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire &&
- test_path_is_missing .git/refs/heads/prune-entire &&
+ test_must_fail git rev-parse refs/heads/prune-entire &&
test_must_fail git reflog exists refs/heads/prune-entire
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 12/22] t5304: restyle: trim empty lines, drop ':' before >
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (10 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 11/22] t7003: use rev-parse rather than FS inspection Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 13/22] t5304: use "reflog expire --all" to clear the reflog Han-Wen Nienhuys via GitGitGadget
` (10 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t5304-prune.sh | 74 +++++++++++++-----------------------------------
1 file changed, 20 insertions(+), 54 deletions(-)
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 3475b06aeb26..15d56d3d8791 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -22,30 +22,25 @@ add_blob() {
}
test_expect_success setup '
-
- : > file &&
+ >file &&
git add file &&
test_tick &&
git commit -m initial &&
git gc
-
'
test_expect_success 'prune stale packs' '
-
orig_pack=$(echo .git/objects/pack/*.pack) &&
- : > .git/objects/tmp_1.pack &&
- : > .git/objects/tmp_2.pack &&
+ >.git/objects/tmp_1.pack &&
+ >.git/objects/tmp_2.pack &&
test-tool chmtime =-86501 .git/objects/tmp_1.pack &&
git prune --expire 1.day &&
test_path_is_file $orig_pack &&
test_path_is_file .git/objects/tmp_2.pack &&
test_path_is_missing .git/objects/tmp_1.pack
-
'
test_expect_success 'prune --expire' '
-
add_blob &&
git prune --expire=1.hour.ago &&
verbose test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
@@ -54,11 +49,9 @@ test_expect_success 'prune --expire' '
git prune --expire 1.day &&
verbose test $before = $(git count-objects | sed "s/ .*//") &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc: implicit prune --expire' '
-
add_blob &&
test-tool chmtime =-$((2*$week-30)) $BLOB_FILE &&
git gc &&
@@ -68,33 +61,25 @@ test_expect_success 'gc: implicit prune --expire' '
git gc &&
verbose test $before = $(git count-objects | sed "s/ .*//") &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc: refuse to start with invalid gc.pruneExpire' '
-
git config gc.pruneExpire invalid &&
test_must_fail git gc
-
'
test_expect_success 'gc: start with ok gc.pruneExpire' '
-
git config gc.pruneExpire 2.days.ago &&
git gc
-
'
test_expect_success 'prune: prune nonsense parameters' '
-
test_must_fail git prune garbage &&
test_must_fail git prune --- &&
test_must_fail git prune --no-such-option
-
'
test_expect_success 'prune: prune unreachable heads' '
-
git config core.logAllRefUpdates false &&
mv .git/logs .git/logs.old &&
: > file2 &&
@@ -104,11 +89,9 @@ test_expect_success 'prune: prune unreachable heads' '
git reset HEAD^ &&
git prune &&
test_must_fail git reset $tmp_head --
-
'
test_expect_success 'prune: do not prune detached HEAD with no reflog' '
-
git checkout --detach --quiet &&
git commit --allow-empty -m "detached commit" &&
# verify that there is no reflogs
@@ -116,75 +99,61 @@ test_expect_success 'prune: do not prune detached HEAD with no reflog' '
test_path_is_missing .git/logs &&
git prune -n >prune_actual &&
test_must_be_empty prune_actual
-
'
test_expect_success 'prune: prune former HEAD after checking out branch' '
-
head_oid=$(git rev-parse HEAD) &&
git checkout --quiet main &&
git prune -v >prune_actual &&
grep "$head_oid" prune_actual
-
'
test_expect_success 'prune: do not prune heads listed as an argument' '
-
- : > file2 &&
+ >file2 &&
git add file2 &&
git commit -m temporary &&
tmp_head=$(git rev-list -1 HEAD) &&
git reset HEAD^ &&
git prune -- $tmp_head &&
git reset $tmp_head --
-
'
test_expect_success 'gc --no-prune' '
-
add_blob &&
test-tool chmtime =-$((5001*$day)) $BLOB_FILE &&
git config gc.pruneExpire 2.days.ago &&
git gc --no-prune &&
verbose test 1 = $(git count-objects | sed "s/ .*//") &&
test_path_is_file $BLOB_FILE
-
'
test_expect_success 'gc respects gc.pruneExpire' '
-
git config gc.pruneExpire 5002.days.ago &&
git gc &&
test_path_is_file $BLOB_FILE &&
git config gc.pruneExpire 5000.days.ago &&
git gc &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc --prune=<date>' '
-
add_blob &&
test-tool chmtime =-$((5001*$day)) $BLOB_FILE &&
git gc --prune=5002.days.ago &&
test_path_is_file $BLOB_FILE &&
git gc --prune=5000.days.ago &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc --prune=never' '
-
add_blob &&
git gc --prune=never &&
test_path_is_file $BLOB_FILE &&
git gc --prune=now &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc respects gc.pruneExpire=never' '
-
git config gc.pruneExpire never &&
add_blob &&
git gc &&
@@ -192,17 +161,14 @@ test_expect_success 'gc respects gc.pruneExpire=never' '
git config gc.pruneExpire now &&
git gc &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'prune --expire=never' '
-
add_blob &&
git prune --expire=never &&
test_path_is_file $BLOB_FILE &&
git prune &&
test_path_is_missing $BLOB_FILE
-
'
test_expect_success 'gc: prune old objects after local clone' '
@@ -222,16 +188,16 @@ test_expect_success 'gc: prune old objects after local clone' '
test_expect_success 'garbage report in count-objects -v' '
test_when_finished "rm -f .git/objects/pack/fake*" &&
test_when_finished "rm -f .git/objects/pack/foo*" &&
- : >.git/objects/pack/foo &&
- : >.git/objects/pack/foo.bar &&
- : >.git/objects/pack/foo.keep &&
- : >.git/objects/pack/foo.pack &&
- : >.git/objects/pack/fake.bar &&
- : >.git/objects/pack/fake.keep &&
- : >.git/objects/pack/fake.pack &&
- : >.git/objects/pack/fake.idx &&
- : >.git/objects/pack/fake2.keep &&
- : >.git/objects/pack/fake3.idx &&
+ >.git/objects/pack/foo &&
+ >.git/objects/pack/foo.bar &&
+ >.git/objects/pack/foo.keep &&
+ >.git/objects/pack/foo.pack &&
+ >.git/objects/pack/fake.bar &&
+ >.git/objects/pack/fake.keep &&
+ >.git/objects/pack/fake.pack &&
+ >.git/objects/pack/fake.idx &&
+ >.git/objects/pack/fake2.keep &&
+ >.git/objects/pack/fake3.idx &&
git count-objects -v 2>stderr &&
grep "index file .git/objects/pack/fake.idx is too small" stderr &&
grep "^warning:" stderr | sort >actual &&
@@ -250,12 +216,12 @@ EOF
test_expect_success 'clean pack garbage with gc' '
test_when_finished "rm -f .git/objects/pack/fake*" &&
test_when_finished "rm -f .git/objects/pack/foo*" &&
- : >.git/objects/pack/foo.keep &&
- : >.git/objects/pack/foo.pack &&
- : >.git/objects/pack/fake.idx &&
- : >.git/objects/pack/fake2.keep &&
- : >.git/objects/pack/fake2.idx &&
- : >.git/objects/pack/fake3.keep &&
+ >.git/objects/pack/foo.keep &&
+ >.git/objects/pack/foo.pack &&
+ >.git/objects/pack/fake.idx &&
+ >.git/objects/pack/fake2.keep &&
+ >.git/objects/pack/fake2.idx &&
+ >.git/objects/pack/fake3.keep &&
git gc &&
git count-objects -v 2>stderr &&
grep "^warning:" stderr | sort >actual &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 13/22] t5304: use "reflog expire --all" to clear the reflog
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (11 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 12/22] t5304: restyle: trim empty lines, drop ':' before > Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 14/22] test-lib: provide test prereq REFFILES Han-Wen Nienhuys via GitGitGadget
` (9 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
This test checks that unreachable objects are really removed. For the test to
work, it has to ensure that no reflog retain any reachable objects.
Previously, it did this by manipulating the file system to remove reflog in the
first test, and relying on git not updating the reflog if the relevant logfile
doesn't exist in follow-up tests.
Now, explicitly clear the reflog using 'reflog expire'. This reduces the
dependency between test functions. It also is more amenable to use with
reftable, which has no concept of (non)-existence of a reflog
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t5304-prune.sh | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 15d56d3d8791..7cabb85ca6e1 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -81,12 +81,12 @@ test_expect_success 'prune: prune nonsense parameters' '
test_expect_success 'prune: prune unreachable heads' '
git config core.logAllRefUpdates false &&
- mv .git/logs .git/logs.old &&
- : > file2 &&
+ >file2 &&
git add file2 &&
git commit -m temporary &&
tmp_head=$(git rev-list -1 HEAD) &&
git reset HEAD^ &&
+ git reflog expire --all &&
git prune &&
test_must_fail git reset $tmp_head --
'
@@ -94,9 +94,7 @@ test_expect_success 'prune: prune unreachable heads' '
test_expect_success 'prune: do not prune detached HEAD with no reflog' '
git checkout --detach --quiet &&
git commit --allow-empty -m "detached commit" &&
- # verify that there is no reflogs
- # (should be removed and disabled by previous test)
- test_path_is_missing .git/logs &&
+ git reflog expire --all &&
git prune -n >prune_actual &&
test_must_be_empty prune_actual
'
@@ -104,6 +102,7 @@ test_expect_success 'prune: do not prune detached HEAD with no reflog' '
test_expect_success 'prune: prune former HEAD after checking out branch' '
head_oid=$(git rev-parse HEAD) &&
git checkout --quiet main &&
+ git reflog expire --all &&
git prune -v >prune_actual &&
grep "$head_oid" prune_actual
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 14/22] test-lib: provide test prereq REFFILES
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (12 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 13/22] t5304: use "reflog expire --all" to clear the reflog Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 15/22] t1407: require REFFILES for for_each_reflog test Han-Wen Nienhuys via GitGitGadget
` (8 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
REFFILES can be used to mark tests that are specific to the packed/loose ref
storage format and its limitations. Marking such tests is a preparation for
introducing the reftable storage backend.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/README | 6 ++++++
t/test-lib.sh | 2 ++
2 files changed, 8 insertions(+)
diff --git a/t/README b/t/README
index 1a2072b2c8a2..9e7012230203 100644
--- a/t/README
+++ b/t/README
@@ -1126,6 +1126,12 @@ use these, and "test_set_prereq" for how to define your own.
Git wasn't compiled with NO_PTHREADS=YesPlease.
+ - REFFILES
+
+ Test is specific to packed/loose ref storage, and should be
+ disabled for other ref storage backends
+
+
Tips for Writing Tests
----------------------
diff --git a/t/test-lib.sh b/t/test-lib.sh
index adaf03543e81..76929a886fbf 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1488,6 +1488,8 @@ parisc* | hppa*)
;;
esac
+test_set_prereq REFFILES
+
( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
test -z "$NO_PERL" && test_set_prereq PERL
test -z "$NO_PTHREADS" && test_set_prereq PTHREADS
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 15/22] t1407: require REFFILES for for_each_reflog test
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (13 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 14/22] test-lib: provide test prereq REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 16/22] t1414: mark corruption test with REFFILES Han-Wen Nienhuys via GitGitGadget
` (7 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Add extensive comment why this test needs a REFFILES annotation.
I tried forcing universal reflog creation with core.logAllRefUpdates=true, but
that apparently also doesn't cause reflogs to be created for pseudorefs
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1407-worktree-ref-store.sh | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index d3fe77751122..ad8006c81397 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -52,7 +52,14 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' '
test_cmp expected actual
'
-test_expect_success 'for_each_reflog()' '
+# Some refs (refs/bisect/*, pseudorefs) are kept per worktree, so they should
+# only appear in the for-each-reflog output if it is called from the correct
+# worktree, which is exercised in this test. This test is poorly written (and
+# therefore marked REFFILES) for mulitple reasons: 1) it creates invalidly
+# formatted log entres. 2) it uses direct FS access for creating the reflogs. 3)
+# PSEUDO-WT and refs/bisect/random do not create reflogs by default, so it is
+# not testing a realistic scenario.
+test_expect_success REFFILES 'for_each_reflog()' '
echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
mkdir -p .git/logs/refs/bisect &&
echo $ZERO_OID > .git/logs/refs/bisect/random &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 16/22] t1414: mark corruption test with REFFILES
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (14 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 15/22] t1407: require REFFILES for for_each_reflog test Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 17/22] t2017: mark --orphan/logAllRefUpdates=false test as REFFILES Han-Wen Nienhuys via GitGitGadget
` (6 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
The test checks what happens if reflog and ref database disagree on the state of
the latest commit. This seems to require accessing reflog storage directly.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1414-reflog-walk.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/t/t1414-reflog-walk.sh b/t/t1414-reflog-walk.sh
index 80d94704d012..ea64cecf47bf 100755
--- a/t/t1414-reflog-walk.sh
+++ b/t/t1414-reflog-walk.sh
@@ -119,7 +119,9 @@ test_expect_success 'min/max age uses entry date to limit' '
test_cmp expect actual
'
-test_expect_success 'walk prefers reflog to ref tip' '
+# Create a situation where the reflog and ref database disagree about the latest
+# state of HEAD.
+test_expect_success REFFILES 'walk prefers reflog to ref tip' '
head=$(git rev-parse HEAD) &&
one=$(git rev-parse one) &&
ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 17/22] t2017: mark --orphan/logAllRefUpdates=false test as REFFILES
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (15 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 16/22] t1414: mark corruption test with REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 18/22] t1404: mark tests that muck with .git directly " Han-Wen Nienhuys via GitGitGadget
` (5 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
In reftable, there is no notion of a per-ref 'existence' of a reflog. Each
reflog entry has its own key, so it is not possible to distinguish between
{reflog doesn't exist,reflog exists but is empty}. This makes the logic
in log_ref_setup() (file refs/files-backend.c), which depends on the existence
of the reflog file infeasible.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t2017-checkout-orphan.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
index c7adbdd39ab9..88d6992a5e1f 100755
--- a/t/t2017-checkout-orphan.sh
+++ b/t/t2017-checkout-orphan.sh
@@ -76,7 +76,7 @@ test_expect_success '--orphan makes reflog by default' '
git rev-parse --verify delta@{0}
'
-test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '
+test_expect_success REFFILES '--orphan does not make reflog when core.logAllRefUpdates = false' '
git checkout main &&
git config core.logAllRefUpdates false &&
git checkout --orphan epsilon &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 18/22] t1404: mark tests that muck with .git directly as REFFILES.
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (16 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 17/22] t2017: mark --orphan/logAllRefUpdates=false test as REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 19/22] t7900: stop checking for loose refs Han-Wen Nienhuys via GitGitGadget
` (4 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
The packed/loose ref storage is an overlay combination of packed-refs (refs and
tags in a single file) and one-file-per-ref. This creates all kinds of edge
cases related to directory/file conflicts, (non-)empty directories, and the
locking scheme, none of which applies to reftable.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1404-update-ref-errors.sh | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index 8b51c4efc135..b729c1f48030 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -189,7 +189,7 @@ test_expect_success 'one new ref is a simple prefix of another' '
'
-test_expect_success 'empty directory should not fool rev-parse' '
+test_expect_success REFFILES 'empty directory should not fool rev-parse' '
prefix=refs/e-rev-parse &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -199,7 +199,7 @@ test_expect_success 'empty directory should not fool rev-parse' '
test_cmp expected actual
'
-test_expect_success 'empty directory should not fool for-each-ref' '
+test_expect_success REFFILES 'empty directory should not fool for-each-ref' '
prefix=refs/e-for-each-ref &&
git update-ref $prefix/foo $C &&
git for-each-ref $prefix >expected &&
@@ -209,14 +209,14 @@ test_expect_success 'empty directory should not fool for-each-ref' '
test_cmp expected actual
'
-test_expect_success 'empty directory should not fool create' '
+test_expect_success REFFILES 'empty directory should not fool create' '
prefix=refs/e-create &&
mkdir -p .git/$prefix/foo/bar/baz &&
printf "create %s $C\n" $prefix/foo |
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool verify' '
+test_expect_success REFFILES 'empty directory should not fool verify' '
prefix=refs/e-verify &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -225,7 +225,7 @@ test_expect_success 'empty directory should not fool verify' '
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool 1-arg update' '
+test_expect_success REFFILES 'empty directory should not fool 1-arg update' '
prefix=refs/e-update-1 &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -234,7 +234,7 @@ test_expect_success 'empty directory should not fool 1-arg update' '
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool 2-arg update' '
+test_expect_success REFFILES 'empty directory should not fool 2-arg update' '
prefix=refs/e-update-2 &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -243,7 +243,7 @@ test_expect_success 'empty directory should not fool 2-arg update' '
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool 0-arg delete' '
+test_expect_success REFFILES 'empty directory should not fool 0-arg delete' '
prefix=refs/e-delete-0 &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -252,7 +252,7 @@ test_expect_success 'empty directory should not fool 0-arg delete' '
git update-ref --stdin
'
-test_expect_success 'empty directory should not fool 1-arg delete' '
+test_expect_success REFFILES 'empty directory should not fool 1-arg delete' '
prefix=refs/e-delete-1 &&
git update-ref $prefix/foo $C &&
git pack-refs --all &&
@@ -466,7 +466,7 @@ test_expect_success 'incorrect old value blocks indirect no-deref delete' '
test_cmp expected output.err
'
-test_expect_success 'non-empty directory blocks create' '
+test_expect_success REFFILES 'non-empty directory blocks create' '
prefix=refs/ne-create &&
mkdir -p .git/$prefix/foo/bar &&
: >.git/$prefix/foo/bar/baz.lock &&
@@ -485,7 +485,7 @@ test_expect_success 'non-empty directory blocks create' '
test_cmp expected output.err
'
-test_expect_success 'broken reference blocks create' '
+test_expect_success REFFILES 'broken reference blocks create' '
prefix=refs/broken-create &&
mkdir -p .git/$prefix &&
echo "gobbledigook" >.git/$prefix/foo &&
@@ -504,7 +504,7 @@ test_expect_success 'broken reference blocks create' '
test_cmp expected output.err
'
-test_expect_success 'non-empty directory blocks indirect create' '
+test_expect_success REFFILES 'non-empty directory blocks indirect create' '
prefix=refs/ne-indirect-create &&
git symbolic-ref $prefix/symref $prefix/foo &&
mkdir -p .git/$prefix/foo/bar &&
@@ -524,7 +524,7 @@ test_expect_success 'non-empty directory blocks indirect create' '
test_cmp expected output.err
'
-test_expect_success 'broken reference blocks indirect create' '
+test_expect_success REFFILES 'broken reference blocks indirect create' '
prefix=refs/broken-indirect-create &&
git symbolic-ref $prefix/symref $prefix/foo &&
echo "gobbledigook" >.git/$prefix/foo &&
@@ -543,7 +543,7 @@ test_expect_success 'broken reference blocks indirect create' '
test_cmp expected output.err
'
-test_expect_success 'no bogus intermediate values during delete' '
+test_expect_success REFFILES 'no bogus intermediate values during delete' '
prefix=refs/slow-transaction &&
# Set up a reference with differing loose and packed versions:
git update-ref $prefix/foo $C &&
@@ -600,7 +600,7 @@ test_expect_success 'no bogus intermediate values during delete' '
test_must_fail git rev-parse --verify --quiet $prefix/foo
'
-test_expect_success 'delete fails cleanly if packed-refs file is locked' '
+test_expect_success REFFILES 'delete fails cleanly if packed-refs file is locked' '
prefix=refs/locked-packed-refs &&
# Set up a reference with differing loose and packed versions:
git update-ref $prefix/foo $C &&
@@ -616,7 +616,7 @@ test_expect_success 'delete fails cleanly if packed-refs file is locked' '
test_cmp unchanged actual
'
-test_expect_success 'delete fails cleanly if packed-refs.new write fails' '
+test_expect_success REFFILES 'delete fails cleanly if packed-refs.new write fails' '
# Setup and expectations are similar to the test above.
prefix=refs/failed-packed-refs &&
git update-ref $prefix/foo $C &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 19/22] t7900: stop checking for loose refs
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (17 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 18/22] t1404: mark tests that muck with .git directly " Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 20/22] t7003: check reflog existence only for REFFILES Han-Wen Nienhuys via GitGitGadget
` (3 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Given that git-maintenance simply calls out git-pack-refs, it seems superfluous
to test the functionality of pack-refs itself, as that is covered by
t3210-pack-refs.sh.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t7900-maintenance.sh | 2 --
1 file changed, 2 deletions(-)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index b93ae014ee58..58f46c77e666 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -356,8 +356,6 @@ test_expect_success 'pack-refs task' '
done &&
GIT_TRACE2_EVENT="$(pwd)/pack-refs.txt" \
git maintenance run --task=pack-refs &&
- ls .git/refs/heads/ >after &&
- test_must_be_empty after &&
test_subcommand git pack-refs --all --prune <pack-refs.txt
'
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 20/22] t7003: check reflog existence only for REFFILES
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (18 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 19/22] t7900: stop checking for loose refs Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 21/22] t4202: mark bogus head hash test with REFFILES Han-Wen Nienhuys via GitGitGadget
` (2 subsequent siblings)
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t7003-filter-branch.sh | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index cf30055c88dd..e18a21895238 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -396,7 +396,10 @@ test_expect_success '--prune-empty is able to prune entire branch' '
git branch prune-entire B &&
git filter-branch -f --prune-empty --index-filter "git update-index --remove A.t B.t" prune-entire &&
test_must_fail git rev-parse refs/heads/prune-entire &&
- test_must_fail git reflog exists refs/heads/prune-entire
+ if test_have_prereq REFFILES
+ then
+ test_must_fail git reflog exists refs/heads/prune-entire
+ fi
'
test_expect_success '--remap-to-ancestor with filename filters' '
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 21/22] t4202: mark bogus head hash test with REFFILES
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (19 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 20/22] t7003: check reflog existence only for REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 16:56 ` [PATCH v3 22/22] t1415: set REFFILES for test specific to storage format Han-Wen Nienhuys via GitGitGadget
2021-05-31 23:57 ` [PATCH v3 00/22] Prepare tests for reftable backend Ævar Arnfjörð Bjarmason
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
In reftable, hashes are correctly formed by design.
Split off test for git-log in empty repo.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t4202-log.sh | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 327991fcea49..39e746fbcbe2 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1834,14 +1834,20 @@ test_expect_success 'log --graph --no-walk is forbidden' '
test_must_fail git log --graph --no-walk
'
-test_expect_success 'log diagnoses bogus HEAD hash' '
+test_expect_success 'log on empty repo fails' '
git init empty &&
test_when_finished "rm -rf empty" &&
test_must_fail git -C empty log 2>stderr &&
- test_i18ngrep does.not.have.any.commits stderr &&
+ test_i18ngrep does.not.have.any.commits stderr
+'
+
+test_expect_success REFFILES 'log diagnoses bogus HEAD hash' '
+ git init empty &&
+ test_when_finished "rm -rf empty" &&
echo 1234abcd >empty/.git/refs/heads/main &&
test_must_fail git -C empty log 2>stderr &&
- test_i18ngrep broken stderr'
+ test_i18ngrep broken stderr
+'
test_expect_success 'log diagnoses bogus HEAD symref' '
git init empty &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* [PATCH v3 22/22] t1415: set REFFILES for test specific to storage format
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (20 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 21/22] t4202: mark bogus head hash test with REFFILES Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 16:56 ` Han-Wen Nienhuys via GitGitGadget
2021-05-31 23:57 ` [PATCH v3 00/22] Prepare tests for reftable backend Ævar Arnfjörð Bjarmason
22 siblings, 0 replies; 138+ messages in thread
From: Han-Wen Nienhuys via GitGitGadget @ 2021-05-31 16:56 UTC (permalink / raw)
To: git
Cc: SZEDER Gábor, Ævar Arnfjörð Bjarmason,
Han-Wen Nienhuys, Han-Wen Nienhuys, Han-Wen Nienhuys
From: Han-Wen Nienhuys <hanwen@google.com>
Packing refs (and therefore checking that certain refs are not packed)
is a property of the packed/loose ref storage. Add a comment to explain
what the test checks.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t1415-worktree-refs.sh | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 7ab91241ab7c..66f27d0fdfc9 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -16,7 +16,10 @@ test_expect_success 'setup' '
git -C wt2 update-ref refs/worktree/foo HEAD
'
-test_expect_success 'refs/worktree must not be packed' '
+# The 'packed-refs' file is stored directly in .git/. This means it is global
+# to the repository, and can only contain refs that are shared across all
+# worktrees.
+test_expect_success REFFILES 'refs/worktree must not be packed' '
git pack-refs --all &&
test_path_is_missing .git/refs/tags/wt1 &&
test_path_is_file .git/refs/worktree/foo &&
--
gitgitgadget
^ permalink raw reply related [flat|nested] 138+ messages in thread
* Re: [PATCH v3 00/22] Prepare tests for reftable backend
2021-05-31 16:56 ` [PATCH v3 00/22] " Han-Wen Nienhuys via GitGitGadget
` (21 preceding siblings ...)
2021-05-31 16:56 ` [PATCH v3 22/22] t1415: set REFFILES for test specific to storage format Han-Wen Nienhuys via GitGitGadget
@ 2021-05-31 23:57 ` Ævar Arnfjörð Bjarmason
22 siblings, 0 replies; 138+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-05-31 23:57 UTC (permalink / raw)
To: Han-Wen Nienhuys via GitGitGadget
Cc: git, SZEDER Gábor, Han-Wen Nienhuys, Han-Wen Nienhuys
On Mon, May 31 2021, Han-Wen Nienhuys via GitGitGadget wrote:
> Rewrites some tests to avoid direct filesystem access.
>
> Introduces the test prereq REFFILES to mark other tests that depend on
> specifics of the files ref backend.
>
> changes in v3 (relative to v2 from Apr 27)
>
> * address avarab's feedback.
I've looked this v3 over, and I think this is ready to go, and would
like to see this get merged down to next etc. Any minor quibbles I might
have are far outweighed by a desire to get this whole topic back on
track.
A bit more detail:
Per my feedback on v2 I'm not sure we're getting the right trade-off of
over skipping tests v.s. asserting new behavior, but as noted there
we've got the chicken & egg problem that we don't have reftable yet, so
we can't assert the new behavior, just discuss it in the abstract.
So our goal with this shouldn't be to get this perfect, but just get the
tests closer to not being file-backend specific.
Once we have this topic in "master" and a re-rolled reftable on top we
can always (and I plan to do this) see where exactly we end up differing
under reftable v.s. file backend.
Maybe that'll result in a different approach for a few tests being
changed here.
^ permalink raw reply [flat|nested] 138+ messages in thread