git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Erik Faye-Lund <kusmabite@gmail.com>
Cc: git@vger.kernel.org, steveire@gmail.com,
	felipe.contreras@gmail.com, gitster@pobox.com
Subject: Re: [PATCH] only warn about ambiguous refs if stderr is a tty
Date: Mon, 9 May 2011 06:32:08 -0400	[thread overview]
Message-ID: <20110509103208.GA9060@sigill.intra.peff.net> (raw)
In-Reply-To: <BANLkTimR_S-px-MfRy0pKGrjxOgSC_=e=A@mail.gmail.com>

On Mon, May 09, 2011 at 10:41:02AM +0200, Erik Faye-Lund wrote:

> > I disagree. If I do:
> >
> >  git foo 2>errors
> >
> > I would certainly expect any relevant errors to end up in that file. As
> > for why I would do that, two cases I can think of offhand are:
> >
> >  1. Test scripts, which use this extensively.
> >
> >  2. Sometimes cron jobs will capture chatty output in a file and show
> >     it only in the case of some error condition.
> >
> 
> I was talking about warnings, not errors. But I can also see that one
> would sometimes want warnings even when not connected to a tty, but
> perhaps only when -v is specified?

I know. I meant a script like this:

  cat >>foo.sh <<'EOF'
  # go to branch in question
  git checkout "$1"

  # note some point of interest
  sha1=`git rev-parse "$2"`

  # do some script-specific inspection of $sha1, and
  # merge if it looks OK
  if test -z "$(git log ..$sha1 -- some-path)"; then
    git merge $sha1 || exit 1
  fi
  EOF

It may produce some chatty output (like "switched to branch..."). So I
redirect it to a file, and if everything is successful, that output is
uninteresting. But if it fails, then I want to see everything. So I do
something like:

  if ! foo.sh master topic >output.tmp 2>&1; then
    cat output.tmp
    exit 1
  fi

If the merge fails, it will produce an error message. But I _also_ want
to see any warnings that were generated by it and earlier commands, like
rev-parse (e.g., an ambiguous ref warning might help us understand why
the merge failed).

Obviously this is a pretty trivial example that I cooked up for this
email. But the concept of stash-stderr-and-report-on-error is a pretty
common pattern for cron jobs.

> > Yeah, if we disambiguate, I would be tempted to say that "HEAD" always
> > unambiguously refers to "HEAD".
> 
> While that would touch less code, my gut tells me it's a bit more
> fragile. But perhaps you're right; I can't come up with any real
> arguments (i.e use cases that I care about) on top of my head.

Honestly, I'm kind of surprised it's not that way already. It would make
sense to me that "upper" levels would take precedence over lower levels,
but that ambiguity would occur within a level. So if I say "foo", we
would look for:

  1. $GIT_DIR/foo, with no ambiguity

  2. $GIT_DIR/refs/foo, with no ambiguity

  3. $GIT_DIR/refs/tags/foo
     $GIT_DIR/refs/heads/foo
     $GIT_DIR/refs/remotes/foo

     And note any ambiguity between those three.

Which is not very different than what we do today, except that things
like HEAD and FETCH_HEAD would always be unambiguously about the
top-level.

> > And "refs/HEAD" should already work, no?
> 
> No:
> $ git init foo
> $ cd foo/
> $ echo "foo" > bar
> $ git add bar
> $ git commit -m.
> [master (root-commit) fc0cbef] .
> warning: LF will be replaced by CRLF in bar.
> The file will have its original line endings in your working directory.
>  1 files changed, 1 insertions(+), 0 deletions(-)
>  create mode 100644 bar
> $ git show refs/HEAD
> fatal: ambiguous argument 'refs/HEAD': unknown revision or path not in
> the working tree.
> Use '--' to separate paths from revisions

Of course, because there is no refs/HEAD at all. I meant "if you have
ambiguity between $GIT_DIR/HEAD and $GIT_DIR/refs/HEAD", then saying
"refs/HEAD" should disambiguate already. In your example, there is no
ambiguity.

What I failed to notice is that the likely disambiguator is actually
"refs/heads/HEAD" if you erroneously made a branch.

Try this:

  # A repo with two commits
  git init repo && cd repo &&
  echo content >file &&
  git add file &&
  git commit -m one &&
  echo content >>file &&
  git commit -a -m two &&

  # And an ambiguously named ref called HEAD, pointing to "one";
  # our real HEAD is still pointing to "two"
  git branch HEAD HEAD^ &&

  # This should warn of ambiguity, but show "two"
  git log -1 --oneline HEAD

  # And this should not be ambiguous at all, and show "one"
  git log -1 --oneline refs/heads/HEAD

  # You can even do the same thing with refs/HEAD if you want, but
  # you have to use plumbing to get such a ref.
  git branch -d HEAD
  git update-ref refs/HEAD HEAD^

  # same as before, ambiguous "two"
  git log -1 --oneline HEAD

  # or we can use refs/HEAD to get "one"
  git log -1 --oneline refs/HEAD

So most of that makes sense to me. We choose $GIT_DIR/HEAD over other
options, and you can specifically refer to something further down by
its fully-qualified name.

The only thing that I think we might want to change is that "HEAD" is
considered ambiguous with "refs/heads/HEAD". On the other hand, it seems
a little insane to name your branch that, given that it has a
well-established meaning in git. I admit I haven't been following this
thread too closely. What is the reason not to tell the user "sorry, that
is an insane branch name. Accept the ambiguity warning, or choose a
different name"?

-Peff

  reply	other threads:[~2011-05-09 10:32 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-17 10:02 Creating remote branch called HEAD corrupts remote clones Stephen Kelly
2011-01-20 11:14 ` Stephen Kelly
2011-01-20 13:03   ` Thomas Rast
2011-01-20 15:05     ` Stephen Kelly
2011-01-20 15:41       ` Erik Faye-Lund
2011-01-20 16:00         ` Stephen Kelly
2011-01-20 17:32   ` Felipe Contreras
2011-01-20 19:21     ` Wesley J. Landaker
2011-01-20 19:53 ` Junio C Hamano
2011-01-20 20:38   ` Jeff King
2011-01-20 21:43     ` Junio C Hamano
2011-01-20 21:54       ` Jeff King
2011-01-20 23:52         ` Felipe Contreras
2011-01-21 17:37           ` Junio C Hamano
2011-01-22 12:46             ` Felipe Contreras
2011-02-20 13:17               ` Stephen Kelly
2011-04-26 12:09                 ` Stephen Kelly
2011-04-26 18:18                   ` Felipe Contreras
2011-04-27  9:18                     ` Stephen Kelly
2011-04-27  9:48                       ` Felipe Contreras
2011-04-27 11:29                         ` Stephen Kelly
2011-04-27 11:32                           ` Felipe Contreras
2011-04-27 11:37                             ` Stephen Kelly
2011-04-27 12:26                               ` Felipe Contreras
2011-04-27 12:21                           ` Erik Faye-Lund
2011-04-27 12:49                             ` Erik Faye-Lund
2011-05-02 19:26                               ` Stephen Kelly
2011-05-02 19:43                                 ` Erik Faye-Lund
2011-05-03 17:54                                   ` Felipe Contreras
2011-05-03 18:08                                     ` Stephen Kelly
2011-05-03 19:20                                       ` Felipe Contreras
2011-05-04 12:35                                     ` Erik Faye-Lund
2011-05-09  7:51                                       ` [PATCH] only warn about ambiguous refs if stderr is a tty Erik Faye-Lund
2011-05-09  8:03                                         ` Jeff King
2011-05-09  8:41                                           ` Erik Faye-Lund
2011-05-09 10:32                                             ` Jeff King [this message]
2011-05-09 12:37                                               ` Erik Faye-Lund
2011-05-09 12:49                                                 ` Jeff King
2011-05-09 16:33                                                   ` Junio C Hamano
2011-05-09 22:09                                                     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110509103208.GA9060@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kusmabite@gmail.com \
    --cc=steveire@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).