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

Re: [BUG] branch renamed to 'HEAD'

From
Jacob Keller <jacob.keller@gmail.com>
Date
Feb 28, 2017, 22:48 UTC
Message-ID
<CA+P7+xpfRjr4BKkLLcbCS-yXXYoiHeWr9Y9ubHA-Xxqa9c7q8w@mail.gmail.com>
In-Reply-To
<20170228120633.zkwfqms57fk7dkl5@sigill.intra.peff.net>
On Tue, Feb 28, 2017 at 4:06 AM, Jeff King <peff@peff.net> wrote:
Show 67 quoted lines
> On Mon, Feb 27, 2017 at 07:53:02PM -0500, Jeff King wrote:
>
>> On Mon, Feb 27, 2017 at 04:33:36PM -0800, Junio C Hamano wrote:
>>
>> > A flag to affect the behaviour (as opposed to &flag as a secondary
>> > return value, like Peff's patch does) can be made to work.  Perhaps
>> > a flag that says "keep the input as is if the result is not a local
>> > branch name" would pass an input "@" intact and that may be
>> > sufficient to allow "git branch -m @" to rename the current branch
>> > to "@" (I do not think it is a sensible rename, though ;-).  But
>> > probably some callers need to keep the original input and compare
>> > with the result to see if we expanded anything if we go that route.
>> > At that point, I am not sure if there are much differences in the
>> > ease of use between the two approaches.
>>
>> I just went into more detail in my reply to Jacob, but I do think this
>> is a workable approach (and fortunately we seem to have banned bare "@"
>> as a name, along with anything containing "@{}", so I think we would end
>> up rejecting these nonsense names).
>>
>> I'll see if I can work up a patch. We'll still need to pass the flag
>> around through the various functions, but at least it will be a flag and
>> not a confusing negated out-parameter.
>
> OK, I have a series which fixes this (diffstat below). When I audited
> the other callers of interpret_branch_name() and strbuf_branchname(), it
> turned out to be even more complicated. The callers basically fall into
> a few buckets:
>
>   1. Callers like get_sha1() and merge_name() pass the result to
>      dwim_ref(), and are prepared to handle anything.
>
>   2. Some callers stick "refs/heads/" in front of the result, and
>      obviously only want local names. Most of git-branch and
>      git-checkout fall into this boat.
>
>   3. "git branch -d" can delete local _or_ remote branches, depending on
>      the "-r" flag. So the expansion it wants varies, and we need to
>      handle "just local" or "just remote".
>
> So I converted the "only_branch" flag to an "allowed" bit-field. No
> callers actually ask for more than a single type at once, but it was
> easy to do it that way. It serves all of the callers, and will easily
> adapt for the future (e.g., if "git branch -a -d" were ever allowed).
>
>   [1/8]: interpret_branch_name: move docstring to header file
>   [2/8]: strbuf_branchname: drop return value
>   [3/8]: strbuf_branchname: add docstring
>   [4/8]: interpret_branch_name: allow callers to restrict expansions
>   [5/8]: t3204: test git-branch @-expansion corner cases
>   [6/8]: branch: restrict @-expansions when deleting
>   [7/8]: strbuf_check_ref_format(): expand only local branches
>   [8/8]: checkout: restrict @-expansions when finding branch
>
>  builtin/branch.c                      |   5 +-
>  builtin/checkout.c                    |   2 +-
>  builtin/merge.c                       |   2 +-
>  cache.h                               |  32 ++++++++-
>  refs.c                                |   2 +-
>  revision.c                            |   2 +-
>  sha1_name.c                           |  76 ++++++++++-----------
>  strbuf.h                              |  21 +++++-
>  t/t3204-branch-name-interpretation.sh | 122 ++++++++++++++++++++++++++++++++++
>  9 files changed, 220 insertions(+), 44 deletions(-)
>  create mode 100755 t/t3204-branch-name-interpretation.sh
>
> -Peff

I didn't find any problems besides what you had already outlined before I started reading the series. It looks pretty much like I thought it would. I like the idea of saying "I want X" rather than the command returning "This was a Y"

Previous: Jeff KingNext: Junio C Hamano
Message 14 of 15 in “Re: [BUG] branch renamed to 'HEAD'”
  1. Jeff KingFeb 28, 2017
  2. 1/8 interpret_branch_name: move docstring to header fileJeff King, Feb 28, 2017
  3. 4/8 interpret_branch_name: allow callers to restrict expansionsJeff King, Feb 28, 2017
  4. Jeff KingFeb 28, 2017
  5. Jeff KingFeb 28, 2017
  6. Junio C HamanoFeb 28, 2017
  7. Jeff KingFeb 28, 2017
  8. 3/8 strbuf_branchname: add docstringJeff King, Feb 28, 2017
  9. 8/8 checkout: restrict @-expansions when finding branchJeff King, Feb 28, 2017
  10. 6/8 branch: restrict @-expansions when deletingJeff King, Feb 28, 2017
  11. 7/8 strbuf_check_ref_format(): expand only local branchesJeff King, Feb 28, 2017
  12. 5/8 t3204: test git-branch @-expansion corner casesJeff King, Feb 28, 2017
  13. 2/8 strbuf_branchname: drop return valueJeff King, Feb 28, 2017
  14. Jacob KellerFeb 28, 2017
  15. Junio C HamanoMar 1, 2017

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.