Git development
 help / color / mirror / Atom feed
* Re: [PATCH 00/12] Group reffiles tests
From: Junio C Hamano @ 2024-01-18  1:17 UTC (permalink / raw)
  To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This series groups REFFILES specific tests together. These tests are
> currently grouped together across the test suite based on functionality.
> However, since they exercise low-level behavior specific to the refs backend
> being used (in these cases, the ref-files backend), group them together
> based on which refs backend they test. This way, in the near future when the
> reftables backend gets upstreamed we can add tests that exercise the
> reftables backend close by in the t06xx area.
>
> These patches also remove the REFFILES prerequisite, since all the tests in
> t06xx are reffiles specific.

As we already have REFFILES lazy prereq, even _before_ we enable the
reftable backend, I think that we should start t0600 and t0602 with

	. ./test-lib.sh
	if ! test_have_prereq REFFILES
	then
		skip_all='skipping reffiles specific tests'
		test_done
	fi

which is more in line with the existing convention.  It is more
efficient than "forcing t0600 and t0602 to run always with reffiles"
when you have a CI job that uses reftable for all tests and another
CI job that uses reffiles for all tests.

> In the near future, once the reftable backend is upstreamed, all
> the tests in t06xx will be forced to run with the reffiles
> backend.

Presumably if there are reftable backend specific tests, they will
also be given names out of t06xx range, right?  And then they will
be skipped when the test is not using reftable as the default ref
backend, using the REFTABLE prerequisite in a similar way as shown
above for REFFILES, right?

Thanks.


^ permalink raw reply

* [Bug?] "git diff --no-rename A B"
From: Junio C Hamano @ 2024-01-18  1:07 UTC (permalink / raw)
  To: git

When the user misspells "--no-renames" as "--no-rename", it seems
that the rename detection (which is ont by default these days) still
kicks in, which means the misspelt option is silently ignored.
Should we show an error message instead?

^ permalink raw reply

* Re: [PATCH 03/12] t1414: convert test to use Git commands instead of writing refs manually
From: Junio C Hamano @ 2024-01-18  0:56 UTC (permalink / raw)
  To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <19233aa0d4496b66d67fbee82fb8d9b6b35a03cb.1705521155.git.gitgitgadget@gmail.com>

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  # 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' '
> +test_expect_success 'walk prefers reflog to ref tip' '
> +	test_commit A &&
> +	test_commit B &&
> +	git reflog delete HEAD@{0} &&
>  	head=$(git rev-parse HEAD) &&
> +	A=$(git rev-parse A) &&
>  
> +	echo $A >expect &&

You do not need an intermediate variable A, i.e.

	git rev-parse A >expect &&

would suffice.  Also it seems that $head is no longer used
because you do not manufacture a reflog entry yourself, so the two
assignments to $A and $head can be removed.

>  	git log -g --format=%H -1 >actual &&
>  	test_cmp expect actual
>  '

The resulting code makes the intent of the test much clearer.
Nicely done.

^ permalink raw reply

* Re: [PATCH 02/12] remove REFFILES prerequisite
From: Junio C Hamano @ 2024-01-18  0:46 UTC (permalink / raw)
  To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <624ad202305138c312e9db7d9cc590baf4e576ab.1705521155.git.gitgitgadget@gmail.com>

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> These tests are compatible with the reftable backend and thus do not
> need the REFFILES prerequisite.

May want to give a bit more backstory here?  After all, 53af25e4
(t1405: mark test that checks existence as REFFILES, 2022-01-31) and
53af25e4 (t1405: mark test that checks existence as REFFILES,
2022-01-31) marked these tests to require REFFILES and they explain
the reason for doing so was exactly because the reftable backend did
not have the notion of "the reflog for this ref exists" that is
independent from "the reflog for this ref exists and has one or more
reflog records".  If your work on the reftable backend during the
past few years added support for "already exists, but there is no
entry yet" state for reflogs, that would be great, but it would make
sense to explain why they suddenly have become "compatible with the
reftable backend".

Thanks.

>
> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/t1405-main-ref-store.sh  | 2 +-
>  t/t2017-checkout-orphan.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
> index e4627cf1b61..62c1eadb190 100755
> --- a/t/t1405-main-ref-store.sh
> +++ b/t/t1405-main-ref-store.sh
> @@ -112,7 +112,7 @@ test_expect_success 'delete_reflog(HEAD)' '
>  	test_must_fail git reflog exists HEAD
>  '
>  
> -test_expect_success REFFILES 'create-reflog(HEAD)' '
> +test_expect_success 'create-reflog(HEAD)' '
>  	$RUN create-reflog HEAD &&
>  	git reflog exists HEAD
>  '
> diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh
> index 947d1587ac8..a5c7358eeab 100755
> --- a/t/t2017-checkout-orphan.sh
> +++ b/t/t2017-checkout-orphan.sh
> @@ -86,7 +86,7 @@ test_expect_success '--orphan makes reflog by default' '
>  	git rev-parse --verify delta@{0}
>  '
>  
> -test_expect_success REFFILES '--orphan does not make reflog when core.logAllRefUpdates = false' '
> +test_expect_success '--orphan does not make reflog when core.logAllRefUpdates = false' '
>  	git checkout main &&
>  	git config core.logAllRefUpdates false &&
>  	git checkout --orphan epsilon &&

^ permalink raw reply

* Re: [PATCH 01/12] t3210: move to t0602
From: Junio C Hamano @ 2024-01-18  0:40 UTC (permalink / raw)
  To: John Cai via GitGitGadget; +Cc: git, John Cai
In-Reply-To: <0e2b6e197ab2fbfc81a42fd601b6aaf41e38929f.1705521155.git.gitgitgadget@gmail.com>

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: John Cai <johncai86@gmail.com>
>
> Move t3210 to t0602, since these tests are reffiles specific in that
> they modify loose refs manually. This is part of the effort to
> categorize these tests together based on the ref backend they test. When
> we upstream the reftable backend, we can add more tests to t06xx. This
> way, all tests that test specific ref backend behavior will be grouped
> together.

So, ... is the idea to have (1) majority of ref tests, against which
all backends ought to behave the same way, will be written in
backend agnostic way (e.g., we have seen some patches to stop
touching the filesystem .git/refs/ hierarchy manually), and (2) some
backend specific tests will be grouped in a small number of test
script files for each backend and they all will use t6xx numbrs?

OK.  Sounds like a good plan to me.




> Signed-off-by: John Cai <johncai86@gmail.com>
> ---
>  t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename t/{t3210-pack-refs.sh => t0602-reffiles-pack-refs.sh} (100%)
>
> diff --git a/t/t3210-pack-refs.sh b/t/t0602-reffiles-pack-refs.sh
> similarity index 100%
> rename from t/t3210-pack-refs.sh
> rename to t/t0602-reffiles-pack-refs.sh

^ permalink raw reply

* Re: [PATCH 1/1] builtin/show-ref: treat directory directory as non-existing in --exists
From: Junio C Hamano @ 2024-01-17 23:07 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Toon Claes, Eric Sunshine, git
In-Reply-To: <ZadzwA6vNnRPbKYh@tanuki>

Patrick Steinhardt <ps@pks.im> writes:

> With that in mind I think it is okay to already mention the new backend
> in commit messages -- at least I have been doing that, as well. Also,
> the tree already knows about the reftable backend because we have both
> the library and technical documentation in it for quite some time
> already.
>
> The patch itself looks good to me, thanks! Whether we want to reroll
> just to amend the commit message I'll leave to you and others to decide.

Yup.  The patch looks good.  Here is what I tentatively queued.

----- >8 -----
From: Toon Claes <toon@iotcl.com>
Date: Wed, 10 Jan 2024 15:15:59 +0100
Subject: [PATCH] builtin/show-ref: treat directory directory as non-existing
 in --exists

9080a7f178 (builtin/show-ref: add new mode to check for reference
existence, 2023-10-31) added the option --exists to git-show-ref(1).

When you use this option against a ref that doesn't exist, but it is
a parent directory of an existing ref, you get the following error:

    $ git show-ref --exists refs/heads
    error: failed to look up reference: Is a directory

