diff mbox series

[v2,4/6] worktree: teach `list --porcelain` to annotate locked worktree

Message ID 20210117234244.95106-5-rafaeloliveira.cs@gmail.com (mailing list archive)
State New
Headers show
Series teach `worktree list` verbose mode and prunable annotations | expand

Commit Message

Rafael Silva Jan. 17, 2021, 11:42 p.m. UTC
Commit c57b3367be (worktree: teach `list` to annotate locked worktree,
2020-10-11) taught "git worktree list" to annotate locked worktrees by
appending "locked" text to its output, however, this is not listed in
the --porcelain format.

Teach "list --porcelain" to do the same and add a "locked" attribute
followed by its reason, thus making both default and porcelain format
consistent. If the locked reason is not available then only "locked"
is shown.

The output of the "git worktree list --porcelain" becomes like so:

    $ git worktree list --porcelain
    ...
    worktree /path/to/locked
    HEAD 123abcdea123abcd123acbd123acbda123abcd12
    detached
    locked

    worktree /path/to/locked-with-reason
    HEAD abc123abc123abc123abc123abc123abc123abc1
    detached
    locked reason why it is locked
    ...

The locked reason is quoted to prevent newline characters (i.e: LF or
CRLF) breaking the --porcelain format as each attribute is shown per
line. For example:

   $ git worktree list --porcelain
   ...
   locked worktree's path mounted in\nremovable device
   ...

Furthermore, let's update the documentation to state that some
attributes in the porcelain format might be listed alone or together
with its value depending whether the value is available or not. Thus
documenting the case of the new "locked" attribute.

Additionally, c57b3367be (worktree: teach `list` to annotate locked
worktree, 2020-10-11) introduced a new test to ensure locked worktrees
are listed with "locked" annotation. However, the test does not clean up
after itself as "git worktree prune" is not going to remove the locked
worktree in the first place. This not only leaves the test in an unclean
state it also potentially breaks following tests that relies on the
"git worktree list" output. Let's fix that by unlocking the worktree
before the "prune" command.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com>
---
 Documentation/git-worktree.txt | 16 ++++++++++++++--
 builtin/worktree.c             | 13 +++++++++++++
 t/t2402-worktree-list.sh       | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 2 deletions(-)

Comments

Eric Sunshine Jan. 18, 2021, 3:55 a.m. UTC | #1
On Sun, Jan 17, 2021 at 6:43 PM Rafael Silva
<rafaeloliveira.cs@gmail.com> wrote:
> Commit c57b3367be (worktree: teach `list` to annotate locked worktree,
> 2020-10-11) taught "git worktree list" to annotate locked worktrees by
> appending "locked" text to its output, however, this is not listed in
> the --porcelain format.
>
> Teach "list --porcelain" to do the same and add a "locked" attribute
> followed by its reason, thus making both default and porcelain format
> consistent. If the locked reason is not available then only "locked"
> is shown.
>
> The output of the "git worktree list --porcelain" becomes like so:
>
>     $ git worktree list --porcelain
>     ...
>     worktree /path/to/locked
>     HEAD 123abcdea123abcd123acbd123acbda123abcd12
>     detached
>     locked
>
>     worktree /path/to/locked-with-reason
>     HEAD abc123abc123abc123abc123abc123abc123abc1
>     detached
>     locked reason why it is locked
>     ...

All good.

> The locked reason is quoted to prevent newline characters (i.e: LF or
> CRLF) breaking the --porcelain format as each attribute is shown per
> line. For example:
>
>    $ git worktree list --porcelain
>    ...
>    locked worktree's path mounted in\nremovable device
>    ...

I have a bit to say about this below.

> Additionally, c57b3367be (worktree: teach `list` to annotate locked
> worktree, 2020-10-11) introduced a new test to ensure locked worktrees
> are listed with "locked" annotation. However, the test does not clean up
> after itself as "git worktree prune" is not going to remove the locked
> worktree in the first place. This not only leaves the test in an unclean
> state it also potentially breaks following tests that relies on the
> "git worktree list" output. Let's fix that by unlocking the worktree
> before the "prune" command.

