git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 0/7] teach `worktree list` verbose mode and prunable annotations

From
Rafael Silva <rafaeloliveira.cs@gmail.com>
Date
Jan 27, 2021, 08:03 UTC
Message-ID
<20210127080310.89639-1-rafaeloliveira.cs@gmail.com>
In-Reply-To
<20210119212739.77882-1-rafaeloliveira.cs@gmail.com>

In c57b3367be (worktree: teach `list` to annotate locked worktree, 2020-10-11) we taught `git worktree list` to annotate working trees that is locked by appending "locked" text in order to signalize to the user that a working tree is locked. During the review, there was some discussion about additional annotations and information that `list` command could provide to the user that has long been envisioned and mentioned in [2], [3] and [4].

This patch series addresses some of these changes by teaching `worktree list` to show "prunable" annotation, adding verbose mode and extending the --porcelain format with prunable and locked annotation as follow up from [1]. Additionally, it addresses one shortcoming for porcelain format to escape any newline characters (LF or CRLF) and quote the lock reason to prevent breaking the format that is mentioned in [4] and [1] during the review cycle.

This patch series includes:
1. The first patch moves the should_prune_worktree() machinery to the top-level
   worktree.c exposing the function as general API, that will be reference
   by should_prune_worktree() wrapper implemented on the second patch.
   The original idea was to not only move should_prune_worktree() but also
   refactor to accept a "struct worktree" and load the information directly,
   which allows simplify the `prune` command by reusing get_worktrees().
   However this seems to also require refactoring get_worktrees() itself
   to return "non-valid" working trees that can/should be pruned. This is
   also mentioned in [5]. Having the wrapper function makes it easier to add
   the prunable annotation without touching the get_worktrees() and the
   other worktree sub commands. The refactoring can be addressed in a
   future patch, if this turns out to be good idea. One possible approach
   is to teach get_worktrees() to take additional flags that will tell
   whether to return only valid or all worktrees in GIT_DIR/worktrees/
   directory and address its own possible shortcoming.
2. Introduces the worktree_prune_reason() to discovery whether a worktree
   is prunable along with two new fields in the `worktree` structure. The
   function mimics the workree_lock_reason() API.
3. The third patch changes worktree_lock_reason() to be more gentle for
   the main working tree to simply returning NULL instead of aborting the
   program via assert() macro. This allow us to simplify the code that
   checks if the working tree is locked for default and porcelain format.
   This changes is also mentioned in [6].
4. Fix t2402 added in [1] to ensure the locked worktree is properly cleaned up.
5. The fourth patch adds the "locked" attribute for the porcelain format
   in order to make both default and --porcelain format consistent.
6. The fifth patch introduces "prunable" annotation for both default
   and --porcelain format.
7. The sixth patch introduces verbose mode to expand the `list` default
   format and show each annotation reason when its available.
Series is built on top of 66e871b664 (The third batch, 2021-01-15).

[1] https://lore.kernel.org/git/20200928154953.30396-1-rafaeloliveira.cs@gmail.com/ [2] https://lore.kernel.org/git/CAPig+cQF6V8HNdMX5AZbmz3_w2WhSfA4SFfNhQqxXBqPXTZL+w@mail.gmail.com/ [3] https://lore.kernel.org/git/CAPig+cSGXqJuaZPhUhOVX5X=LMrjVfv8ye_6ncMUbyKox1i7QA@mail.gmail.com/ [4] https://lore.kernel.org/git/CAPig+cTitWCs5vB=0iXuUyEY22c0gvjXvY1ZtTT90s74ydhE=A@mail.gmail.com/ [5] https://lore.kernel.org/git/CACsJy8ChM99n6skQCv-GmFiod19mnwwH4j-6R+cfZSiVFAxjgA@mail.gmail.com/ [6] https://lore.kernel.org/git/xmqq8sctlgzx.fsf@gitster.c.googlers.com/

Changes in v4 =============

Thanks Eric Sunshine and Phillip Wood for the review and suggestions.
 * Added documentation to explain that the lock reason is quoted with
   the same rules as described for `core.quotePath`.
 * The `worktree unlock` issued in the test cleanup is splitted and
   executed after each `worktree lock` to ensure the unlock is only
   done after we know each locked command was successful.  
 * Fix a couple of grammos in the [4/7] commit message.

Changes in v3 =============

