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

Re: [BUG/PATCH] contrib/subtree: allow addition of remote branch with name not locally present

From
Paul Campbell <pcampbell@kemitix.net>
Date
Feb 28, 2013, 21:37 UTC
Message-ID
<CALeLG_mryWOE_KrFDAf6Z7=w6kdaXii4rHzPYBfRbsjt4TZTAw@mail.gmail.com>
In-Reply-To
<7vwqtti91k.fsf@alter.siamese.dyndns.org>
On Thu, Feb 28, 2013 at 12:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 43 quoted lines
> Paul Campbell <pcampbell@kemitix.net> writes:
>
>> cmd_add() attempts to check for the validity of refspec for the repository
>> it is about to add as a subtree. It tries to do so before contacting the
>> repository. If the refspec happens to exist locally (say 'master') then
>> the test passes and the repo is fetched. If the refspec doesn't exist
>> locally then the test fails and the remote repo is never contacted.
>>
>> Removing the tests still works as the git fetch command fails with the
>> perfectly accurate error:
>>
>>   fatal: Couldn't find remote ref <refspec>
>>
>> Signed-off-by: Paul Campbell <pcampbell@carnegiecollege.ac.uk>
>> ---
>>
>> I must confess to not understanding the comment preceding the
>> rev-parse test. Given that it is against the rev-parse and not the
>> cmd_add_repository call I'm assuming it can be removed.
>
> This is contrib/ material and I do not use the command, so anything
> I say should be taken with a moderate amount of salt, but I think
> the comment is talking about _not_ accepting a full storing refspec,
> or worse yet wildcard, e.g.
>
>         refs/heads/topic:refs/remotes/origin/topic
>         refs/heads/*:refs/remotes/origin/*
>
> which will not make sense given that you are only adding a single
> commit via "cmd_add".  Also, if you have already fetched from the
> remote, "rev-parse $2^{commit}" at this point will protect against
> a typo by the end user.
>
> As you noticed, checking if $2 is a commit using rev-parse _before_
> fetching $2 from the remote repository is not a valid way to check
> against such errors.  So I tend to agree that removing the "die"
> will be a good idea.
>
> Typing "tipoc" when the user meant "topic" will error out at the
> "git fetch" done in cmd_add_repository, but that fetch will happily
> fetch and store whatever a refspec specifies, so you might want to
> replace the bogus "rev-parse before fetching" check to "reject
> refspec" with something else.
Thanks for the feedback.

Checking for a ':' or a '*' in the refspec should stop the storage name and wildcards getting through.

Rerolling the patch with new test.
-- 
Paul [W] Campbell
Previous: Junio C Hamano
Message 3 of 3 in “contrib/subtree: allow addition of remote branch with name not locally present”
  1. contrib/subtree: allow addition of remote branch with name not locally presentPaul Campbell, Feb 27, 2013
  2. Junio C HamanoFeb 28, 2013
  3. Paul CampbellFeb 28, 2013

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.