The actual code change to fix this bug is about as minimal as it gets,
but the explanation you've written here is lengthy enough and nicely
self-contained that it suggests splitting it off to its own patch. And
since you can re-use this paragraph almost verbatim as the commit
message, it shouldn't require much work to do so. On the other hand,
it is itself not necessarily worth a re-roll, but if you do re-roll,
perhaps it's worth considering.

> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -377,8 +377,10 @@ Porcelain Format
>  The porcelain format has a line per attribute.  Attributes are listed with a
>  label and value separated by a single space.  Boolean attributes (like `bare`
>  and `detached`) are listed as a label only, and are present only
> -if the value is true.  The first attribute of a working tree is always
> -`worktree`, an empty line indicates the end of the record.  For example:
> +if the value is true.  Some attributes (like `locked`) can be listed as a label
> +only or with a value depending whether a reason is available.  The first

Perhaps:
s/depending whether/depending upon whether/

> +attribute of a working tree is always `worktree`, an empty line indicates the
> +end of the record.  For example:
> @@ -393,6 +395,16 @@ worktree /path/to/other-linked-worktree
> +worktree /path/to/linked-worktree-locked
> +HEAD 5678abc5678abc5678abc5678abc5678abc5678c
> +branch refs/heads/locked
> +locked
> +
> +worktree /path/to/linked-worktree-locked-with-reason
> +HEAD 3456def3456def3456def3456def3456def3456b
> +branch refs/heads/locked-with-reason
> +locked reason why is locked

I was momentarily confused by the branch named `locked` with the
`locked` attribute in the first new stanza. Perhaps take a hint from
the second new stanza and call the first one `locked-no-reason`:

    worktree /path/to/linked-worktree-locked-no-reason
    HEAD 5678abc5678abc5678abc5678abc5678abc5678c
    branch refs/heads/locked-no-reason
    locked

Again, though, not worth a re-roll.

> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -579,6 +582,16 @@ static void show_worktree_porcelain(struct worktree *wt)
> +       reason = worktree_lock_reason(wt);
> +       if (reason && *reason) {
> +               struct strbuf sb = STRBUF_INIT;
> +               quote_c_style(reason, &sb, NULL, CQUOTE_NODQ);
> +               printf("locked %s\n", sb.buf);
> +               strbuf_release(&sb);
> +       } else if (reason)
> +               printf("locked\n");

This needs a change, and it's totally my fault that it does. In my
previous review, I mentioned that if the lock reason contains special
characters, we want those special characters escaped and the reason
quoted, but _only_ if it contains special characters. However, I then
incorrectly said to call quote_c_style() with CQUOTE_NODQ to achieve
that behavior. In fact, CQUOTE_NODQ gives us the wrong behavior since
it avoids quoting the string which, as Phillip pointed out, makes it
impossible to distinguish between a string which just happens to
contain the two-character sequence '\' and 'n', and an escaped newline
"\n". So, the above should really be:

    quote_c_style(reason, &sb, NULL, 0);

The example in the commit message should be adjusted to account for
this change, as well:

    In porcelain mode, if the lock reason contains special characters
    such as newlines, they are escaped with backslashes and the entire
    reason is enclosed in double quotes. For example:

    $ git worktree list --porcelain
    ...
    locked "worktree's path mounted in\nremovable device"
    ...

And, of course, the new test will need a slight adjustment.

> diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
> @@ -66,11 +66,43 @@ test_expect_success '"list" all worktrees with locked annotation' '
> +test_expect_success '"list" all worktrees --porcelain with locked' '
> +       test_when_finished "rm -rf locked1 locked2 unlocked out actual expect && git worktree prune" &&
> +       echo "locked" >expect &&
> +       echo "locked with reason" >>expect &&
> +       git worktree add --detach locked1 &&
> +       git worktree add --detach locked2 &&
> +       git worktree add --detach unlocked &&
> +       git worktree lock locked1 &&
> +       git worktree lock locked2 --reason "with reason" &&
> +       test_when_finished "git worktree unlock locked1 && git worktree unlock locked2" &&
> +       git worktree list --porcelain >out &&
> +       grep "^locked" out >actual &&
> +       test_cmp expect actual
> +'