when the ref-files backend is in use.  To be more clear to user,
hide the error about having found a directory.  What matters to the
user is that the named ref does not exist.  Instead, print the same
error as when the ref was not found:

    error: reference does not exist

Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/show-ref.c  | 2 +-
 t/t1403-show-ref.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 7aac525a87..6025ce223d 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -239,7 +239,7 @@ static int cmd_show_ref__exists(const char **refs)
 	if (refs_read_raw_ref(get_main_ref_store(the_repository), ref,
 			      &unused_oid, &unused_referent, &unused_type,
 			      &failure_errno)) {
-		if (failure_errno == ENOENT) {
+		if (failure_errno == ENOENT || failure_errno == EISDIR) {
 			error(_("reference does not exist"));
 			ret = 2;
 		} else {
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index b50ae6fcf1..a8055f7fe1 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -260,9 +260,9 @@ test_expect_success '--exists with non-commit object' '
 
 test_expect_success '--exists with directory fails with generic error' '
 	cat >expect <<-EOF &&
-	error: failed to look up reference: Is a directory
+	error: reference does not exist
 	EOF
-	test_expect_code 1 git show-ref --exists refs/heads 2>err &&
+	test_expect_code 2 git show-ref --exists refs/heads 2>err &&
 	test_cmp expect err
 '
 
-- 
2.43.0-367-g186b115d30


^ permalink raw reply related

* Re: [PATCH v2] rebase: Fix documentation about used shell in -x
From: Junio C Hamano @ 2024-01-17 22:38 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: git
In-Reply-To: <20240117085347.948960-1-nik.borisov@suse.com>

Nikolay Borisov <nik.borisov@suse.com> writes:

> The shell used when using the -x option is erroneously documented to be
> the one pointed to by the $SHELL environmental variable. This was true
> when rebase was implemented as a shell script but this is no longer
> true.
>
> Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
> ---

Ah, our mails crossed, and this is much better than my "tentative"
one.  Let me replace it with this one and queue.

Thanks.

>  Documentation/git-rebase.txt | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 25516c45d8b8..2cd55aedc0f9 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -963,10 +963,9 @@ The interactive rebase will stop when a command fails (i.e. exits with
>  non-0 status) to give you an opportunity to fix the problem. You can
>  continue with `git rebase --continue`.
>
> -The "exec" command launches the command in a shell (the one specified
> -in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
> -use shell features (like "cd", ">", ";" ...). The command is run from
> -the root of the working tree.
> +The "exec" command launches the command in a shell (the default one, usually
> +/bin/sh), so you can use shell features (like "cd", ">", ";" ...). The command
> +is run from the root of the working tree.
>
>  ----------------------------------
>  $ git rebase -i --exec "make test"
> --
> 2.34.1

^ permalink raw reply

* Re: [PATCH] rebase: Fix documentation about used shell in -x
From: Junio C Hamano @ 2024-01-17 22:35 UTC (permalink / raw)
  To: Kristoffer Haugsbakk, Nikolay Borisov, Jeff King; +Cc: git
In-Reply-To: <b491d954-b1a4-4000-95fb-fc83bf815edc@app.fastmail.com>

"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:

> Hi
>
> Some nitpicks since it seems like there will be another round (v2):
>
>> rebase: Fix documentation about used shell in -x
>
> Lower-case “fix” is more conventional.[1]
>
>> SHELL_PATH constant at build time. This erroneous statement in the
>> documentation sent me on a 10 minute wild goose chase wondering why my
>> $SHELL was pointing to /bin/bash and my /bin/sh to dash and git was
>> using dash and not bash.
>
> I think anecdotes are not kept in the commit message, usually? Often they
> are put after the three-hyphen/three-dash line.
> ...
>     The shell used when using the -x option is the one pointed to by the
>     SHELL_PATH constant at build time.
>
>     Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
>     ---
>       This erroneous statement in the documentation sent me on a 10 minute
>       wild goose chase wondering why my $SHELL was pointing to /bin/bash and
>       my /bin/sh to dash and git was using dash and not bash.
>
>      Documentation/git-rebase.txt | 2 +-
>      1 file changed, 1 insertion(+), 1 deletion(-)

Yup, that looks better.  Here is what I will queue tentatively, with
the improvement suggested by Peff.

----- >8 -----
Subject: [PATCH] rebase: fix documentation about used shell in -x

The shell used when using the -x option is the one pointed to by the
SHELL_PATH constant at build time, not $SHELL environment variable.

We could leave the parenthetical explanation about what shell is
used, but it depends on the build and platform (Windows do not even
use SHELL_PATH build-time knob).  Because Git executes lots of
things using a shell, and it always uses the default shell, it
probably is better to just stop at saying "launches the command in a
shell" without going into more details.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
Helped-by: Jeff King <peff@peff.net>
Helped-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * We could say things like

    - it is a possibility for the future to add how the default
      shell is decided (including the use of SHELL_PATH) in a more
      central part of the doucmentation like git(1)

    - at least not giving a wrong information would prevent the
      future developers wasting time on experimenting various
      settings of the $SHELL variable

   in the log message, if we want.

 Documentation/git-rebase.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index b4526ca246..51489ea686 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -957,8 +957,7 @@ The interactive rebase will stop when a command fails (i.e. exits with
 non-0 status) to give you an opportunity to fix the problem. You can
 continue with `git rebase --continue`.
 
-The "exec" command launches the command in a shell (the one specified
-in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
+The "exec" command launches the command in a shell, so you can
 use shell features (like "cd", ">", ";" ...). The command is run from
 the root of the working tree.
 

^ permalink raw reply related

* Re: [PATCH v3] tests: move t0009-prio-queue.sh to the new unit testing framework
From: Junio C Hamano @ 2024-01-17 22:15 UTC (permalink / raw)
  To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
In-Reply-To: <xmqqa5p3vczi.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> I forgot to examine the contents of the tests themselves.
> ...

FYI: taking them all together, here is what I tentatively queued on
top of what was posted as v3 before I start doing today's
integration cycle.

Thanks.

----- >8 -----
Subject: [PATCH] SQUASH???

---
 t/unit-tests/t-prio-queue.c | 57 ++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/t/unit-tests/t-prio-queue.c b/t/unit-tests/t-prio-queue.c
index 0b826b463e..3014a67ac2 100644
--- a/t/unit-tests/t-prio-queue.c
+++ b/t/unit-tests/t-prio-queue.c
@@ -22,44 +22,43 @@ static int show(int *v)
 static int test_prio_queue(int *input, int *result)
 {
 	struct prio_queue pq = { intcmp };
-	int i = 0;
+	int i, val;
 
-	while (*input) {
-		int *val = input++;
+	for (i = 0; (val = *input); input++) {
 		void *peek, *get;
-		switch(*val) {
-			case GET:
-				peek = prio_queue_peek(&pq);
+		switch (val) {
+		case GET:
+			peek = prio_queue_peek(&pq);
+			get = prio_queue_get(&pq);
+			if (peek != get)
+				BUG("peek and get results don't match");
+			result[i++] = show(get);
+			break;
+		case DUMP:
+			while ((peek = prio_queue_peek(&pq))) {
 				get = prio_queue_get(&pq);
 				if (peek != get)
 					BUG("peek and get results don't match");
 				result[i++] = show(get);
-				break;
-			case DUMP:
-				while ((peek = prio_queue_peek(&pq))) {
-					get = prio_queue_get(&pq);
-					if (peek != get)
-						BUG("peek and get results don't match");
-					result[i++] = show(get);
-				}
-				break;
-			case STACK:
-				pq.compare = NULL;
-				break;
-			case REVERSE:
-				prio_queue_reverse(&pq);
-				break;
-			default:
-				prio_queue_put(&pq, val);
-				break;
+			}
+			break;
+		case STACK:
+			pq.compare = NULL;
+			break;
+		case REVERSE:
+			prio_queue_reverse(&pq);
+			break;
+		default:
+			prio_queue_put(&pq, input);
+			break;
 		}
 	}
 	clear_prio_queue(&pq);
 	return 0;
 }
 
-#define BASIC_INPUT 1, 2, 3, 4, 5, 5, DUMP
-#define BASIC_EXPECTED 1, 2, 3, 4, 5, 5
+#define BASIC_INPUT 2, 6, 3, 10, 9, 5, 7, 4, 5, 8, 1, DUMP
+#define BASIC_EXPECTED 1, 2, 3, 4, 5, 5, 6, 7, 8, 9, 10
 
 #define MIXED_PUT_GET_INPUT 6, 2, 4, GET, 5, 3, GET, GET, 1, DUMP
 #define MIXED_PUT_GET_EXPECTED 2, 3, 4, 1, 5, 6
@@ -67,8 +66,8 @@ static int test_prio_queue(int *input, int *result)
 #define EMPTY_QUEUE_INPUT 1, 2, GET, GET, GET, 1, 2, GET, GET, GET
 #define EMPTY_QUEUE_EXPECTED 1, 2, MISSING, 1, 2, MISSING
 
-#define STACK_INPUT STACK, 1, 5, 4, 6, 2, 3, DUMP
-#define STACK_EXPECTED 3, 2, 6, 4, 5, 1
+#define STACK_INPUT STACK, 8, 1, 5, 4, 6, 2, 3, DUMP
+#define STACK_EXPECTED 3, 2, 6, 4, 5, 1, 8
 
 #define REVERSE_STACK_INPUT STACK, 1, 2, 3, 4, 5, 6, REVERSE, DUMP
 #define REVERSE_STACK_EXPECTED 1, 2, 3, 4, 5, 6
@@ -76,7 +75,7 @@ static int test_prio_queue(int *input, int *result)
 #define TEST_INPUT(INPUT, EXPECTED, name)			\
   static void test_##name(void)				\
 {								\
-	int input[] = {INPUT};					\
+	int input[] = {INPUT, 0};				\
 	int expected[] = {EXPECTED};				\
 	int result[ARRAY_SIZE(expected)];			\
 	test_prio_queue(input, result);				\
-- 
2.43.0-367-g186b115d30




^ permalink raw reply related

* Re: [PATCH] push: improve consistency of output when "up to date"
From: Dragan Simic @ 2024-01-17 22:02 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Benji Kay via GitGitGadget, git, Benji Kay
In-Reply-To: <ZaBhHSCT7EOgTo/N@nand.local>

On 2024-01-11 22:43, Taylor Blau wrote:
> On Thu, Jan 11, 2024 at 09:27:29PM +0000, Benji Kay via GitGitGadget 
> wrote:
>> From: okaybenji <okaybenji@gmail.com>
>> 
>> When one issues the pull command, one may see "Already up to date."
>> When issuing the push command, one may have seen "Everything 
>> up-to-date".
>> To improve consistency, "Everything up to date." is printed instead.
>> (The hyphens have been removed, and a period has been added.)
>> 
>> Signed-off-by: okaybenji <okaybenji@gmail.com>
>> ---
>>     push: improve consistency of output when "up to date"
>> 
>> Published-As: 
>> https://github.com/gitgitgadget/git/releases/tag/pr-1638%2Fokaybenji%2Fup-to-date-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
>> pr-1638/okaybenji/up-to-date-v1
>> Pull-Request: https://github.com/gitgitgadget/git/pull/1638
>> 
>>  transport.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/transport.c b/transport.c
>> index bd7899e9bf5..c42cb4e58b4 100644
>> --- a/transport.c
>> +++ b/transport.c
>> @@ -1467,7 +1467,7 @@ int transport_push(struct repository *r,
>>  	if (porcelain && !push_ret)
>>  		puts("Done");
>>  	else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
>> -		fprintf(stderr, "Everything up-to-date\n");
>> +		fprintf(stderr, "Everything up to date.\n");
> 
> Between the two, I have a vague preference towards "up-to-date", which
> would suggest changing the pull command's output to read "Already
> up-to-date". Personally I think that neither of them should include a
> period in their output, but whichever we decide should be done so
> consistently between the two.

I'm not a native English speaker, but I spent years contributing to 
English Wikipedia.  According to the manual of style employed by 
Wikipedia, which is based mainly on The Chicago Manual of Style, 
hyphenated forms should not be used at the ends of sentences, or at the 
ends of sentence-like forms.  In this case, we don't have complete 
sentences.

[1] https://www.chicagomanualofstyle.org/home.html

^ permalink raw reply

* Re: [PATCH v3] tests: move t0009-prio-queue.sh to the new unit testing framework
From: Junio C Hamano @ 2024-01-17 21:58 UTC (permalink / raw)
  To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
In-Reply-To: <pull.1642.v3.git.1705502304219.gitgitgadget@gmail.com>

I forgot to examine the contents of the tests themselves.

> -cat >expect <<'EOF'
> -1
> -2
> -3
> -4
> -5
> -5
> -6
> -7
> -8
> -9
> -10
> -EOF
> -test_expect_success 'basic ordering' '
> -	test-tool prio-queue 2 6 3 10 9 5 7 4 5 8 1 dump >actual &&
> -	test_cmp expect actual
> -'

This seems to have been lost from the converted test.  Your basic
input test feeds an already sorted array of 6 items and dump to see
they are the same already sorted array, which is a lot less
interesting than the above.

> -cat >expect <<'EOF'
> -2
> -3
> -4
> -1
> -5
> -6
> -EOF
> -test_expect_success 'mixed put and get' '
> -	test-tool prio-queue 6 2 4 get 5 3 get get 1 dump >actual &&
> -	test_cmp expect actual
> -'

This is a faithful conversion.

> -cat >expect <<'EOF'
> -1
> -2
> -NULL
> -1
> -2
> -NULL
> -EOF
> -test_expect_success 'notice empty queue' '
> -	test-tool prio-queue 1 2 get get get 1 2 get get get >actual &&
> -	test_cmp expect actual
> -'

This too.

> -cat >expect <<'EOF'
> -3
> -2
> -6
> -4
> -5
> -1
> -8
> -EOF
> -test_expect_success 'stack order' '
> -	test-tool prio-queue stack 8 1 5 4 6 2 3 dump >actual &&
> -	test_cmp expect actual
> -'

This test got truncated in your version, which is not horribly
wrong, but if we claim "move t0009 to unit testing", people would
expect to see a conversion faithful to the original.  And with the
use of result[ARRAY_SIZE(expected)], there is no reason to truncate
the original test with this version, no?

Thanks.

^ permalink raw reply

* Re: [PATCH v3] tests: move t0009-prio-queue.sh to the new unit testing framework
From: Junio C Hamano @ 2024-01-17 21:52 UTC (permalink / raw)
  To: Chandra Pratap via GitGitGadget; +Cc: git, Chandra Pratap, Chandra Pratap
In-Reply-To: <pull.1642.v3.git.1705502304219.gitgitgadget@gmail.com>

"Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +static int test_prio_queue(int *input, int *result)
> +{
> +	struct prio_queue pq = { intcmp };
> +	int i = 0;
> +
> +	while (*input) {
> +		int *val = input++;
> +		void *peek, *get;
> +		switch(*val) {

Ah, this one is a bit tricky.  I would have expected that we can
make val a pure integer, but because we need to give the address of
the element to be placed in the queue, not the value to be placed as
an element in the queue, to prio_queue_put(), we would need the
pointer.

I wonder if writing this as a more conventional for(;;) loop would
make it easier to handle, i.e.

	int val, i;

	for (i = 0; (val = *input); input++) {
		switch (val) {
		case ...:
		...
		default:
			prio_queue_put(&pq, input);
			break;
		}
	}

> +...
> +			default:
> +				prio_queue_put(&pq, val);
> +				break;
> +		}
> +	}
> +	clear_prio_queue(&pq);
> +	return 0;
> +}

> +#define TEST_INPUT(INPUT, EXPECTED, name)			\
> +  static void test_##name(void)				\
> +{								\
> +	int input[] = {INPUT};					\

I think I missed this myself in my review the last round, but we
need the sentinel value 0 at the end, i.e.,

	int input[] = {INPUT, 0};

because the test_prio_queue() loops "while (*input)".  Otherwise
the loop would not terminate.

> +	int expected[] = {EXPECTED};				\
> +	int result[ARRAY_SIZE(expected)];			\

These arrays are correct.

> +	test_prio_queue(input, result);				\
> +	check(!memcmp(expected, result, sizeof(expected)));	\

So is this check.

> +}
> +
> +TEST_INPUT(BASIC_INPUT, BASIC_EXPECTED, basic)
> +TEST_INPUT(MIXED_PUT_GET_INPUT, MIXED_PUT_GET_EXPECTED, mixed)
> +TEST_INPUT(EMPTY_QUEUE_INPUT, EMPTY_QUEUE_EXPECTED, empty)
> +TEST_INPUT(STACK_INPUT, STACK_EXPECTED, stack)
> +TEST_INPUT(REVERSE_STACK_INPUT, REVERSE_STACK_EXPECTED, reverse)
> +
> +int cmd_main(int argc, const char **argv)
> +{
> +	TEST(test_basic(), "prio-queue works for basic input");
> +	TEST(test_mixed(), "prio-queue works for mixed put & get commands");
> +	TEST(test_empty(), "prio-queue works when queue is empty");
> +	TEST(test_stack(), "prio-queue works when used as a LIFO stack");
> +	TEST(test_reverse(), "prio-queue works when LIFO stack is reversed");
> +
> +	return test_done();
> +}

Other than that, looking good.  Thanks.

^ permalink raw reply

* Re: [PATCH v6 0/2] t7501: add tests for --include, --only, --signoff
From: Junio C Hamano @ 2024-01-17 21:33 UTC (permalink / raw)
  To: Ghanshyam Thakkar; +Cc: git, phillip.wood123, christian.couder
In-Reply-To: <20240117161421.17333-1-shyamthakkar001@gmail.com>

Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:

> The v6 addresses the comments from Junio, which suggested to improve the
> TODO comment describing the potential bug for --include. Also, replace
> here-doc with expected output file for better debugging, as suggested by
> Junio.
>
> --signoff tests remain unchanged.
>
> Ghanshyam Thakkar (2):
>   t7501: add tests for --include and --only
>   t7501: add tests for --amend --signoff
>
>  t/t7501-commit-basic-functionality.sh | 98 ++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 2 deletions(-)

Looks quite good.  Let's declare victory and merge it down to 'next'
soonish.

Thanks.

^ permalink raw reply

* Re: [DISCUSS] Introducing Rust into the Git project
From: Dragan Simic @ 2024-01-17 21:30 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Taylor Blau, git
In-Reply-To: <CABPp-BFWsWCGogqQ=haMsS4OhOdSwc3frcAxa6soQR5ORTceOA@mail.gmail.com>

On 2024-01-11 17:57, Elijah Newren wrote:
> Hi Dragan,

I apologize for my delayed response.

> On Wed, Jan 10, 2024 at 9:39 PM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> 
>> On 2024-01-11 01:33, Elijah Newren wrote:
>> > On Wed, Jan 10, 2024 at 1:57 PM Dragan Simic <dsimic@manjaro.org>
>> > wrote:
>> >>
>> >> Thus, Git should probably follow the same approach of not converting
>> >> the
>> >> already existing code
>> >
>> > I disagree with this.  I saw significant performance improvements
>> > through converting some existing Git code to Rust.  Granted, it was
>> > only a small amount of code, but the performance benefits I saw
>> > suggested we'd see more by also doing similar conversions elsewhere.
>> > (Note that I kept the old C code and then conditionally compiled
>> > either Rust or C versions of what I was converting.)
>> 
>> Well, it's also possible that improving the old C code could also 
>> result
>> in some performance improvements.  Thus, quite frankly, I don't see 
>> that
>> as a valid argument to rewrite some existing C code in Rust.
> 
> Yes, and I've made many performance improvements in the C code in git.
> Sometimes I make some of the code 5% or 20% faster.  Sometimes 1-3
> orders of magnitude faster.  Once over 60 orders of magnitude
> faster.[1]  Look around in git's history; I've done a fair amount of
> performance stuff.

Thank you very much for your work!

> And I'm specifically arguing that I feel limited in some of the
> performance work that can be done by remaining in C.  Part of my
> reason for interest in Rust is exactly because I think it can help us
> improve performance in ways that are far more difficult to achieve in
> C.  And this isn't just guesswork, I've done some trials with it.
> Further, I even took the time to document some of these reasons
> elsewhere in this thread[2].  Arguing that some performance
> improvements can be done in C is thus entirely missing the point.
> 
> If you want to dismiss the performance angle of argument for Rust, you
> should take the time to address the actual reasons raised for why it
> could make it easier to improve performance relative to continuing in
> C.
> 
> Also, as a heads up since you seem to be relatively new to the list:
> your position will probably carry more weight with others if you take
> the time to understand, acknowledge, and/or address counterpoints of
> the other party.  It is certainly fine to simply express some concerns
> without doing so (Randall and Patrick did a good job of this in this
> thread), but when you simply assert that the benefits others point out
> simply don't exist (e.g. your "Quite frankly, that would _only_
> complicate things and cause fragmentation." (emphasis added) from your
> first email in this thread[3], and which this latest email of yours
> somewhat looks like as well), others may well start applying a
> discount to any positions you state.  Granted, it's totally up to you,
> but I'm just giving a hint about how I think you might be able to be
> more persuasive.

I totally agree with your suggestions, and I'm thankful for the time it 
took you to write it all down.  I'll take your advice and refrain myself 
from expressing my opinions in this thread.

> [1] A couple examples: 6a5fb966720 ("Change default merge backend from
> recursive to ort", 2021-08-04) and 8d92fb29270 ("dir: replace
> exponential algorithm with a linear one", 2020-04-01)
> [2] Footnote 6 of
> https://lore.kernel.org/git/CABPp-BFOmwV-xBtjvtenb6RFz9wx2VWVpTeho0k=D8wsCCVwqQ@mail.gmail.com/
> [3] 
> https://lore.kernel.org/git/b2651b38a4f7edaf1c5ffee72af00e46@manjaro.org/

^ permalink raw reply

* Re: [PATCH 3/3] submodule-config.c: strengthen URL fsck check
From: Victoria Dye @ 2024-01-17 21:19 UTC (permalink / raw)
  To: Patrick Steinhardt, Victoria Dye via GitGitGadget; +Cc: git
In-Reply-To: <ZZ46MrjSocJl-kpU@tanuki>

Patrick Steinhardt wrote:
> On Tue, Jan 09, 2024 at 05:53:37PM +0000, Victoria Dye via GitGitGadget wrote:
>> From: Victoria Dye <vdye@github.com>
>>
>> Update the validation of "curl URL" submodule URLs (i.e. those that specify
>> an "http[s]" or "ftp[s]" protocol) in 'check_submodule_url()' to catch more
>> invalid URLs. The existing validation using 'credential_from_url_gently()'
>> parses certain URLs incorrectly, leading to invalid submodule URLs passing
>> 'git fsck' checks. Conversely, 'url_normalize()' - used to validate remote
>> URLs in 'remote_get()' - correctly identifies the invalid URLs missed by
>> 'credential_from_url_gently()'.
> 
> Okay, so we retain the wrong behavior of `credential_from_url_gently()`,
> right? I wonder whether this can be abused in any way, doubly so because
> the function gets invoked with untrusted input from the remote server
> when we handle redirects in `http_request_reauth()`. But the redirect
> URL we end up passing to `credential_from_url_gently()` would have to
> contain a non-numeric port, and curl seemingly does not know to handle
> those either.

Correct, nothing about 'credential_from_url_gently()' changes here. As for
whether it could be abused - I don't *think* so, but I'm definitely not a
security expert. If it helps, here's a more detailed breakdown of the issue:

In 'credential_from_url_1()', suppose we have URL
"http://example.com:test/repo.git". Stepping through the variables:

- 'cp' is "example.com:test/repo.git"
- 'at' is NULL
- 'colon' is ":test/repo.git"
- 'slash' is "/repo.git"

Because 'at' is NULL, we set 'host = cp'. Later, because 'slash - host > 0',
we call 'url_decode_mem()' on "example.com:test" (which, in this case,
doesn't change anything) and the result 'host' to "example.com:test".

The issue for the fsck check is that 'credential_from_url_gently()' doesn't
validate the hostname it extracts (e.g. whether ':' precedes a valid port,
or if the hostname contains a '%'-escaped sequence). I don't *think* that
could be abused since, like you said, cURL should just reject the invalid
URL altogether.

> 
> Other callsites include fsck (which you're fixing) and the credential
> store (which is entirely user-controlled). It would be great regardless
> to fix the underlying bug in `credential_from_url_gently()` eventually
> though. But I do not think that this has to be part this patch series
> here, which is a strict improvement.

Agreed! I think normalizing the URL before trying to extract the credentials
may be all that's needed to avoid surprise URL errors, but that probably
warrants a separate patch submission (with appropriately thorough testing).

> 
> Thanks!
> 
> Patrick


^ permalink raw reply

* [PATCH 12/12] t5312: move reffiles specific tests to t0600
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

Move a few tests into t0600 since they specifically test the packed-refs
file and thus are specific to the reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 30 ++++++++++++++++++++++++++++++
 t/t5312-prune-corruption.sh | 26 --------------------------
 2 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index c88576dfea5..190155f592d 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -571,4 +571,34 @@ test_expect_success 'log diagnoses bogus HEAD symref' '
 	test_grep broken stderr
 '
 
+# we do not want to count on running pack-refs to
+# actually pack it, as it is perfectly reasonable to
+# skip processing a broken ref
+test_expect_success 'create packed-refs file with broken ref' '
+	test_tick && git commit --allow-empty -m one &&
+	recoverable=$(git rev-parse HEAD) &&
+	test_tick && git commit --allow-empty -m two &&
+	missing=$(git rev-parse HEAD) &&
+	rm -f .git/refs/heads/main &&
+	cat >.git/packed-refs <<-EOF &&
+	$missing refs/heads/main
+	$recoverable refs/heads/other
+	EOF
+	echo $missing >expect &&
+	git rev-parse refs/heads/main >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'pack-refs does not silently delete broken packed ref' '
+	git pack-refs --all --prune &&
+	git rev-parse refs/heads/main >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success  'pack-refs does not drop broken refs during deletion' '
+	git update-ref -d refs/heads/other &&
+	git rev-parse refs/heads/main >actual &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh
index 230cb387122..d8d2e304687 100755
--- a/t/t5312-prune-corruption.sh
+++ b/t/t5312-prune-corruption.sh
@@ -111,30 +111,4 @@ test_expect_success 'pack-refs does not silently delete broken loose ref' '
 	test_cmp expect actual
 '
 
-# we do not want to count on running pack-refs to
-# actually pack it, as it is perfectly reasonable to
-# skip processing a broken ref
-test_expect_success REFFILES 'create packed-refs file with broken ref' '
-	rm -f .git/refs/heads/main &&
-	cat >.git/packed-refs <<-EOF &&
-	$missing refs/heads/main
-	$recoverable refs/heads/other
-	EOF
-	echo $missing >expect &&
-	git rev-parse refs/heads/main >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success REFFILES 'pack-refs does not silently delete broken packed ref' '
-	git pack-refs --all --prune &&
-	git rev-parse refs/heads/main >actual &&
-	test_cmp expect actual
-'
-
-test_expect_success REFFILES  'pack-refs does not drop broken refs during deletion' '
-	git update-ref -d refs/heads/other &&
-	git rev-parse refs/heads/main >actual &&
-	test_cmp expect actual
-'
-
 test_done
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH 11/12] t4202: move reffiles specific tests to t0600
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

Move two tests into t0600 since they write loose reflog refs manually
and thus are specific to the reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 17 +++++++++++++++++
 t/t4202-log.sh              | 17 -----------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index bee61b2d19d..c88576dfea5 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -554,4 +554,21 @@ test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
 	test_cmp expect actual
 '
 
+test_expect_success '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_grep broken stderr
+'
+
+test_expect_success 'log diagnoses bogus HEAD symref' '
+	git init empty &&
+	test-tool -C empty ref-store main create-symref HEAD refs/heads/invalid.lock &&
+	test_must_fail git -C empty log 2>stderr &&
+	test_grep broken stderr &&
+	test_must_fail git -C empty log --default totally-bogus 2>stderr &&
+	test_grep broken stderr
+'
+
 test_done
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index ddd205f98ab..60fe60d7610 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -2255,23 +2255,6 @@ test_expect_success 'log on empty repo fails' '
 	test_grep 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_grep broken stderr
-'
-
-test_expect_success REFFILES 'log diagnoses bogus HEAD symref' '
-	git init empty &&
-	test-tool -C empty ref-store main create-symref HEAD refs/heads/invalid.lock &&
-	test_must_fail git -C empty log 2>stderr &&
-	test_grep broken stderr &&
-	test_must_fail git -C empty log --default totally-bogus 2>stderr &&
-	test_grep broken stderr
-'
-
 test_expect_success 'log does not default to HEAD when rev input is given' '
 	git log --branches=does-not-exist >actual &&
 	test_must_be_empty actual
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 10/12] t3903: move reffiles specific tests to t0600
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

Move this test into t0600 with other reffiles specific tests since it
modifies reflog refs manually and thus is specific to the reffiles
backend.

This change also consolidates setup_stash() into test-lib-functions.sh

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 27 +++++++++++++++++++++++
 t/t3903-stash.sh            | 43 -------------------------------------
 t/test-lib-functions.sh     | 16 ++++++++++++++
 3 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 704b73fdc54..bee61b2d19d 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -527,4 +527,31 @@ test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
        test_must_fail git rev-parse --verify broken
 '
 
+test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
+	git init repo &&
+	(
+		cd repo &&
+		setup_stash
+	) &&
+	echo 9 >repo/file &&
+
+	old_oid="$(git -C repo rev-parse stash@{0})" &&
+	git -C repo stash &&
+	new_oid="$(git -C repo rev-parse stash@{0})" &&
+
+	cat >expect <<-EOF &&
+	$(test_oid zero) $old_oid
+	$old_oid $new_oid
+	EOF
+	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	test_cmp expect actual &&
+
+	git -C repo stash drop stash@{1} &&
+	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
+	cat >expect <<-EOF &&
+	$(test_oid zero) $new_oid
+	EOF
+	test_cmp expect actual
+'
+
 test_done
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 34faeac3f1c..0b0e7b19fdc 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -42,22 +42,6 @@ diff_cmp () {
 	rm -f "$1.compare" "$2.compare"
 }
 
-setup_stash() {
-	echo 1 >file &&
-	git add file &&
-	echo unrelated >other-file &&
-	git add other-file &&
-	test_tick &&
-	git commit -m initial &&
-	echo 2 >file &&
-	git add file &&
-	echo 3 >file &&
-	test_tick &&
-	git stash &&
-	git diff-files --quiet &&
-	git diff-index --cached --quiet HEAD
-}
-
 test_expect_success 'stash some dirty working directory' '
 	setup_stash
 '
@@ -200,33 +184,6 @@ test_expect_success 'drop stash reflog updates refs/stash' '
 	test_cmp expect actual
 '
 
-test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
-	git init repo &&
-	(
-		cd repo &&
-		setup_stash
-	) &&
-	echo 9 >repo/file &&
-
-	old_oid="$(git -C repo rev-parse stash@{0})" &&
-	git -C repo stash &&
-	new_oid="$(git -C repo rev-parse stash@{0})" &&
-
-	cat >expect <<-EOF &&
-	$(test_oid zero) $old_oid
-	$old_oid $new_oid
-	EOF
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
-	test_cmp expect actual &&
-
-	git -C repo stash drop stash@{1} &&
-	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
-	cat >expect <<-EOF &&
-	$(test_oid zero) $new_oid
-	EOF
-	test_cmp expect actual
-'
-
 test_expect_success 'stash pop' '
 	git reset --hard &&
 	git stash pop &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index b5eaf7fdc11..68a6c8402d0 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1958,3 +1958,19 @@ test_trailing_hash () {
 		test-tool hexdump |
 		sed "s/ //g"
 }
+
+# Stash some changes
+setup_stash() { echo 1 >file &&
+	git add file &&
+	echo unrelated >other-file &&
+	git add other-file &&
+	test_tick &&
+	git commit -m initial &&
+	echo 2 >file &&
+	git add file &&
+	echo 3 >file &&
+	test_tick &&
+	git stash &&
+	git diff-files --quiet &&
+	git diff-index --cached --quiet HEAD
+}
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 09/12] t1503: move reffiles specific tests to t0600
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

Move this test to t0600 with other reffiles specific tests since it
checks for loose refs and is specific to the reffiles backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 5 +++++
 t/t1503-rev-parse-verify.sh | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 8526e5cf987..704b73fdc54 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -522,4 +522,9 @@ test_expect_success 'refs/worktree must not be packed' '
 	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
 '
 
+test_expect_success SYMLINKS 'ref resolution not confused by broken symlinks' '
+       ln -s does-not-exist .git/refs/heads/broken &&
+       test_must_fail git rev-parse --verify broken
+'
+
 test_done
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index bc136833c10..79df65ec7f6 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -144,11 +144,6 @@ test_expect_success 'main@{n} for various n' '
 	test_must_fail git rev-parse --verify main@{$Np1}
 '
 
-test_expect_success SYMLINKS,REFFILES 'ref resolution not confused by broken symlinks' '
-	ln -s does-not-exist .git/refs/heads/broken &&
-	test_must_fail git rev-parse --verify broken
-'
-
 test_expect_success 'options can appear after --verify' '
 	git rev-parse --verify HEAD >expect &&
 	git rev-parse --verify -q HEAD >actual &&
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 08/12] t1415: move reffiles specific tests to t0600
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

Move this test into t0600 with other reffiles specific tests since it
checks for individua loose refs and thus is specific to the reffiles
backend.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 20 ++++++++++++++++++++
 t/t1415-worktree-refs.sh    | 11 -----------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 0b28a2cc5ea..8526e5cf987 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -502,4 +502,24 @@ test_expect_success 'empty reflog' '
 	test_must_be_empty err
 '
 
+# 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 'refs/worktree must not be packed' '
+	test_commit initial &&
+	test_commit wt1 &&
+	test_commit wt2 &&
+	git worktree add wt1 wt1 &&
+	git worktree add wt2 wt2 &&
+	git checkout initial &&
+	git update-ref refs/worktree/foo HEAD &&
+	git -C wt1 update-ref refs/worktree/foo HEAD &&
+	git -C wt2 update-ref refs/worktree/foo HEAD &&
+	git pack-refs --all &&
+	test_path_is_missing .git/refs/tags/wt1 &&
+	test_path_is_file .git/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
+'
+
 test_done
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 3b531842dd4..eb4eec8becb 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -17,17 +17,6 @@ test_expect_success 'setup' '
 	git -C wt2 update-ref refs/worktree/foo HEAD
 '
 
-# 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 &&
-	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
-	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
-'
-
 test_expect_success 'refs/worktree are per-worktree' '
 	test_cmp_rev worktree/foo initial &&
 	( cd wt1 && test_cmp_rev worktree/foo wt1 ) &&
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 07/12] t1410: move reffiles specific tests to t0600
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

Move these tests to t0600 with other reffiles specific tests since they
do things like take a lock on an individual ref, and write directly into
the reflog refs

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 51 +++++++++++++++++++++++++++++++++++++
 t/t1410-reflog.sh           | 42 ------------------------------
 2 files changed, 51 insertions(+), 42 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 09fbe312092..0b28a2cc5ea 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -451,4 +451,55 @@ test_expect_success 'for_each_reflog()' '
 	test_cmp expected actual
 '
 
+# Triggering the bug detected by this test requires a newline to fall
+# exactly BUFSIZ-1 bytes from the end of the file. We don't know
+# what that value is, since it's platform dependent. However, if
+# we choose some value N, we also catch any D which divides N evenly
+# (since we will read backwards in chunks of D). So we choose 8K,
+# which catches glibc (with an 8K BUFSIZ) and *BSD (1K).
+#
+# Each line is 114 characters, so we need 75 to still have a few before the
+# last 8K. The 89-character padding on the final entry lines up our
+# newline exactly.
+test_expect_success SHA1 'parsing reverse reflogs at BUFSIZ boundaries' '
+	git checkout -b reflogskip &&
+	zf=$(test_oid zero_2) &&
+	ident="abc <xyz> 0000000001 +0000" &&
+	for i in $(test_seq 1 75); do
+		printf "$zf%02d $zf%02d %s\t" $i $(($i+1)) "$ident" &&
+		if test $i = 75; then
+			for j in $(test_seq 1 89); do
+				printf X || return 1
+			done
+		else
+			printf X
+		fi &&
+		printf "\n" || return 1
+	done >.git/logs/refs/heads/reflogskip &&
+	git rev-parse reflogskip@{73} >actual &&
+	echo ${zf}03 >expect &&
+	test_cmp expect actual
+'
+
+# This test takes a lock on an individual ref; this is not supported in
+# reftable.
+test_expect_success 'reflog expire operates on symref not referrent' '
+	git branch --create-reflog the_symref &&
+	git branch --create-reflog referrent &&
+	git update-ref referrent HEAD &&
+	git symbolic-ref refs/heads/the_symref refs/heads/referrent &&
+	test_when_finished "rm -f .git/refs/heads/referrent.lock" &&
+	touch .git/refs/heads/referrent.lock &&
+	git reflog expire --expire=all the_symref
+'
+
+test_expect_success 'empty reflog' '
+	test_when_finished "rm -rf empty" &&
+	git init empty &&
+	test_commit -C empty A &&
+	>empty/.git/logs/refs/heads/foo &&
+	git -C empty reflog expire --all 2>err &&
+	test_must_be_empty err
+'
+
 test_done
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index a0ff8d51f04..d2f5f42e674 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -354,36 +354,6 @@ test_expect_success 'stale dirs do not cause d/f conflicts (reflogs off)' '
 	test_must_be_empty actual
 '
 
-# Triggering the bug detected by this test requires a newline to fall
-# exactly BUFSIZ-1 bytes from the end of the file. We don't know
-# what that value is, since it's platform dependent. However, if
-# we choose some value N, we also catch any D which divides N evenly
-# (since we will read backwards in chunks of D). So we choose 8K,
-# which catches glibc (with an 8K BUFSIZ) and *BSD (1K).
-#
-# Each line is 114 characters, so we need 75 to still have a few before the
-# last 8K. The 89-character padding on the final entry lines up our
-# newline exactly.
-test_expect_success REFFILES,SHA1 'parsing reverse reflogs at BUFSIZ boundaries' '
-	git checkout -b reflogskip &&
-	zf=$(test_oid zero_2) &&
-	ident="abc <xyz> 0000000001 +0000" &&
-	for i in $(test_seq 1 75); do
-		printf "$zf%02d $zf%02d %s\t" $i $(($i+1)) "$ident" &&
-		if test $i = 75; then
-			for j in $(test_seq 1 89); do
-				printf X || return 1
-			done
-		else
-			printf X
-		fi &&
-		printf "\n" || return 1
-	done >.git/logs/refs/heads/reflogskip &&
-	git rev-parse reflogskip@{73} >actual &&
-	echo ${zf}03 >expect &&
-	test_cmp expect actual
-'
-
 test_expect_success 'no segfaults for reflog containing non-commit sha1s' '
 	git update-ref --create-reflog -m "Creating ref" \
 		refs/tests/tree-in-reflog HEAD &&
@@ -397,18 +367,6 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '
 	test_line_count = 3 actual
 '
 
-# This test takes a lock on an individual ref; this is not supported in
-# reftable.
-test_expect_success REFFILES 'reflog expire operates on symref not referrent' '
-	git branch --create-reflog the_symref &&
-	git branch --create-reflog referrent &&
-	git update-ref referrent HEAD &&
-	git symbolic-ref refs/heads/the_symref refs/heads/referrent &&
-	test_when_finished "rm -f .git/refs/heads/referrent.lock" &&
-	touch .git/refs/heads/referrent.lock &&
-	git reflog expire --expire=all the_symref
-'
-
 test_expect_success 'continue walking past root commits' '
 	git init orphanage &&
 	(
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 06/12] t1406: move reffiles specific tests to t0600
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

Move this test to t0600 with the rest of the tests that are specific to
reffiles. This test reaches into reflog directories manually, and so are
specific to reffiles.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh   | 48 +++++++++++++++++++++++++++++++++++
 t/t1407-worktree-ref-store.sh | 37 ---------------------------
 2 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 53ac4b9b5b8..09fbe312092 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -403,4 +403,52 @@ test_expect_success 'delete fails cleanly if packed-refs.new write fails' '
 	test_cmp unchanged actual
 '
 
+RWT="test-tool ref-store worktree:wt"
+RMAIN="test-tool ref-store worktree:main"
+
+test_expect_success 'setup worktree' '
+	test_commit first &&
+	git worktree add -b wt-main wt &&
+	(
+		cd wt &&
+		test_commit second
+	)
+'
+
+# 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 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 'for_each_reflog()' '
+	echo $ZERO_OID > .git/logs/PSEUDO-MAIN &&
+	mkdir -p     .git/logs/refs/bisect &&
+	echo $ZERO_OID > .git/logs/refs/bisect/random &&
+
+	echo $ZERO_OID > .git/worktrees/wt/logs/PSEUDO-WT &&
+	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
+	echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
+
+	$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
+	cat >expected <<-\EOF &&
+	HEAD 0x1
+	PSEUDO-WT 0x0
+	refs/bisect/wt-random 0x0
+	refs/heads/main 0x0
+	refs/heads/wt-main 0x0
+	EOF
+	test_cmp expected actual &&
+
+	$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
+	cat >expected <<-\EOF &&
+	HEAD 0x1
+	PSEUDO-MAIN 0x0
+	refs/bisect/random 0x0
+	refs/heads/main 0x0
+	refs/heads/wt-main 0x0
+	EOF
+	test_cmp expected actual
+'
+
 test_done
diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index 05b1881c591..48b1c92a414 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -53,41 +53,4 @@ test_expect_success 'create_symref(FOO, refs/heads/main)' '
 	test_cmp expected actual
 '
 
-# 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 &&
-
-	echo $ZERO_OID > .git/worktrees/wt/logs/PSEUDO-WT &&
-	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
-	echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
-
-	$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
-	cat >expected <<-\EOF &&
-	HEAD 0x1
-	PSEUDO-WT 0x0
-	refs/bisect/wt-random 0x0
-	refs/heads/main 0x0
-	refs/heads/wt-main 0x0
-	EOF
-	test_cmp expected actual &&
-
-	$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
-	cat >expected <<-\EOF &&
-	HEAD 0x1
-	PSEUDO-MAIN 0x0
-	refs/bisect/random 0x0
-	refs/heads/main 0x0
-	refs/heads/wt-main 0x0
-	EOF
-	test_cmp expected actual
-'
-
 test_done
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 04/12] t1404: move reffiles specific tests to t0600
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

These tests modify loose refs manually and are specific to the reffiles
backend. Move these to t0600 to be part of a test suite of reffiles
specific tests.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh  | 398 +++++++++++++++++++++++++++++++++++
 t/t1404-update-ref-errors.sh | 378 ---------------------------------
 2 files changed, 398 insertions(+), 378 deletions(-)
 create mode 100755 t/t0600-reffiles-backend.sh

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
new file mode 100755
index 00000000000..332c8cbc004
--- /dev/null
+++ b/t/t0600-reffiles-backend.sh
@@ -0,0 +1,398 @@
+#!/bin/sh
+
+test_description='Test reffiles backend'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+# Test adding and deleting D/F-conflicting references in a single
+# transaction.
+df_test() {
+	prefix="$1"
+	pack=: symadd=false symdel=false add_del=false addref= delref=
+	shift
+	while test $# -gt 0
+	do
+		case "$1" in
+		--pack)
+			pack="git pack-refs --all"
+			shift
+			;;
+		--sym-add)
+			# Perform the add via a symbolic reference
+			symadd=true
+			shift
+			;;
+		--sym-del)
+			# Perform the del via a symbolic reference
+			symdel=true
+			shift
+			;;
+		--del-add)
+			# Delete first reference then add second
+			add_del=false
+			delref="$prefix/r/$2"
+			addref="$prefix/r/$3"
+			shift 3
+			;;
+		--add-del)
+			# Add first reference then delete second
+			add_del=true
+			addref="$prefix/r/$2"
+			delref="$prefix/r/$3"
+			shift 3
+			;;
+		*)
+			echo 1>&2 "Extra args to df_test: $*"
+			return 1
+			;;
+		esac
+	done
+	git update-ref "$delref" $C &&
+	if $symadd
+	then
+		addname="$prefix/s/symadd" &&
+		git symbolic-ref "$addname" "$addref"
+	else
+		addname="$addref"
+	fi &&
+	if $symdel
+	then
+		delname="$prefix/s/symdel" &&
+		git symbolic-ref "$delname" "$delref"
+	else
+		delname="$delref"
+	fi &&
+	cat >expected-err <<-EOF &&
+	fatal: cannot lock ref $SQ$addname$SQ: $SQ$delref$SQ exists; cannot create $SQ$addref$SQ
+	EOF
+	$pack &&
+	if $add_del
+	then
+		printf "%s\n" "create $addname $D" "delete $delname"
+	else
+		printf "%s\n" "delete $delname" "create $addname $D"
+	fi >commands &&
+	test_must_fail git update-ref --stdin <commands 2>output.err &&
+	test_cmp expected-err output.err &&
+	printf "%s\n" "$C $delref" >expected-refs &&
+	git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
+	test_cmp expected-refs actual-refs
+}
+
+test_expect_success 'setup' '
+	git commit --allow-empty -m Initial &&
+	C=$(git rev-parse HEAD) &&
+	git commit --allow-empty -m Second &&
+	D=$(git rev-parse HEAD) &&
+	git commit --allow-empty -m Third &&
+	E=$(git rev-parse HEAD)
+'
+
+test_expect_success 'empty directory should not fool rev-parse' '
+	prefix=refs/e-rev-parse &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	echo "$C" >expected &&
+	git rev-parse $prefix/foo >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '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 &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	git for-each-ref $prefix >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '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' '
+	prefix=refs/e-verify &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "verify %s $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 1-arg update' '
+	prefix=refs/e-update-1 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "update %s $D\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 2-arg update' '
+	prefix=refs/e-update-2 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "update %s $D $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 0-arg delete' '
+	prefix=refs/e-delete-0 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "delete %s\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'empty directory should not fool 1-arg delete' '
+	prefix=refs/e-delete-1 &&
+	git update-ref $prefix/foo $C &&
+	git pack-refs --all &&
+	mkdir -p .git/$prefix/foo/bar/baz &&
+	printf "delete %s $C\n" $prefix/foo |
+	git update-ref --stdin
+'
+
+test_expect_success 'D/F conflict prevents add long + delete short' '
+	df_test refs/df-al-ds --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents add short + delete long' '
+	df_test refs/df-as-dl --add-del foo foo/bar
+'
+
+test_expect_success 'D/F conflict prevents delete long + add short' '
+	df_test refs/df-dl-as --del-add foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents delete short + add long' '
+	df_test refs/df-ds-al --del-add foo foo/bar
+'
+
+test_expect_success 'D/F conflict prevents add long + delete short packed' '
+	df_test refs/df-al-dsp --pack --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents add short + delete long packed' '
+	df_test refs/df-as-dlp --pack --add-del foo foo/bar
+'
+
+test_expect_success 'D/F conflict prevents delete long packed + add short' '
+	df_test refs/df-dlp-as --pack --del-add foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents delete short packed + add long' '
+	df_test refs/df-dsp-al --pack --del-add foo foo/bar
+'
+
+# Try some combinations involving symbolic refs...
+
+test_expect_success 'D/F conflict prevents indirect add long + delete short' '
+	df_test refs/df-ial-ds --sym-add --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short' '
+	df_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect add short + indirect delete long' '
+	df_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar
+'
+
+test_expect_success 'D/F conflict prevents indirect delete long + indirect add short' '
+	df_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect add long + delete short packed' '
+	df_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short packed' '
+	df_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents add long + indirect delete short packed' '
+	df_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo
+'
+
+test_expect_success 'D/F conflict prevents indirect delete long packed + indirect add short' '
+	df_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo
+'
+
+test_expect_success 'non-empty directory blocks create' '
+	prefix=refs/ne-create &&
+	mkdir -p .git/$prefix/foo/bar &&
+	: >.git/$prefix/foo/bar/baz.lock &&
+	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/foo $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/foo $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'broken reference blocks create' '
+	prefix=refs/broken-create &&
+	mkdir -p .git/$prefix &&
+	echo "gobbledigook" >.git/$prefix/foo &&
+	test_when_finished "rm -f .git/$prefix/foo" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/foo $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/foo $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'non-empty directory blocks indirect create' '
+	prefix=refs/ne-indirect-create &&
+	git symbolic-ref $prefix/symref $prefix/foo &&
+	mkdir -p .git/$prefix/foo/bar &&
+	: >.git/$prefix/foo/bar/baz.lock &&
+	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/symref $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ
+	EOF
+	printf "%s\n" "update $prefix/symref $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success 'broken reference blocks indirect create' '
+	prefix=refs/broken-indirect-create &&
+	git symbolic-ref $prefix/symref $prefix/foo &&
+	echo "gobbledigook" >.git/$prefix/foo &&
+	test_when_finished "rm -f .git/$prefix/foo" &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/symref $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err &&
+	cat >expected <<-EOF &&
+	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
+	EOF
+	printf "%s\n" "update $prefix/symref $D $C" |
+	test_must_fail git update-ref --stdin 2>output.err &&
+	test_cmp expected output.err
+'
+
+test_expect_success '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 &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	# Now try to update the reference, but hold the `packed-refs` lock
+	# for a while to see what happens while the process is blocked:
+	: >.git/packed-refs.lock &&
+	test_when_finished "rm -f .git/packed-refs.lock" &&
+	{
+		# Note: the following command is intentionally run in the
+		# background. We increase the timeout so that `update-ref`
+		# attempts to acquire the `packed-refs` lock for much longer
+		# than it takes for us to do the check then delete it:
+		git -c core.packedrefstimeout=30000 update-ref -d $prefix/foo &
+	} &&
+	pid2=$! &&
+	# Give update-ref plenty of time to get to the point where it tries
+	# to lock packed-refs:
+	sleep 1 &&
+	# Make sure that update-ref did not complete despite the lock:
+	kill -0 $pid2 &&
+	# Verify that the reference still has its old value:
+	sha1=$(git rev-parse --verify --quiet $prefix/foo || echo undefined) &&
+	case "$sha1" in
+	$D)
+		# This is what we hope for; it means that nothing
+		# user-visible has changed yet.
+		: ;;
+	undefined)
+		# This is not correct; it means the deletion has happened
+		# already even though update-ref should not have been
+		# able to acquire the lock yet.
+		echo "$prefix/foo deleted prematurely" &&
+		break
+		;;
+	$C)
+		# This value should never be seen. Probably the loose
+		# reference has been deleted but the packed reference
+		# is still there:
+		echo "$prefix/foo incorrectly observed to be C" &&
+		break
+		;;
+	*)
+		# WTF?
+		echo "unexpected value observed for $prefix/foo: $sha1" &&
+		break
+		;;
+	esac >out &&
+	rm -f .git/packed-refs.lock &&
+	wait $pid2 &&
+	test_must_be_empty out &&
+	test_must_fail git rev-parse --verify --quiet $prefix/foo
+'
+
+test_expect_success '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 &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	git for-each-ref $prefix >unchanged &&
+	# Now try to delete it while the `packed-refs` lock is held:
+	: >.git/packed-refs.lock &&
+	test_when_finished "rm -f .git/packed-refs.lock" &&
+	test_must_fail git update-ref -d $prefix/foo >out 2>err &&
+	git for-each-ref $prefix >actual &&
+	test_grep "Unable to create $SQ.*packed-refs.lock$SQ: " err &&
+	test_cmp unchanged actual
+'
+
+test_expect_success '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 &&
+	git pack-refs --all &&
+	git update-ref $prefix/foo $D &&
+	git for-each-ref $prefix >unchanged &&
+	# This should not happen in practice, but it is an easy way to get a
+	# reliable error (we open with create_tempfile(), which uses O_EXCL).
+	: >.git/packed-refs.new &&
+	test_when_finished "rm -f .git/packed-refs.new" &&
+	test_must_fail git update-ref -d $prefix/foo &&
+	git for-each-ref $prefix >actual &&
+	test_cmp unchanged actual
+'
+
+test_done
diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index 0369beea33b..6edf3dca9d5 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -34,81 +34,6 @@ test_update_rejected () {
 	test_cmp unchanged actual
 }
 
