Re: [PATCH v5 0/1] upload-pack.c: fix filter spec quoting bug

Jacob Vosmaer [off-list ref] writes:

I updated the test based on the latest round of feedback.

Thanks, but this came a bit too late to wholesale replace the patch;
the previous round was good enough and is in 'next' already.

Looking at the change between two versions, I guess that the change
in tests may be worth salvaging, so perhaps you can make it into an
incremental "the test we added earlier was good, but let's make it
even better by doing X and Y" patch to be applied on top?

Thanks.

Jacob Vosmaer (1):
  upload-pack.c: fix filter spec quoting bug

 t/t5544-pack-objects-hook.sh | 10 ++++++++++
 upload-pack.c                |  9 +--------
 2 files changed, 11 insertions(+), 8 deletions(-)

Apply a few leftover improvements from the review of ad5df6b782
(upload-pack.c: fix filter spec quoting bug).

1. Instead of enumerating objects reachable from HEAD, enumerate all
reachable objects, because HEAD has not special significance in this
test.

2. Instead of relying on the knowledge that "? in rev-list output
means partial clone", explicitly verify that there are no blobs with
cat-file.
---
 t/t5544-pack-objects-hook.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t5544-pack-objects-hook.sh b/t/t5544-pack-objects-hook.sh
index f5ba663d64..dd5f44d986 100755
--- a/t/t5544-pack-objects-hook.sh
+++ b/t/t5544-pack-objects-hook.sh

@@ -64,8 +64,9 @@ test_expect_success 'hook works with partial clone' '
 	test_config_global uploadpack.packObjectsHook ./hook &&
 	test_config_global uploadpack.allowFilter true &&
 	git clone --bare --no-local --filter=blob:none . dst.git &&
-	git -C dst.git rev-list --objects --missing=print HEAD >objects &&
-	grep "^?" objects
+	git -C dst.git rev-list --objects --missing=allow-any --no-object-names --all >objects &&
+	git -C dst.git cat-file --batch-check="%(objecttype)" <objects >types &&
+	! grep blob types
 '
 
 test_done

-- 
2.30.0

On Tue, Feb 2, 2021 at 5:40 AM Jacob Vosmaer [off-list ref] wrote:

Apply a few leftover improvements from the review of ad5df6b782
(upload-pack.c: fix filter spec quoting bug).

1. Instead of enumerating objects reachable from HEAD, enumerate all
reachable objects, because HEAD has not special significance in this
test.

2. Instead of relying on the knowledge that "? in rev-list output
means partial clone", explicitly verify that there are no blobs with
cat-file.
---

Missing Signed-off-by:.

Apply a few leftover improvements from the review of ad5df6b782
(upload-pack.c: fix filter spec quoting bug).

1. Instead of enumerating objects reachable from HEAD, enumerate all
reachable objects, because HEAD has not special significance in this
test.

2. Instead of relying on the knowledge that "? in rev-list output
means partial clone", explicitly verify that there are no blobs with
cat-file.

Signed-off-by: Jacob Vosmaer <redacted>
---
 t/t5544-pack-objects-hook.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t5544-pack-objects-hook.sh b/t/t5544-pack-objects-hook.sh
index f5ba663d64..dd5f44d986 100755
--- a/t/t5544-pack-objects-hook.sh
+++ b/t/t5544-pack-objects-hook.sh

@@ -64,8 +64,9 @@ test_expect_success 'hook works with partial clone' '
 	test_config_global uploadpack.packObjectsHook ./hook &&
 	test_config_global uploadpack.allowFilter true &&
 	git clone --bare --no-local --filter=blob:none . dst.git &&
-	git -C dst.git rev-list --objects --missing=print HEAD >objects &&
-	grep "^?" objects
+	git -C dst.git rev-list --objects --missing=allow-any --no-object-names --all >objects &&
+	git -C dst.git cat-file --batch-check="%(objecttype)" <objects >types &&
+	! grep blob types
 '
 
 test_done

-- 
2.30.0