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

2 messages, 2 authors, 2021-08-04 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2021-08-03 21:29:04

Øystein Walle [off-list ref] writes:
The constraint on passing both these options simultaneously has been
present since long before clone was ported to C. At the time no
configuration referencing the remote repository was written at all in
bare clones.

Since df61c88979 (clone: also configure url for bare clones, 2010-03-29)
the remote repository is mentioned in the configuration file also for
bare repos, so it makes sense to allow the user to rename it if they
wish.
Sounds sensible.
Ævar, I was a bit melodramatic when I wrote "especially useful". I have
toned the commit message down a bit :-) In truth, I don't personally
have a use-case for this (I did reach out to the person who asked about
it in #git but did't get a reply) and have no problems with seeing this
patch ultimately rejected. It's just a result of me seeing it asked
about and getting an itch from it. But in my humble opinion this is now
an "artificial" constraint (for lack of a better term) and should be
removed on the grounds that there is no reason for it to be there in the
first place.
Yup, that is exactly my thought when I responded to your v1.
+test_expect_success '--bare works with -o/--origin' '
+	git clone --bare --origin=somewhere parent clone-bare &&
+	url="$(git -C clone-bare config --local remote.somewhere.url)" &&
+	test -n "$url" &&
+	test_must_fail git -C clone-bare config --local remote.origin.url
 '
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".  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

But stepping back a bit, I think this shows another reason why use
of '--origin' with '--bare' as-is may not be so pleasant to use.

In a repository _with_ working tree, this lack of "what is 'origin'
called in this repository?" is not a problem because you'd get these
after cloning:

    [remote "somewhere"]
	url = ...
	fetch = ...
    [branch "master"]
	remote = "somewhere"
	merge = refs/heads/master

You can say "git fetch" or "git pull" without the remote name and we
will know which remote to interact with, because our branch knows
which remote to fetch from.

In a bare repository, however, you only get this:

    [remote "somewhere"]
	url = ...

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.

We may need a bit _more_ work (e.g. leave an optional configuration
remote.originName = "somewhere" when "--bare --origin somewhere" is
used, and teach "git fetch" to pay attention to it, instead of
assuming 'origin') before "--bare --origin somewhere" becomes truly
usable.  And I suspect that "git fetch" is not the only one that
needs such "fix".

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

From: Øystein Walle <hidden>
Date: 2021-08-04 13:30:21

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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help