-# Test adding and deleting D/F-conflicting references in a single
-# transaction.
-df_test() {
-	prefix="$1"
-	pack=: symadd=false symdel=false add_del=false addref= delref=
-	shift
-	while test $# -gt 0
-	do
-		case "$1" in
-		--pack)
-			pack="git pack-refs --all"
-			shift
-			;;
-		--sym-add)
-			# Perform the add via a symbolic reference
-			symadd=true
-			shift
-			;;
-		--sym-del)
-			# Perform the del via a symbolic reference
-			symdel=true
-			shift
-			;;
-		--del-add)
-			# Delete first reference then add second
-			add_del=false
-			delref="$prefix/r/$2"
-			addref="$prefix/r/$3"
-			shift 3
-			;;
-		--add-del)
-			# Add first reference then delete second
-			add_del=true
-			addref="$prefix/r/$2"
-			delref="$prefix/r/$3"
-			shift 3
-			;;
-		*)
-			echo 1>&2 "Extra args to df_test: $*"
-			return 1
-			;;
-		esac
-	done
-	git update-ref "$delref" $C &&
-	if $symadd
-	then
-		addname="$prefix/s/symadd" &&
-		git symbolic-ref "$addname" "$addref"
-	else
-		addname="$addref"
-	fi &&
-	if $symdel
-	then
-		delname="$prefix/s/symdel" &&
-		git symbolic-ref "$delname" "$delref"
-	else
-		delname="$delref"
-	fi &&
-	cat >expected-err <<-EOF &&
-	fatal: cannot lock ref $SQ$addname$SQ: $SQ$delref$SQ exists; cannot create $SQ$addref$SQ
-	EOF
-	$pack &&
-	if $add_del
-	then
-		printf "%s\n" "create $addname $D" "delete $delname"
-	else
-		printf "%s\n" "delete $delname" "create $addname $D"
-	fi >commands &&
-	test_must_fail git update-ref --stdin <commands 2>output.err &&
-	test_cmp expected-err output.err &&
-	printf "%s\n" "$C $delref" >expected-refs &&
-	git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
-	test_cmp expected-refs actual-refs
-}
-
 test_expect_success 'setup' '
 
 	git commit --allow-empty -m Initial &&
