Re: [PATCH 0/3] Minor fixes on 'git worktree'

3 messages, 2 authors, 2016-11-25 · open the first message on its own page

Re: [PATCH 0/3] Minor fixes on 'git worktree'

From: Junio C Hamano <hidden>
Date: 2016-11-23 16:52:25

Nguyễn Thái Ngọc Duy  [off-list ref] writes:
This fixes two things:

 - make sure the first item is always the main worktree even if we
   fail to retrieve some info

 - keep 'worktree list' order stable (which in turn fixes the random
   failure on my 'worktree-move' series
Nguyễn Thái Ngọc Duy (3):
  worktree.c: zero new 'struct worktree' on allocation
  get_worktrees() must return main worktree as first item even on error
  worktree list: keep the list sorted

 builtin/worktree.c | 26 ++++++++++++++++++++++----
 worktree.c         | 20 ++++----------------
 2 files changed, 26 insertions(+), 20 deletions(-)
Any tests?

Re: [PATCH 0/3] Minor fixes on 'git worktree'

From: Duy Nguyen <hidden>
Date: 2016-11-25 12:25:21

On Wed, Nov 23, 2016 at 11:52 PM, Junio C Hamano [off-list ref] wrote:
Nguyễn Thái Ngọc Duy  [off-list ref] writes:
quoted
This fixes two things:

 - make sure the first item is always the main worktree even if we
   fail to retrieve some info

 - keep 'worktree list' order stable (which in turn fixes the random
   failure on my 'worktree-move' series
Nguyễn Thái Ngọc Duy (3):
  worktree.c: zero new 'struct worktree' on allocation
  get_worktrees() must return main worktree as first item even on error
  worktree list: keep the list sorted

 builtin/worktree.c | 26 ++++++++++++++++++++++----
 worktree.c         | 20 ++++----------------
 2 files changed, 26 insertions(+), 20 deletions(-)
Any tests?
I discarded the idea of adding test for sorting because it relies on
filesystems. A passed test may just mean the filesystem happens to
return files in "good" order. But I guess a test wouldn't hurt.
Somewhere out there some user may still have a "bad" filesystem and
the test could help catch breakage then.

Adding the test for the failed parse_ref() is possible, I think. But
since that function is destined to die, as I promised to use
refs-provided api instead of rolling out a custom ref parser, and I'm
going to have another hard look at refs subsystem for the
gc-not-looking-at-worktree-refs problem, may I make another promise to
add tests after this function is gone? It should happen "soon".
-- 
Duy

Re: [PATCH 0/3] Minor fixes on 'git worktree'

From: Duy Nguyen <hidden>
Date: 2016-11-25 13:45:54

On Fri, Nov 25, 2016 at 7:24 PM, Duy Nguyen [off-list ref] wrote:
Adding the test for the failed parse_ref() is possible, I think. But
since that function is destined to die, as I promised to use
refs-provided api instead of rolling out a custom ref parser, and I'm
going to have another hard look at refs subsystem for the
gc-not-looking-at-worktree-refs problem, may I make another promise to
add tests after this function is gone?
Never mind. I went ahead and made a test case anyway because why not.

But that's not worth an email. An interesting thing maybe worth
sharing is, if HEAD is broken (the only reason we would fail to create
"struct worktree" for main worktree), then "git worktree list" from
main worktree would fail too, because repo setup code fails to parse
HEAD as well and die(). That makes the "always show main worktree"
patch useless, since we won't get far enough to execute
get_worktrees() in the first place. However, you can still do "git
worktree list" from a linked worktree, gitdir setup code will not stop
you (how can it). And the patch is still needed.
-- 
Duy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help