So, the purpose of the `unlocked` worktree in this test is to prove
that it didn't accidentally get annotated with `locked`? (Since, if it
did get annotated, then `actual` would contain too many lines and not
match `expect`.) Is that correct?

> +test_expect_success '"list" all worktrees --porcelain with locked reason newline escaped' '
> +       test_when_finished "rm -rf locked_lf locked_crlf out actual expect && git worktree prune" &&
> +       printf "locked locked\\\\r\\\\nreason\n" >expect &&
> +       printf "locked locked\\\\nreason\n" >>expect &&
> +       git worktree add --detach locked_lf &&
> +       git worktree add --detach locked_crlf &&
> +       printf "locked\nreason\n\n" >reason_lf &&
> +       printf "locked\r\nreason\n\n" >reason_crlf &&

The trailing "\n\n" is unneeded. Due to the way `$(...)` expansion
works (dropping trailing whitespace), you'll get the same successful
test result with:

    printf "locked\nreason\n" >reason_lf &&
    printf "locked\r\nreason\n" >reason_crlf &&

and even with:

    printf "locked\nreason" >reason_lf &&
    printf "locked\r\nreason" >reason_crlf &&

> +       git worktree lock locked_lf --reason "$(cat reason_lf)" &&
> +       git worktree lock locked_crlf --reason "$(cat reason_crlf)" &&

You could also just embed the `printf`'s here rather than using these
temporary files.

    git worktree lock --reason $(printf "...") <path> &&

Or, if we care only about testing LF, and not about CRLF, even this would work:

    git worktree lock --reason "reason with
    newline" <path> &&

but that gets a bit ugly.

Anyhow, all the line terminator commentary about this test is a matter
of personal taste, probably not worth a re-roll or even changing.
Rafael Silva Jan. 19, 2021, 8:20 a.m. UTC | #2
Thanks for the review.

Eric Sunshine writes:

> On Sun, Jan 17, 2021 at 6:43 PM Rafael Silva
> <rafaeloliveira.cs@gmail.com> wrote:

>> Additionally, c57b3367be (worktree: teach `list` to annotate locked
>> worktree, 2020-10-11) introduced a new test to ensure locked worktrees
>> are listed with "locked" annotation. However, the test does not clean up
>> after itself as "git worktree prune" is not going to remove the locked
>> worktree in the first place. This not only leaves the test in an unclean
>> state it also potentially breaks following tests that relies on the
>> "git worktree list" output. Let's fix that by unlocking the worktree
>> before the "prune" command.
>
> The actual code change to fix this bug is about as minimal as it gets,
> but the explanation you've written here is lengthy enough and nicely
> self-contained that it suggests splitting it off to its own patch. And
> since you can re-use this paragraph almost verbatim as the commit
> message, it shouldn't require much work to do so. On the other hand,
> it is itself not necessarily worth a re-roll, but if you do re-roll,
> perhaps it's worth considering.
>

make sense, I'll re-roll with this change on its own patch. I actually
thought about splitting it off during as well, but I wasn't sure whether
this was a good idea. Now that you mentioned here I guess it sounds a
like reasonable change for the next version.


>> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
>> @@ -377,8 +377,10 @@ Porcelain Format
>>  The porcelain format has a line per attribute.  Attributes are listed with a
>>  label and value separated by a single space.  Boolean attributes (like `bare`
>>  and `detached`) are listed as a label only, and are present only
>> -if the value is true.  The first attribute of a working tree is always
>> -`worktree`, an empty line indicates the end of the record.  For example:
>> +if the value is true.  Some attributes (like `locked`) can be listed as a label
>> +only or with a value depending whether a reason is available.  The first
>
> Perhaps:
> s/depending whether/depending upon whether/
>

Yeah, I think that's sounds bit better.

