misleading message printed when worktree add fails

misleading message printed when worktree add fails

[Baruch Burstein]

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)

run `git worktree add <path> <branch>`, where <branch> is an already
checked-out branch

What did you expect to happen? (Expected behavior)

fail to create worktree

What happened instead? (Actual behavior)

worktree was not created, but a confusing message was printed:
"Preparing worktree (checking out '<branch>')"

What's different between what you expected and what actually happened?

The printed message seems to indicate that the command was a success,
when in fact
it was not

Anything else you want to add:

Please review the rest of the bug report below.
You can delete any lines you don't wish to share.


[System Info]
git version:
git version 2.33.1.windows.1
cpu: x86_64
built from commit: 05d80adb775077d673fab685009ede4a1003ed92
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
uname: Windows 10.0 18363
compiler info: gnuc: 10.3
libc info: no libc information available
$SHELL (typically, interactive shell): C:\Program Files\Git\usr\bin\bash.exe

Re: misleading message printed when worktree add fails

[Eric Sunshine]

Thanks for reporting. Inline comments below...

On Sun, Nov 28, 2021 at 1:53 AM Baruch Burstein <bmburstein@gmail.com> wrote:
> What did you do before the bug happened? (Steps to reproduce your issue)
>
> run `git worktree add <path> <branch>`, where <branch> is an already
> checked-out branch
>
> What happened instead? (Actual behavior)
>
> worktree was not created, but a confusing message was printed:
> "Preparing worktree (checking out '<branch>')"
>
> What's different between what you expected and what actually happened?
>
> The printed message seems to indicate that the command was a success,
> when in fact
> it was not

When I perform these actions, I see a "fatal error" message which
clearly indicates failure, not success:

    % git worktree add ../foo bar
    Preparing worktree (checking out 'bar')
    fatal: 'bar' is already checked out at '.../wherever'

Is your installation not showing the "fatal error" message?

Or, are you saying that the "Preparing worktree" message is
misleading? I read that message as telling me what the command is
trying to do (i.e. "Preparing"), and the error happens as it makes the
attempts, which seems logical to me, thus I don't see anything
misleading in that.

Are you reading it differently, as if it is saying that it already
performed that action (i.e. "Prepared")?

Re: misleading message printed when worktree add fails

[Baruch Burstein]

I see them in the other order:

    fatal: 'master' is already checked out at 'C:/Users/bmbur/temp'
    Preparing worktree (checking out 'master')

which I understood as "there was an error, but we managed to recover
somehow and set up the worktree anyway"


On Sun, Nov 28, 2021 at 2:37 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> Thanks for reporting. Inline comments below...
>
> On Sun, Nov 28, 2021 at 1:53 AM Baruch Burstein <bmburstein@gmail.com> wrote:
> > What did you do before the bug happened? (Steps to reproduce your issue)
> >
> > run `git worktree add <path> <branch>`, where <branch> is an already
> > checked-out branch
> >
> > What happened instead? (Actual behavior)
> >
> > worktree was not created, but a confusing message was printed:
> > "Preparing worktree (checking out '<branch>')"
> >
> > What's different between what you expected and what actually happened?
> >
> > The printed message seems to indicate that the command was a success,
> > when in fact
> > it was not
>
> When I perform these actions, I see a "fatal error" message which
> clearly indicates failure, not success:
>
>     % git worktree add ../foo bar
>     Preparing worktree (checking out 'bar')
>     fatal: 'bar' is already checked out at '.../wherever'
>
> Is your installation not showing the "fatal error" message?
>
> Or, are you saying that the "Preparing worktree" message is
> misleading? I read that message as telling me what the command is
> trying to do (i.e. "Preparing"), and the error happens as it makes the
> attempts, which seems logical to me, thus I don't see anything
> misleading in that.
>
> Are you reading it differently, as if it is saying that it already
> performed that action (i.e. "Prepared")?

Re: misleading message printed when worktree add fails

[Eric Sunshine]

[please comment inline on this list rather than top-posting]

On Sun, Nov 28, 2021 at 11:47 AM Baruch Burstein <bmburstein@gmail.com> wrote:
> > When I perform these actions, I see a "fatal error" message which
> > clearly indicates failure, not success:
> >
> >     % git worktree add ../foo bar
> >     Preparing worktree (checking out 'bar')
> >     fatal: 'bar' is already checked out at '.../wherever'
>
> I see them in the other order:
>
>     fatal: 'master' is already checked out at 'C:/Users/bmbur/temp'
>     Preparing worktree (checking out 'master')
>
> which I understood as "there was an error, but we managed to recover
> somehow and set up the worktree anyway"

Okay, that's happening because the "Preparing" message is sent to
stdout, whereas the "fatal" error is sent to stderr, and the streams
are being flushed on Windows in a different order than what we
typically see on Unix platforms even though the "Preparing" message is
actually output first by the code.

A general fix (not specific to git-worktree) might be to have die()
(or maybe vreportf()?) flush stdout before reporting the error message
on stderr. That should make output order more predictable, such that
general status messages appear before error messages.

RE: misleading message printed when worktree add fails

[rsbecker]

On November 28, 2021 12:01 PM, Eric Sunshine wrote:
> On Sun, Nov 28, 2021 at 11:47 AM Baruch Burstein <bmburstein@gmail.com>
> wrote:
> > > When I perform these actions, I see a "fatal error" message which
> > > clearly indicates failure, not success:
> > >
> > >     % git worktree add ../foo bar
> > >     Preparing worktree (checking out 'bar')
> > >     fatal: 'bar' is already checked out at '.../wherever'
> >
> > I see them in the other order:
> >
> >     fatal: 'master' is already checked out at 'C:/Users/bmbur/temp'
> >     Preparing worktree (checking out 'master')
> >
> > which I understood as "there was an error, but we managed to recover
> > somehow and set up the worktree anyway"
> 
> Okay, that's happening because the "Preparing" message is sent to stdout,
> whereas the "fatal" error is sent to stderr, and the streams are being flushed
> on Windows in a different order than what we typically see on Unix
> platforms even though the "Preparing" message is actually output first by the
> code.
> 
> A general fix (not specific to git-worktree) might be to have die() (or maybe
> vreportf()?) flush stdout before reporting the error message on stderr. That
> should make output order more predictable, such that general status
> messages appear before error messages.

This is probably a good idea. There is no guarantee even in Unix regarding stream buffer flushing. If there is a reason to sequence output between stdout and stderr, then fflush is important.

-Randall

Re: misleading message printed when worktree add fails

[Eric Sunshine]

On Sun, Nov 28, 2021 at 12:00 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> Okay, that's happening because the "Preparing" message is sent to
> stdout, whereas the "fatal" error is sent to stderr, and the streams
> are being flushed on Windows in a different order than what we
> typically see on Unix platforms even though the "Preparing" message is
> actually output first by the code.
>
> A general fix (not specific to git-worktree) might be to have die()
> (or maybe vreportf()?) flush stdout before reporting the error message
> on stderr. That should make output order more predictable, such that
> general status messages appear before error messages.

An RFC patch has been posted to address this issue.
https://lore.kernel.org/git/20211130043946.19987-1-sunshine@sunshineco.com/