All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Brandon Casey <casey@nrlssc.navy.mil>
Cc: git@vger.kernel.org, galak@kernel.crashing.org,
	Brandon Casey <drafnel@gmail.com>
Subject: Re: [PATCH 1/2] t/t5510: demonstrate failure to fetch when current branch has merge ref
Date: Wed, 25 Aug 2010 14:28:23 -0700	[thread overview]
Message-ID: <7vy6bupeko.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: O7UxM6KEqdDAhjJAF7ODSonSLShoyHHhaZNp8vb1mx2_JFqmMUj1Op5xiv_-bSd8xW04rLMul2k@cipher.nrlssc.navy.mil

Brandon Casey <casey@nrlssc.navy.mil> writes:

> From: Brandon Casey <drafnel@gmail.com>
>
> When 'git fetch' is supplied just a repository URL (not a remote name),
> and without a fetch refspec, it should fetch from the remote HEAD branch
> and update FETCH_HEAD with the fetched ref.  Currently, when 'git fetch'
> is called like this, it fails to retrieve anything, and does not update
> FETCH_HEAD, if the current checked-out branch has a configured merge ref.
>
> i.e. this fetch fails to retrieve anything nor update FETCH_HEAD:
>
>    git checkout master
>    git config branch.master.merge refs/heads/master
>    git fetch git://git.kernel.org/pub/scm/git/git.git

Hmph, we can call it a regression, since we certainly won't see this
failure with versions of git that is unaware of branch.*.merge.

But what should we be expecting?

Just as a datapoint, an ancient git (e.g. v1.4.0), the above command line
would have fetched the HEAD from the remote side, no matter what that
symref is pointing at.  Your [2/2] patch replicates this behaviour, which
is fine by me [*1*].

Your test only checks if we leave _anything_ in FETCH_HEAD, and does not
check if we only fetch one, if we fetch all the refs, or if we fetch what
the configuration branch.*.merge asks for (but without the corresponding
branch.*.remote configuration, doing so is pointless).

I think it would be better to have two tests.  One arranges the current
branch to track the same branch the HEAD at the remote points at, and the
other arranges the current branch to track a branch different from the
HEAD at the remote points at.  In both cases, as "fetch" should ignore the
configuration, we should get the object pointed by the HEAD on the remote
side.

Thanks.


[Footnote]

*1* A plausible alternative is to match the given URL against list of
existing remote.<name>.url (make sure there is only one), and behave as if
that the remote name is given.  I can be persuaded either way, but not
looking at the configuration feels a lot simpler to explain and understand
(i.e. "with name, we use the set of configuration variable given to that
name; without name, there is no configuration for us to look up").

  reply	other threads:[~2010-08-25 21:28 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-24  6:59 reducing object store size with remote alternates or shallow clone? Kumar Gala
2010-08-24 16:45 ` Junio C Hamano
2010-08-24 18:15   ` Brandon Casey
2010-08-24 18:59     ` Junio C Hamano
2010-08-24 23:29       ` Brandon Casey
2010-08-25 17:52         ` [PATCH 1/2] t/t5510: demonstrate failure to fetch when current branch has merge ref Brandon Casey
2010-08-25 21:28           ` Junio C Hamano [this message]
2010-08-25 17:52         ` [PATCH 2/2] builtin/fetch.c: ignore merge config when not fetching from branch's remote Brandon Casey
2010-08-25 21:16           ` Jonathan Nieder
2010-08-25 21:41             ` Brandon Casey
2010-08-25 21:54           ` Junio C Hamano
2010-09-09 18:56             ` [PATCH 1/2] builtin/fetch.c: comment that branch->remote_name is usable when has_merge Brandon Casey
2010-09-09 18:56             ` [PATCH 2/2] t/t5510-fetch.sh: improve testing with explicit URL and merge spec Brandon Casey

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=7vy6bupeko.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=casey@nrlssc.navy.mil \
    --cc=drafnel@gmail.com \
    --cc=galak@kernel.crashing.org \
    --cc=git@vger.kernel.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.