v3 is 1-patch bigger than v2 and it includes the changes:
 * Dropped CQUOTE_NODQ flag in the the locked reason to return a string
   enclosed with double quotes if the text reason contains especial
   characters, like newlines.
 * fix in t2402 to ensure the locked worktree is properly cleaned up,
   is move to its own patch.
 * In worktree_prune_reason(), the `path` variable is initialize
   with NULL to make the code easier to follow.
 * The removal of `is_main_worktree()` before `worktree_lock_reason()`
   is moved to the patch that actually changes the API to be more gentle
   with the main worktree instead of refactoring the code in the next
   patches that adds the annotations.
 * Drop the `*reason` in `(reason && *reason)` as we know that
   worktree_prune_reason() is either going to return a non-empty string
   or NULL which makes the code easier to follow.
 * The --verbose test for the locked annotation now tests that "locked"
   annotation stays on the same line when there is no locked reason.
 * Small documentation updates and make the "git worktree --verbose"
   example a little consistent with the worktree path.

Changes in v2 =============

v2 changes considerably from v1 taking into account several comments suggested by Eric Sunshine and Phillip Wood (thanks guys for the insightful comments and patience reviewing my patches :) ).

 * The biggest change are the way the series is organized. In v1 all the
   code was implemented in the fourth patch, all the tests and documentation
   updates was added in sixth and seventh patch respectfully, in v2 each
   patch introduces the annotations and verbose mode together with theirs
   respective test and documentation updates.
 * Several rewrite of the commit messages
 * In v1 the fifth patch was introducing a new function to escape newline
   characters for the "locked" attribute. However, we already have this
   feature from the quote.h API. So the new function is dropped and the
   changes are squashed into v2's fourth patch.
 * The new prunable annotation and locked annotation that was introduced
   by [1] was refactor to not poke around the worktree private fields.
 * Refactoring of the worktree_prune_reason() to cache the prune_reason
   and refactor to early return the use cases following the more common
   pattern used on Git's codebase.
 * Few documentation rewrites, most notably the `--verbose` and `--expire`
   doc sentences for the list command are moved to its own line to clearly
   separate the description from the others commands instead of continuing 
   on the same paragraph.
 * The `git unlock <worktree>` used in the test's cleanup is moved to after
   we know the `git worktree locked` is executed successfully. This ensures
   the unlock doesn't error in the cleanup stage which will make it harder
   to debug the tests.
Rafael Silva (7):
  worktree: libify should_prune_worktree()
  worktree: teach worktree to lazy-load "prunable" reason
  worktree: teach worktree_lock_reason() to gently handle main worktree
  t2402: ensure locked worktree is properly cleaned up
  worktree: teach `list --porcelain` to annotate locked worktree
  worktree: teach `list` to annotate prunable worktree
  worktree: teach `list` verbose mode
 Documentation/git-worktree.txt |  74 ++++++++++++++++++++--
 builtin/worktree.c             | 110 +++++++++++----------------------
 t/t2402-worktree-list.sh       |  96 ++++++++++++++++++++++++++++
 worktree.c                     |  91 ++++++++++++++++++++++++++-
 worktree.h                     |  23 +++++++
 5 files changed, 314 insertions(+), 80 deletions(-)
