Re: [PATCHv5] clone --single: limit the fetch refspec to fetched branch

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 17, 2012, 20:18 UTC
Message-ID: <7v8vc8v1wd.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1347909706-22888-1-git-send-email-ralf.thielow@gmail.com>

Ralf Thielow <ralf.thielow@gmail.com> writes:

> - handle --mirror option (test added)

Handle how? I personally think erroring out is the right way to handle it, but if we care about people who have been misusing the combination of single and mirror, the second best way would be to imply "mirror" and "single" combination as "bare" and "single" without "mirror".

> - install correct refspec if the value of --branch is a tag (test added)

What is the definition of "correct"? I see the documentation says "--branch can also take tags and treat them like detached HEAD", and even though I _think_ allowing tags was a huge mistake, if we claim we treat it like detached HEAD, we should do the same when coming up with the refspec used for the follow-up "fetch".

Whatever we decide to do, the semantics we decided to use at least need to be described in the commit log message, not just in the "changes from the previous iteration". Updating the "Documentation/git-clone.txt" would also be necessary.

Show 5 quoted lines

> +test_expect_success 'refspec contains all branches by default' '
> +	echo "+refs/heads/*:refs/remotes/origin/*" > expect &&
> +	git --git-dir=dir_all/.git config --get remote.origin.fetch > actual &&
> +	test_cmp expect actual
> +'

I still think these checks that know the current implementation details (i.e. what exact configuration variables get what exact values) are wrong thing to have in the longer term. If the desired behaviour (i.e. "later fetch do not screw up") can be tested directly like the later parts of the test in this patch does, how that desired behaviour is implemented should not have to be cast in stone with these tests.