Re: [PATCH v2] clone: Allow combining --bare and --origin

From: Øystein Walle <oystwa@gmail.com>
Date: Aug 4, 2021, 13:30 UTC
Message-ID: <20210804133010.25855-1-oystwa@gmail.com>
In-Reply-To: <xmqqv94mtdyj.fsf@gitster.g>

Hi again,

Thanks for accepting the patch.

> It is somewhat unfortunate that we do not say what the name of the
> "origin" is anywhere in the resulting configuration file.  The only
> way to tell that "--origin somewhere" was used is to notice that there
> is only one remote and its name is "somewhere".

This reads as self-contradictory to me. The word "origin" is nowhere in the configuration file, that's true. But that's because the user chose it to be that way, and the name the user chose is in the there.

The reason I see it as self-contradictory is that I see two different usages of the word "origin" in your email:

 1. A *term* meaning the repository that was cloned (e.g. 'name of the
 "origin"', remote.originName)

 2. The *name* of a remote ('there is only one remote and its name is
 [not "origin"]')

Seems you are aware since you write it in quotes :-)

Both usages appear in the wild and are even mixed sometimes, but in my experience it's not a big deal; it's usually obvious from context. I think the second usage is the common one, but the name is so common that it leads to the first. Is this something we'd like to tackle? If so, it just occured to me that it certainly doesn't help that the switch to change the name referring to the repo that was cloned from "origin" to something else is "--origin".

> I do not think "git fetch" in such a repository knows that it needs to
> fetch from 'somewhere', even whe it is the only remote repository
> available to us.

Changing git fetch to fall back to a remote not named "origin" if that is the only one configured makes perfect sense to me. (I am skeptical about remotes.originName since that favors the first usage outlined above.)

I have cc'ed the origin (pun overtly intended) of this patch and discussion for their take on it.

Show 7 quoted lines

> Instead of "usually the thing is called 'origin', so let's make sure
> it does not exist", we may want to say "there is only one remote and
> it is called somewhere because that is how we named it", i.e.
>
>	git -C clone-bare config --name-only \ --get-regexp
>	"remote\..*\.url" >actual && echo remote.somewhere.url >expect
>	&& test_cmp actual expect

This seems like like a better test than the one I wrote.

By the way, I noticed you already fixed my mistake with the repo name. Thanks for that. I sent this as a v2, but as you can imagine I did it in two steps in real life. First I removed the test then later I wrote a new one, and in between I rebased my changes. In the mean time new tests were added. I noticed they failed, but I didn't realize that was my fault.

Øsse