@@ -191,144 +116,6 @@ test_expect_success 'one new ref is a simple prefix of another' '
 
 '
 
-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 &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	echo "$C" >expected &&
-	git rev-parse $prefix/foo >actual &&
-	test_cmp expected actual
-'
-
-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 &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	git for-each-ref $prefix >actual &&
-	test_cmp expected actual
-'
-
-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 REFFILES 'empty directory should not fool verify' '
-	prefix=refs/e-verify &&
-	git update-ref $prefix/foo $C &&
-	git pack-refs --all &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "verify %s $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-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 &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "update %s $D\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-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 &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "update %s $D $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-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 &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "delete %s\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-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 &&
-	mkdir -p .git/$prefix/foo/bar/baz &&
-	printf "delete %s $C\n" $prefix/foo |
-	git update-ref --stdin
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add long + delete short' '
-	df_test refs/df-al-ds --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add short + delete long' '
-	df_test refs/df-as-dl --add-del foo foo/bar
-'
-
-test_expect_success REFFILES 'D/F conflict prevents delete long + add short' '
-	df_test refs/df-dl-as --del-add foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents delete short + add long' '
-	df_test refs/df-ds-al --del-add foo foo/bar
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add long + delete short packed' '
-	df_test refs/df-al-dsp --pack --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add short + delete long packed' '
-	df_test refs/df-as-dlp --pack --add-del foo foo/bar
-'
-
-test_expect_success REFFILES 'D/F conflict prevents delete long packed + add short' '
-	df_test refs/df-dlp-as --pack --del-add foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents delete short packed + add long' '
-	df_test refs/df-dsp-al --pack --del-add foo foo/bar
-'
-
-# Try some combinations involving symbolic refs...
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add long + delete short' '
-	df_test refs/df-ial-ds --sym-add --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add long + indirect delete short' '
-	df_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add short + indirect delete long' '
-	df_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect delete long + indirect add short' '
-	df_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add long + delete short packed' '
-	df_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect add long + indirect delete short packed' '
-	df_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents add long + indirect delete short packed' '
-	df_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo
-'
-
-test_expect_success REFFILES 'D/F conflict prevents indirect delete long packed + indirect add short' '
-	df_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo
-'
-
 # Test various errors when reading the old values of references...
 
 test_expect_success 'missing old value blocks update' '