Range-diff against v3:
1:  fc4252c129 = 1:  66cd83ba42 worktree: libify should_prune_worktree()
2:  664d84155b = 2:  81f4824028 worktree: teach worktree to lazy-load "prunable" reason
3:  cf334da2f0 = 3:  c62ecf60d6 worktree: teach worktree_lock_reason() to gently handle main worktree
4:  ecb11793d9 ! 4:  d2ea467927 t2402: ensure locked worktree is properly cleaned up
    @@ Metadata
      ## Commit message ##
         t2402: ensure locked worktree is properly cleaned up
     
    -    In c57b3367be (worktree: teach `list` to annotate locked worktree,
    +    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
    +    also potentially breaks following tests that rely on the
         "git worktree list" output.
     
         Let's fix that by unlocking the worktree before the "prune" command.
5:  1fb68562f6 ! 5:  1b7661a0b3 worktree: teach `list --porcelain` to annotate locked worktree
    @@ Commit message
         with its value depending whether the value is available or not. Thus
         documenting the case of the new "locked" attribute.
     
    +    Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
         Helped-by: Eric Sunshine <sunshine@sunshineco.com>
         Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com>
     
    @@ Documentation/git-worktree.txt: worktree /path/to/other-linked-worktree
     +branch refs/heads/locked-with-reason
     +locked reason why is locked
     +
    ++------------
    ++
    ++If the lock reason contains "unusual" characters such as newline, they
    ++are escaped and the entire reason is quoted as explained for the
    ++configuration variable `core.quotePath` (see linkgit:git-config[1]).
    ++For Example:
    ++
    ++------------
    ++$ git worktree list --porcelain
    ++...
    ++locked "reason\nwhy is locked"
    ++...
      ------------
      
      EXAMPLES
    @@ t/t2402-worktree-list.sh: test_expect_success '"list" all worktrees with locked
     +	# unlocked worktree should not be annotated with "locked"
     +	git worktree add --detach unlocked &&
     +	git worktree lock locked1 &&
    ++	test_when_finished "git worktree unlock locked1" &&
     +	git worktree lock locked2 --reason "with reason" &&
    -+	test_when_finished "git worktree unlock locked1 && git worktree unlock locked2" &&
    ++	test_when_finished "git worktree unlock locked2" &&
     +	git worktree list --porcelain >out &&
     +	grep "^locked" out >actual &&
     +	test_cmp expect actual
    @@ t/t2402-worktree-list.sh: test_expect_success '"list" all worktrees with locked
     +	git worktree add --detach locked_lf &&
     +	git worktree add --detach locked_crlf &&
     +	git worktree lock locked_lf --reason "$(printf "locked\nreason")" &&
    ++	test_when_finished "git worktree unlock locked_lf" &&
     +	git worktree lock locked_crlf --reason "$(printf "locked\r\nreason")" &&
    -+	test_when_finished "git worktree unlock locked_lf && git worktree unlock locked_crlf" &&
    ++	test_when_finished "git worktree unlock locked_crlf" &&
     +	git worktree list --porcelain >out &&
     +	grep "^locked" out >actual &&
     +	test_cmp expect actual
6:  834584f826 = 6:  c29ad7ffb1 worktree: teach `list` to annotate prunable worktree
7:  35ad456fbb ! 7:  7938950d04 worktree: teach `list` verbose mode
    @@ t/t2402-worktree-list.sh: test_expect_success '"list" all worktrees with prunabl
     +	git worktree add locked1 --detach &&
     +	git worktree add locked2 --detach &&
     +	git worktree lock locked1 &&
    ++	test_when_finished "git worktree unlock locked1" &&
     +	git worktree lock locked2 --reason "with reason" &&
    -+	test_when_finished "git worktree unlock locked1 && git worktree unlock locked2" &&
    ++	test_when_finished "git worktree unlock locked2" &&
     +	echo "$(git -C locked2 rev-parse --show-toplevel) $(git rev-parse --short HEAD) (detached HEAD)" >expect &&
     +	printf "\tlocked: with reason\n" >>expect &&
     +	git worktree list --verbose >out &&
-- 
2.30.0.373.geade8fefe8
Previous: Rafael SilvaNext: Rafael Silva
Message 78 of 88 in “teach `worktree list` verbose mode and prunable annotations”
  1. 0/7 teach `worktree list` verbose mode and prunable annotationsRafael Silva, Jan 4, 2021
  2. 3/7 worktree: teach worktree_lock_reason() to gently handle main worktreeRafael Silva, Jan 4, 2021
  3. Eric SunshineJan 6, 2021
  4. Rafael SilvaJan 8, 2021
  5. 6/7 worktree: add tests for `list` verbose and annotationsRafael Silva, Jan 4, 2021
  6. Eric SunshineJan 6, 2021
  7. Eric SunshineJan 7, 2021
  8. Rafael SilvaJan 8, 2021
  9. 4/7 worktree: teach `list` prunable annotation and verboseRafael Silva, Jan 4, 2021
  10. Eric SunshineJan 6, 2021
  11. Rafael SilvaJan 8, 2021
  12. 7/7 worktree: document `list` verbose and prunable annotationsRafael Silva, Jan 4, 2021
  13. Eric SunshineJan 6, 2021
  14. Rafael SilvaJan 8, 2021
  15. 5/7 worktree: `list` escape lock reason in --porcelainRafael Silva, Jan 4, 2021
  16. Phillip WoodJan 5, 2021
  17. worktree: add -z option for list subcommandPhillip Wood, Jan 5, 2021
  18. Eric SunshineJan 7, 2021
  19. Phillip WoodJan 8, 2021
  20. Eric SunshineJan 10, 2021
  21. Eric SunshineJan 6, 2021
  22. Rafael SilvaJan 8, 2021
  23. Eric SunshineJan 6, 2021
  24. 1/7 worktree: move should_prune_worktree() to worktree.cRafael Silva, Jan 4, 2021
  25. Eric SunshineJan 6, 2021
  26. Rafael SilvaJan 8, 2021
  27. Eric SunshineJan 6, 2021
  28. Eric SunshineJan 7, 2021
  29. Rafael SilvaJan 8, 2021
  30. 2/7 worktree: implement worktree_prune_reason() wrapperRafael Silva, Jan 4, 2021
  31. Eric SunshineJan 6, 2021
  32. Rafael SilvaJan 8, 2021
  33. Eric SunshineJan 6, 2021
  34. Rafael SilvaJan 8, 2021
  35. Eric SunshineJan 8, 2021
  36. 0/6 teach `worktree list` verbose mode and prunable annotationsRafael Silva, Jan 17, 2021
  37. 1/6 worktree: libify should_prune_worktree()Rafael Silva, Jan 17, 2021
  38. 4/6 worktree: teach `list --porcelain` to annotate locked worktreeRafael Silva, Jan 17, 2021
  39. Eric SunshineJan 18, 2021
  40. Rafael SilvaJan 19, 2021
  41. Eric SunshineJan 19, 2021
  42. 5/6 worktree: teach `list` to annotate prunable worktreeRafael Silva, Jan 17, 2021
  43. Eric SunshineJan 18, 2021
  44. Rafael SilvaJan 19, 2021
  45. Eric SunshineJan 19, 2021
  46. 6/6 worktree: teach `list` verbose modeRafael Silva, Jan 17, 2021
  47. Eric SunshineJan 18, 2021
  48. Eric SunshineJan 18, 2021
  49. 2/6 worktree: teach worktree to lazy-load "prunable" reasonRafael Silva, Jan 17, 2021
  50. Eric SunshineJan 18, 2021
  51. Rafael SilvaJan 19, 2021
  52. 3/6 worktree: teach worktree_lock_reason() to gently handle main worktreeRafael Silva, Jan 17, 2021
  53. Eric SunshineJan 18, 2021
  54. Rafael SilvaJan 19, 2021
  55. 0/7 teach `worktree list` verbose mode and prunable annotationsRafael Silva, Jan 19, 2021
  56. 1/7 worktree: libify should_prune_worktree()Rafael Silva, Jan 19, 2021
  57. 7/7 worktree: teach `list` verbose modeRafael Silva, Jan 19, 2021
  58. Eric SunshineJan 24, 2021
  59. Rafael SilvaJan 24, 2021
  60. 2/7 worktree: teach worktree to lazy-load "prunable" reasonRafael Silva, Jan 19, 2021
  61. 6/7 worktree: teach `list` to annotate prunable worktreeRafael Silva, Jan 19, 2021
  62. Junio C HamanoJan 21, 2021
  63. Rafael SilvaJan 21, 2021
  64. Junio C HamanoJan 21, 2021
  65. 3/7 worktree: teach worktree_lock_reason() to gently handle main worktreeRafael Silva, Jan 19, 2021
  66. 4/7 t2402: ensure locked worktree is properly cleaned upRafael Silva, Jan 19, 2021
  67. Eric SunshineJan 24, 2021
  68. Rafael SilvaJan 24, 2021
  69. 5/7 worktree: teach `list --porcelain` to annotate locked worktreeRafael Silva, Jan 19, 2021
  70. Phillip WoodJan 20, 2021
  71. Junio C HamanoJan 21, 2021
  72. Rafael SilvaJan 21, 2021
  73. Eric SunshineJan 24, 2021
  74. Eric SunshineJan 24, 2021
  75. Rafael SilvaJan 24, 2021
  76. Eric SunshineJan 24, 2021
  77. Rafael SilvaJan 27, 2021
  78. 0/7 teach `worktree list` verbose mode and prunable annotationsRafael Silva, Jan 27, 2021
  79. 2/7 worktree: teach worktree to lazy-load "prunable" reasonRafael Silva, Jan 27, 2021
  80. 6/7 worktree: teach `list` to annotate prunable worktreeRafael Silva, Jan 27, 2021
  81. 5/7 worktree: teach `list --porcelain` to annotate locked worktreeRafael Silva, Jan 27, 2021
  82. 7/7 worktree: teach `list` verbose modeRafael Silva, Jan 27, 2021
  83. 3/7 worktree: teach worktree_lock_reason() to gently handle main worktreeRafael Silva, Jan 27, 2021
  84. 1/7 worktree: libify should_prune_worktree()Rafael Silva, Jan 27, 2021
  85. 4/7 t2402: ensure locked worktree is properly cleaned upRafael Silva, Jan 27, 2021
  86. Eric SunshineJan 30, 2021
  87. Rafael SilvaJan 30, 2021
  88. Junio C HamanoJan 30, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.