>> +attribute of a working tree is always `worktree`, an empty line indicates the
>> +end of the record.  For example:
>> @@ -393,6 +395,16 @@ worktree /path/to/other-linked-worktree
>> +worktree /path/to/linked-worktree-locked
>> +HEAD 5678abc5678abc5678abc5678abc5678abc5678c
>> +branch refs/heads/locked
>> +locked
>> +
>> +worktree /path/to/linked-worktree-locked-with-reason
>> +HEAD 3456def3456def3456def3456def3456def3456b
>> +branch refs/heads/locked-with-reason
>> +locked reason why is locked
>
> I was momentarily confused by the branch named `locked` with the
> `locked` attribute in the first new stanza. Perhaps take a hint from
> the second new stanza and call the first one `locked-no-reason`:
>
>     worktree /path/to/linked-worktree-locked-no-reason
>     HEAD 5678abc5678abc5678abc5678abc5678abc5678c
>     branch refs/heads/locked-no-reason
>     locked
>
> Again, though, not worth a re-roll.
>

This seems like a nice touch. will include this in the next revision.

>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> @@ -579,6 +582,16 @@ static void show_worktree_porcelain(struct worktree *wt)
>> +       reason = worktree_lock_reason(wt);
>> +       if (reason && *reason) {
>> +               struct strbuf sb = STRBUF_INIT;
>> +               quote_c_style(reason, &sb, NULL, CQUOTE_NODQ);
>> +               printf("locked %s\n", sb.buf);
>> +               strbuf_release(&sb);
>> +       } else if (reason)
>> +               printf("locked\n");
>
> This needs a change, and it's totally my fault that it does. In my
> previous review, I mentioned that if the lock reason contains special
> characters, we want those special characters escaped and the reason
> quoted, but _only_ if it contains special characters. However, I then
> incorrectly said to call quote_c_style() with CQUOTE_NODQ to achieve
> that behavior. In fact, CQUOTE_NODQ gives us the wrong behavior since
> it avoids quoting the string which, as Phillip pointed out, makes it
> impossible to distinguish between a string which just happens to
> contain the two-character sequence '\' and 'n', and an escaped newline
> "\n". So, the above should really be:
>
>     quote_c_style(reason, &sb, NULL, 0);
>
> The example in the commit message should be adjusted to account for
> this change, as well:
>
>     In porcelain mode, if the lock reason contains special characters
>     such as newlines, they are escaped with backslashes and the entire
>     reason is enclosed in double quotes. For example:
>
>     $ git worktree list --porcelain
>     ...
>     locked "worktree's path mounted in\nremovable device"
>     ...
>
> And, of course, the new test will need a slight adjustment.
>

Alright, I believe I've got the whole picture now and sorry for the
confusion. You and Phillip clearly stated in the review cycle that the
reason should be quoted because of the aforementioned reasons and I 
dropped when I was working on this version.

I will re-roll and change this in the next revision.

>> diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
>> @@ -66,11 +66,43 @@ test_expect_success '"list" all worktrees with locked annotation' '
>> +test_expect_success '"list" all worktrees --porcelain with locked' '
>> +       test_when_finished "rm -rf locked1 locked2 unlocked out actual expect && git worktree prune" &&
>> +       echo "locked" >expect &&
>> +       echo "locked with reason" >>expect &&
>> +       git worktree add --detach locked1 &&
>> +       git worktree add --detach locked2 &&
>> +       git worktree add --detach unlocked &&
>> +       git worktree lock locked1 &&
>> +       git worktree lock locked2 --reason "with reason" &&
>> +       test_when_finished "git worktree unlock locked1 && git worktree unlock locked2" &&
>> +       git worktree list --porcelain >out &&
>> +       grep "^locked" out >actual &&
>> +       test_cmp expect actual
>> +'
>
> So, the purpose of the `unlocked` worktree in this test is to prove
> that it didn't accidentally get annotated with `locked`? (Since, if it
> did get annotated, then `actual` would contain too many lines and not
> match `expect`.) Is that correct?
>

Yes, this is what I intended to check when adding the `unlocked`
worktree. I'm considering how to make this more explicit so it's clear
for readers why the `unlocked` worktree exists in this test.