@@ -468,169 +255,4 @@ test_expect_success 'incorrect old value blocks indirect no-deref delete' '
 	test_cmp expected output.err
 '
 
-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 &&
-	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/foo $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/foo $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-test_expect_success REFFILES 'broken reference blocks create' '
-	prefix=refs/broken-create &&
-	mkdir -p .git/$prefix &&
-	echo "gobbledigook" >.git/$prefix/foo &&
-	test_when_finished "rm -f .git/$prefix/foo" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/foo $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/foo $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-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 &&
-	: >.git/$prefix/foo/bar/baz.lock &&
-	test_when_finished "rm -f .git/$prefix/foo/bar/baz.lock" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/symref $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ
-	EOF
-	printf "%s\n" "update $prefix/symref $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-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 &&
-	test_when_finished "rm -f .git/$prefix/foo" &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/symref $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err &&
-	cat >expected <<-EOF &&
-	fatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken
-	EOF
-	printf "%s\n" "update $prefix/symref $D $C" |
-	test_must_fail git update-ref --stdin 2>output.err &&
-	test_cmp expected output.err
-'
-
-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 &&
-	git pack-refs --all &&
-	git update-ref $prefix/foo $D &&
-	# Now try to update the reference, but hold the `packed-refs` lock
-	# for a while to see what happens while the process is blocked:
-	: >.git/packed-refs.lock &&
-	test_when_finished "rm -f .git/packed-refs.lock" &&
-	{
-		# Note: the following command is intentionally run in the
-		# background. We increase the timeout so that `update-ref`
-		# attempts to acquire the `packed-refs` lock for much longer
-		# than it takes for us to do the check then delete it:
-		git -c core.packedrefstimeout=30000 update-ref -d $prefix/foo &
-	} &&
-	pid2=$! &&
-	# Give update-ref plenty of time to get to the point where it tries
-	# to lock packed-refs:
-	sleep 1 &&
-	# Make sure that update-ref did not complete despite the lock:
-	kill -0 $pid2 &&
-	# Verify that the reference still has its old value:
-	sha1=$(git rev-parse --verify --quiet $prefix/foo || echo undefined) &&
-	case "$sha1" in
-	$D)
-		# This is what we hope for; it means that nothing
-		# user-visible has changed yet.
-		: ;;
-	undefined)
-		# This is not correct; it means the deletion has happened
-		# already even though update-ref should not have been
-		# able to acquire the lock yet.
-		echo "$prefix/foo deleted prematurely" &&
-		break
-		;;
-	$C)
-		# This value should never be seen. Probably the loose
-		# reference has been deleted but the packed reference
-		# is still there:
-		echo "$prefix/foo incorrectly observed to be C" &&
-		break
-		;;
-	*)
-		# WTF?
-		echo "unexpected value observed for $prefix/foo: $sha1" &&
-		break
-		;;
-	esac >out &&
-	rm -f .git/packed-refs.lock &&
-	wait $pid2 &&
-	test_must_be_empty out &&
-	test_must_fail git rev-parse --verify --quiet $prefix/foo
-'
-
-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 &&
-	git pack-refs --all &&
-	git update-ref $prefix/foo $D &&
-	git for-each-ref $prefix >unchanged &&
-	# Now try to delete it while the `packed-refs` lock is held:
-	: >.git/packed-refs.lock &&
-	test_when_finished "rm -f .git/packed-refs.lock" &&
-	test_must_fail git update-ref -d $prefix/foo >out 2>err &&
-	git for-each-ref $prefix >actual &&
-	test_grep "Unable to create $SQ.*packed-refs.lock$SQ: " err &&
-	test_cmp unchanged actual
-'
-
-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 &&
-	git pack-refs --all &&
-	git update-ref $prefix/foo $D &&
-	git for-each-ref $prefix >unchanged &&
-	# This should not happen in practice, but it is an easy way to get a
-	# reliable error (we open with create_tempfile(), which uses O_EXCL).
-	: >.git/packed-refs.new &&
-	test_when_finished "rm -f .git/packed-refs.new" &&
-	test_must_fail git update-ref -d $prefix/foo &&
-	git for-each-ref $prefix >actual &&
-	test_cmp unchanged actual
-'
-
 test_done
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 05/12] t1405: move reffiles specific tests to t0600
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

