All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Junio Hamano C <gitster@pobox.com>,
	Git List Mailing <git@vger.kernel.org>
Subject: Re: "git symbolic-ref" doesn't do a very good job
Date: Mon, 1 Aug 2022 13:43:06 -0400	[thread overview]
Message-ID: <YugQqp4oN26OFOpt@coredump.intra.peff.net> (raw)
In-Reply-To: <CAHk-=wi5pfUcuaAUz=rifon9d51mshE7k6bkpMXddog0On9jow@mail.gmail.com>

On Sat, Jul 30, 2022 at 05:44:25PM -0700, Linus Torvalds wrote:

> Put another way: I think my patch is likely the right thing to do (and
> I'd personally prefer the stricter check without the ALLOW_ONELEVEL
> flag), but you and Junio are right about it being a bigger change than
> I in my naivete thought it was.
> 
> So I won't really push for this, I suspect this needs very much to be
> a judgement call by you guys.

Just to lay out the options, I think we have:

  1. Do nothing. This breaks nothing. ;)

  2. Your patch, but with ALLOW_ONELEVEL. This fixes nonsense like
     "foo..bar", but doesn't break "FETCH_HEAD". Requires fixing t4202's
     ".lock" example. Replaces the HEAD starts_with("refs/") check.

  3. Your patch as-is. Same as (2), but also breaks FETCH_HEAD.

  4. Your patch, plus any extra tightening of HEAD to refs/heads/. I
     think this is probably breaking too much (I put more details
     elsewhere in the thread).

I'd be in favor of (2), which is really just catching syntactically
invalid crap, and shouldn't break anyone. Technically it's possible
somebody could be using a symref pointing at arbitrary data for
who-knows-what reason, and extracting it with "symbolic-ref", but that
is getting beyond far-fetched, I think.

I'm also tempted by (3), but we should be prepared for obscure breakage
reports.

-Peff

  reply	other threads:[~2022-08-01 17:43 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-30 19:53 "git symbolic-ref" doesn't do a very good job Linus Torvalds
2022-07-30 20:21 ` Linus Torvalds
2022-07-30 20:38   ` Junio C Hamano
2022-07-31  0:18   ` Jeff King
2022-07-31  0:24     ` Jeff King
2022-07-31  0:44       ` Linus Torvalds
2022-08-01 17:43         ` Jeff King [this message]
2022-08-01 17:46           ` Jeff King
2022-08-01 18:15           ` Jeff King
2022-08-01 18:54             ` Junio C Hamano
2022-08-02  0:46               ` Jeff King
2022-08-02  0:57                 ` Junio C Hamano
2022-08-01 19:00             ` Linus Torvalds
2022-07-31 19:10     ` Junio C Hamano
2022-08-01 17:36       ` Jeff King
2022-08-01 17:49         ` Linus Torvalds
2022-08-01 18:04           ` 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=YugQqp4oN26OFOpt@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=torvalds@linux-foundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.