>> +test_expect_success '"list" all worktrees --porcelain with locked reason newline escaped' '
>> +       test_when_finished "rm -rf locked_lf locked_crlf out actual expect && git worktree prune" &&
>> +       printf "locked locked\\\\r\\\\nreason\n" >expect &&
>> +       printf "locked locked\\\\nreason\n" >>expect &&
>> +       git worktree add --detach locked_lf &&
>> +       git worktree add --detach locked_crlf &&
>> +       printf "locked\nreason\n\n" >reason_lf &&
>> +       printf "locked\r\nreason\n\n" >reason_crlf &&
>
> The trailing "\n\n" is unneeded. Due to the way `$(...)` expansion
> works (dropping trailing whitespace), you'll get the same successful
> test result with:
>
>     printf "locked\nreason\n" >reason_lf &&
>     printf "locked\r\nreason\n" >reason_crlf &&
>
> and even with:
>
>     printf "locked\nreason" >reason_lf &&
>     printf "locked\r\nreason" >reason_crlf &&
>
>> +       git worktree lock locked_lf --reason "$(cat reason_lf)" &&
>> +       git worktree lock locked_crlf --reason "$(cat reason_crlf)" &&
>
> You could also just embed the `printf`'s here rather than using these
> temporary files.
>
>     git worktree lock --reason $(printf "...") <path> &&
>

Having the `printf` together with the $(...) expansion seems like the
good simplification for this test. will include in the next revision.

> Or, if we care only about testing LF, and not about CRLF, even this would work:
>
>     git worktree lock --reason "reason with
>     newline" <path> &&
>
> but that gets a bit ugly.
>
> Anyhow, all the line terminator commentary about this test is a matter
> of personal taste, probably not worth a re-roll or even changing.
Eric Sunshine Jan. 19, 2021, 5:16 p.m. UTC | #3
On Tue, Jan 19, 2021 at 3:20 AM Rafael Silva
<rafaeloliveira.cs@gmail.com> wrote:
> Eric Sunshine writes:
> > On Sun, Jan 17, 2021 at 6:43 PM Rafael Silva
> > <rafaeloliveira.cs@gmail.com> wrote:
> >> +               quote_c_style(reason, &sb, NULL, CQUOTE_NODQ);
> >
> > This needs a change, and it's totally my fault that it does. In my
> > previous review, I mentioned that if the lock reason contains special
> > characters, we want those special characters escaped and the reason
> > quoted, but _only_ if it contains special characters. However, I then
> > incorrectly said to call quote_c_style() with CQUOTE_NODQ to achieve
> > that behavior. In fact, CQUOTE_NODQ gives us the wrong behavior since
> > it avoids quoting the string which, as Phillip pointed out, makes it
> > impossible to distinguish between a string which just happens to
> > contain the two-character sequence '\' and 'n', and an escaped newline
> > "\n". So, the above should really be:
> >
> Alright, I believe I've got the whole picture now and sorry for the
> confusion. You and Phillip clearly stated in the review cycle that the
> reason should be quoted because of the aforementioned reasons and I
> dropped when I was working on this version.

It was my fault by confusing you with the misleading mention of CQUOTE_NODQ.

> >> +       git worktree add --detach locked1 &&
> >> +       git worktree add --detach locked2 &&
> >> +       git worktree add --detach unlocked &&
> >
> > So, the purpose of the `unlocked` worktree in this test is to prove
> > that it didn't accidentally get annotated with `locked`? (Since, if it
> > did get annotated, then `actual` would contain too many lines and not
> > match `expect`.) Is that correct?
>
> Yes, this is what I intended to check when adding the `unlocked`
> worktree. I'm considering how to make this more explicit so it's clear
> for readers why the `unlocked` worktree exists in this test.

A simple in-code comment should suffice, I would think:

    git worktree add --detach locked1 &&
    git worktree add --detach locked2 &&
    # unlocked worktree should not be annotated "locked"
    git worktree add --detach unlocked &&
diff mbox series

Patch

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 02a706c4c0..d352a002f2 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -377,8 +377,10 @@  Porcelain Format
 The porcelain format has a line per attribute.  Attributes are listed with a
 label and value separated by a single space.  Boolean attributes (like `bare`
 and `detached`) are listed as a label only, and are present only
-if the value is true.  The first attribute of a working tree is always
-`worktree`, an empty line indicates the end of the record.  For example:
+if the value is true.  Some attributes (like `locked`) can be listed as a label
+only or with a value depending whether a reason is available.  The first
+attribute of a working tree is always `worktree`, an empty line indicates the
+end of the record.  For example:
 
 ------------
 $ git worktree list --porcelain
@@ -393,6 +395,16 @@  worktree /path/to/other-linked-worktree
 HEAD 1234abc1234abc1234abc1234abc1234abc1234a
 detached
 
+worktree /path/to/linked-worktree-locked
+HEAD 5678abc5678abc5678abc5678abc5678abc5678c
+branch refs/heads/locked
+locked
+
+worktree /path/to/linked-worktree-locked-with-reason
+HEAD 3456def3456def3456def3456def3456def3456b
+branch refs/heads/locked-with-reason
+locked reason why is locked
+
 ------------
 
 EXAMPLES
diff --git a/builtin/worktree.c b/builtin/worktree.c
index dd886d5029..37ae277352 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -12,6 +12,7 @@ 
 #include "submodule.h"
 #include "utf8.h"
 #include "worktree.h"
+#include "quote.h"
 
 static const char * const worktree_usage[] = {
 	N_("git worktree add [<options>] <path> [<commit-ish>]"),
@@ -569,6 +570,8 @@  static int add(int ac, const char **av, const char *prefix)
 
 static void show_worktree_porcelain(struct worktree *wt)
 {
+	const char *reason;
+
 	printf("worktree %s\n", wt->path);
 	if (wt->is_bare)
 		printf("bare\n");
@@ -579,6 +582,16 @@  static void show_worktree_porcelain(struct worktree *wt)
 		else if (wt->head_ref)
 			printf("branch %s\n", wt->head_ref);
 	}
+
+	reason = worktree_lock_reason(wt);
+	if (reason && *reason) {
+		struct strbuf sb = STRBUF_INIT;
+		quote_c_style(reason, &sb, NULL, CQUOTE_NODQ);
+		printf("locked %s\n", sb.buf);
+		strbuf_release(&sb);
+	} else if (reason)
+		printf("locked\n");
+
 	printf("\n");
 }
 
diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
index 795ddca2e4..c522190feb 100755
--- a/t/t2402-worktree-list.sh
+++ b/t/t2402-worktree-list.sh
@@ -66,11 +66,43 @@  test_expect_success '"list" all worktrees with locked annotation' '
 	git worktree add --detach locked master &&
 	git worktree add --detach unlocked master &&
 	git worktree lock locked &&
+	test_when_finished "git worktree unlock locked" &&
 	git worktree list >out &&
 	grep "/locked  *[0-9a-f].* locked$" out &&
 	! grep "/unlocked  *[0-9a-f].* locked$" out
 '
 
+test_expect_success '"list" all worktrees --porcelain with locked' '
+	test_when_finished "rm -rf locked1 locked2 unlocked out actual expect && git worktree prune" &&
+	echo "locked" >expect &&
+	echo "locked with reason" >>expect &&
+	git worktree add --detach locked1 &&
+	git worktree add --detach locked2 &&
+	git worktree add --detach unlocked &&
+	git worktree lock locked1 &&
+	git worktree lock locked2 --reason "with reason" &&
+	test_when_finished "git worktree unlock locked1 && git worktree unlock locked2" &&
+	git worktree list --porcelain >out &&
+	grep "^locked" out >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '"list" all worktrees --porcelain with locked reason newline escaped' '
+	test_when_finished "rm -rf locked_lf locked_crlf out actual expect && git worktree prune" &&
+	printf "locked locked\\\\r\\\\nreason\n" >expect &&
+	printf "locked locked\\\\nreason\n" >>expect &&
+	git worktree add --detach locked_lf &&
+	git worktree add --detach locked_crlf &&
+	printf "locked\nreason\n\n" >reason_lf &&
+	printf "locked\r\nreason\n\n" >reason_crlf &&
+	git worktree lock locked_lf --reason "$(cat reason_lf)" &&
+	git worktree lock locked_crlf --reason "$(cat reason_crlf)" &&
+	test_when_finished "git worktree unlock locked_lf && git worktree unlock locked_crlf" &&
+	git worktree list --porcelain >out &&
+	grep "^locked" out >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'bare repo setup' '
 	git init --bare bare1 &&
 	echo "data" >file1 &&