Move this test to t0600 with other reffiles specific tests since it is
reffiles specific in that it looks into the loose refs directory for an
assertion.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t0600-reffiles-backend.sh | 8 ++++++++
 t/t1405-main-ref-store.sh   | 8 --------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 332c8cbc004..53ac4b9b5b8 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -92,6 +92,14 @@ test_expect_success 'setup' '
 	E=$(git rev-parse HEAD)
 '
 
+test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
+	N=`find .git/refs -type f | wc -l` &&
+	test "$N" != 0 &&
+	test-tool ref-store main pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL &&
+	N=`find .git/refs -type f` &&
+	test -z "$N"
+'
+
 test_expect_success 'empty directory should not fool rev-parse' '
 	prefix=refs/e-rev-parse &&
 	git update-ref $prefix/foo $C &&
diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index 62c1eadb190..976bd71efb5 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -15,14 +15,6 @@ test_expect_success 'setup' '
 	test_commit one
 '
 
-test_expect_success REFFILES 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
-	N=`find .git/refs -type f | wc -l` &&
-	test "$N" != 0 &&
-	$RUN pack-refs PACK_REFS_PRUNE,PACK_REFS_ALL &&
-	N=`find .git/refs -type f` &&
-	test -z "$N"
-'
-
 test_expect_success 'create_symref(FOO, refs/heads/main)' '
 	$RUN create-symref FOO refs/heads/main nothing &&
 	echo refs/heads/main >expected &&
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH 03/12] t1414: convert test to use Git commands instead of writing refs manually
From: John Cai via GitGitGadget @ 2024-01-17 19:52 UTC (permalink / raw)
  To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1647.git.git.1705521155.gitgitgadget@gmail.com>

From: John Cai <johncai86@gmail.com>

This test can be re-written to use Git commands rather than writing a
manual ref in the reflog. This way this test no longer needs the
REFFILES prerequisite.

Signed-off-by: John Cai <johncai86@gmail.com>
---
 t/t1414-reflog-walk.sh | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/t/t1414-reflog-walk.sh b/t/t1414-reflog-walk.sh
index ea64cecf47b..c7b3817d3bd 100755
--- a/t/t1414-reflog-walk.sh
+++ b/t/t1414-reflog-walk.sh
@@ -121,13 +121,14 @@ test_expect_success 'min/max age uses entry date to limit' '
 
 # 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' '
+test_expect_success 'walk prefers reflog to ref tip' '
+	test_commit A &&
+	test_commit B &&
+	git reflog delete HEAD@{0} &&
 	head=$(git rev-parse HEAD) &&
-	one=$(git rev-parse one) &&
-	ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
-	echo "$head $one $ident	broken reflog entry" >>.git/logs/HEAD &&
+	A=$(git rev-parse A) &&
 
-	echo $one >expect &&
+	echo $A >expect &&
 	git log -g --format=%H -1 >actual &&
 	test_cmp expect actual
 '
-- 
gitgitgadget


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox