All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clone: Remove constraint on --bare and --origin
@ 2021-08-01  8:25 Øystein Walle
  2021-08-02  2:18 ` Junio C Hamano
  2021-08-02  8:53 ` Ævar Arnfjörð Bjarmason
  0 siblings, 2 replies; 13+ messages in thread
From: Øystein Walle @ 2021-08-01  8:25 UTC (permalink / raw)
  To: git; +Cc: Øystein Walle

This test has been present since long before clone was ported to C. Now
there is no need for it, and since df61c88979 (clone: also configure url
for bare clones, 2010-03-29) it's especially useful to allow both
options.

Signed-off-by: Øystein Walle <oystwa@gmail.com>
---

A question on this constraint popped up on #git the other day. I
investigated a bit and found no particular reason for its existence. All
tests still pass (except the one removed here) and the behavior is as
expected. I realize it might have gone under the radar for 11 years but
it's still worth the noise to remove it, in my opinion.

I wanted to include a bit on the reasoning for the original check in the
commit message but I couldn't find it. 

 builtin/clone.c          | 3 ---
 t/t5606-clone-options.sh | 8 --------
 2 files changed, 11 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 66fe66679c..70ec72ea85 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1014,9 +1014,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		option_bare = 1;
 
 	if (option_bare) {
-		if (option_origin)
-			die(_("--bare and --origin %s options are incompatible."),
-			    option_origin);
 		if (real_git_dir)
 			die(_("--bare and --separate-git-dir are incompatible."));
 		option_no_checkout = 1;
diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
index 3a595c0f82..4a8a2ca6f7 100755
--- a/t/t5606-clone-options.sh
+++ b/t/t5606-clone-options.sh
@@ -30,14 +30,6 @@ test_expect_success 'rejects invalid -o/--origin' '
 
 '
 
-test_expect_success 'disallows --bare with --origin' '
-
-	test_must_fail git clone -o foo --bare parent clone-bare-o 2>err &&
-	test_debug "cat err" &&
-	test_i18ngrep -e "--bare and --origin foo options are incompatible" err
-
-'
-
 test_expect_success 'disallows --bare with --separate-git-dir' '
 
 	test_must_fail git clone --bare --separate-git-dir dot-git-destiation parent clone-bare-sgd 2>err &&
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] clone: Remove constraint on --bare and --origin
  2021-08-01  8:25 [PATCH] clone: Remove constraint on --bare and --origin Øystein Walle
@ 2021-08-02  2:18 ` Junio C Hamano
  2021-08-02  8:53 ` Ævar Arnfjörð Bjarmason
  1 sibling, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2021-08-02  2:18 UTC (permalink / raw)
  To: Øystein Walle; +Cc: git

Øystein Walle <oystwa@gmail.com> writes:

> This test has been present since long before clone was ported to C. Now
> there is no need for it, and since df61c88979 (clone: also configure url
> for bare clones, 2010-03-29) it's especially useful to allow both
> options.
>
> Signed-off-by: Øystein Walle <oystwa@gmail.com>
> ---
>
> A question on this constraint popped up on #git the other day. I
> investigated a bit and found no particular reason for its existence. All
> tests still pass (except the one removed here) and the behavior is as
> expected. I realize it might have gone under the radar for 11 years but
> it's still worth the noise to remove it, in my opinion.
>
> I wanted to include a bit on the reasoning for the original check in the
> commit message but I couldn't find it. 

I suspect that this originally was because "git clone --bare" does
not use any remote-tracking branch (i.e. no refs/remotes/origin/*)
and the only expected way to update a "git clone --bare" repository
was to run "git fetch --mirror [--prune]", so there was no need to
make the nickname "origin" to be configurable.

I do not offhand know what other features in "git clone --bare" that
were added since then affect the resulting repository so that the
name "origin" it leaves there (perhaps in its configuration, if not
names in ref hierarchy) is visible to the end user and deserves to
be customizable.

In short, I think the "don't use --origin in a bare repository" was
not because "doing so will break X and Y", but because "doing so
does not make any practical difference".  So I am OK to lift this
check.  It is a small enough change that is easy to revert if there
were some valid reasons we failed to consider.

Thanks.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] clone: Remove constraint on --bare and --origin
  2021-08-01  8:25 [PATCH] clone: Remove constraint on --bare and --origin Øystein Walle
  2021-08-02  2:18 ` Junio C Hamano
@ 2021-08-02  8:53 ` Ævar Arnfjörð Bjarmason
  2021-08-02 17:49   ` [PATCH v2] clone: Allow combining " Øystein Walle
  1 sibling, 1 reply; 13+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-08-02  8:53 UTC (permalink / raw)
  To: Øystein Walle; +Cc: git


On Sun, Aug 01 2021, Øystein Walle wrote:

> This test has been present since long before clone was ported to C. Now
> there is no need for it, and since df61c88979 (clone: also configure url
> for bare clones, 2010-03-29) it's especially useful to allow both
> options.
>
> Signed-off-by: Øystein Walle <oystwa@gmail.com>
> ---
>
> A question on this constraint popped up on #git the other day. I
> investigated a bit and found no particular reason for its existence. All
> tests still pass (except the one removed here) and the behavior is as
> expected. I realize it might have gone under the radar for 11 years but
> it's still worth the noise to remove it, in my opinion.
>
> I wanted to include a bit on the reasoning for the original check in the
> commit message but I couldn't find it. 

Aside from working in Junio's reply as a summary of the original
justification in the commit message for a v2...

> -test_expect_success 'disallows --bare with --origin' '
> -
> -	test_must_fail git clone -o foo --bare parent clone-bare-o 2>err &&
> -	test_debug "cat err" &&
> -	test_i18ngrep -e "--bare and --origin foo options are incompatible" err
> -
> -'
> -

This just removes the only test, if it's "especially useful" to allow
both options let's replace this with a test that shows and tests for
those use-cases.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-02  8:53 ` Ævar Arnfjörð Bjarmason
@ 2021-08-02 17:49   ` Øystein Walle
  2021-08-03 21:28     ` Junio C Hamano
  2021-08-04  1:16     ` Junio C Hamano
  0 siblings, 2 replies; 13+ messages in thread
From: Øystein Walle @ 2021-08-02 17:49 UTC (permalink / raw)
  To: avarab, gitster; +Cc: git, Øystein Walle

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.

Signed-off-by: Øystein Walle <oystwa@gmail.com>
---

Hi Junio and Ævar,

I investigated a bit more and updated the commit message accordingly.
Instead of just removing the test I have replaced it with one that
checks that the behavior is as intended. 

Æ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.

Thanks,
Øsse

 builtin/clone.c          |  3 ---
 t/t5606-clone-options.sh | 10 +++++-----
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 66fe66679c..70ec72ea85 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1014,9 +1014,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		option_bare = 1;
 
 	if (option_bare) {
-		if (option_origin)
-			die(_("--bare and --origin %s options are incompatible."),
-			    option_origin);
 		if (real_git_dir)
 			die(_("--bare and --separate-git-dir are incompatible."));
 		option_no_checkout = 1;
diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
index 3a595c0f82..c40dde816d 100755
--- a/t/t5606-clone-options.sh
+++ b/t/t5606-clone-options.sh
@@ -30,12 +30,12 @@ test_expect_success 'rejects invalid -o/--origin' '
 
 '
 
-test_expect_success 'disallows --bare with --origin' '
-
-	test_must_fail git clone -o foo --bare parent clone-bare-o 2>err &&
-	test_debug "cat err" &&
-	test_i18ngrep -e "--bare and --origin foo options are incompatible" err
+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
 '
 
 test_expect_success 'disallows --bare with --separate-git-dir' '
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-02 17:49   ` [PATCH v2] clone: Allow combining " Øystein Walle
@ 2021-08-03 21:28     ` Junio C Hamano
  2021-08-04 13:30       ` Øystein Walle
  2021-08-04  1:16     ` Junio C Hamano
  1 sibling, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2021-08-03 21:28 UTC (permalink / raw)
  To: Øystein Walle; +Cc: avarab, git

Øystein Walle <oystwa@gmail.com> 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".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-02 17:49   ` [PATCH v2] clone: Allow combining " Øystein Walle
  2021-08-03 21:28     ` Junio C Hamano
@ 2021-08-04  1:16     ` Junio C Hamano
  1 sibling, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2021-08-04  1:16 UTC (permalink / raw)
  To: Øystein Walle; +Cc: avarab, git

Øystein Walle <oystwa@gmail.com> writes:

> diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
> index 3a595c0f82..c40dde816d 100755
> --- a/t/t5606-clone-options.sh
> +++ b/t/t5606-clone-options.sh
> @@ -30,12 +30,12 @@ test_expect_success 'rejects invalid -o/--origin' '
>  
>  '
>  
> -test_expect_success 'disallows --bare with --origin' '
> -
> -	test_must_fail git clone -o foo --bare parent clone-bare-o 2>err &&
> -	test_debug "cat err" &&
> -	test_i18ngrep -e "--bare and --origin foo options are incompatible" err
> +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
>  '

This breaks a later step that creates clone-bare as this used to use
clone-bare-o and the name was available.

Using clone-bare-o as it used to would probably be the easiest fix.

>  
>  test_expect_success 'disallows --bare with --separate-git-dir' '

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-03 21:28     ` Junio C Hamano
@ 2021-08-04 13:30       ` Øystein Walle
  2021-08-04 17:06         ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Øystein Walle @ 2021-08-04 13:30 UTC (permalink / raw)
  To: gitster; +Cc: git, rn+git

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-04 13:30       ` Øystein Walle
@ 2021-08-04 17:06         ` Junio C Hamano
  2021-08-06 20:23           ` Roman Neuhauser
  2021-08-07 22:08           ` Re* " Junio C Hamano
  0 siblings, 2 replies; 13+ messages in thread
From: Junio C Hamano @ 2021-08-04 17:06 UTC (permalink / raw)
  To: Øystein Walle; +Cc: git, rn+git

Øystein Walle <oystwa@gmail.com> writes:

> 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.

In other words, if there were two remotes in the configuration file,
you cannot tell which one was given to --origin when you made the
repository with "git clone".

> 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 :-) 

May be but #1 is not all that interesting.  

I meant the only one thing.  The user told Git that 'somewhere' is
the word, not 'origin' that is used by those who use the default
configuration, will be used to refer to the remote the repository
was cloned from.  In the first paragraph you quoted, I was referring
to the fact that the knowledge will be lost once you did "git remote
add elsewhere".

We cannot tell between 'somewhere' and 'elsewhere', which one is
what those who use the default configuration would refer to
'origin'---presumably, 'somewhere' being the --origin's argument
when "git clone" was run, has some significance over 'elsewhere' in
the user's mind, even after the latter is added to the repository.

But we'd end up treating them the same.  And something like
remote.originName would help that.  Otherwise, we'd end up sending
this message:

    Even if we give "--bare --origin yourfavouritename" to you now,
    unlike how 'origin' is treated in the default case, in the
    resulting repository, 'yourfavouritename' is not special at all.

Some people may want to treat yourfavouritename is not special at
all, while some people may want to treat yourfavouritename truly as
a replacement for 'origin' that is the default.  The message we
would be sending is that we'd ignore the latter folks.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-04 17:06         ` Junio C Hamano
@ 2021-08-06 20:23           ` Roman Neuhauser
  2021-08-06 22:13             ` Junio C Hamano
  2021-08-07 22:08           ` Re* " Junio C Hamano
  1 sibling, 1 reply; 13+ messages in thread
From: Roman Neuhauser @ 2021-08-06 20:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Øystein Walle, git

Hello,

i'm "the user" in this story.  Muchas gracias to osse for turning
my bickering into a patch.

A little background.  I use --origin a lot (or git remmote rename
afterwards), because origin carries no information about the remote
repository, and I could have cloned any of those.  The URL I used
in `git clone` has little to do with which remotes I'll want to
pull from and which I'll want to push to.  "origin" is suspicious
and I'm used to giving my remotes names that mean something to me.

My need for git clone --bare --origin surfaced when I was writing
a tool for versioning dotfiles (don't we all have one).  It has to
be able to work with pre-existing files in the home dir:

$ git dirs clone $url x
# git-dir is $PWD/.git-dirs/repo.d/x
# work-tree is $PWD

I used git clone --bare / git config core.bare false /
git config core.worktree ... and hit the error message when I tried
to add support for --origin.

# gitster@pobox.com / 2021-08-04 10:06:31 -0700:
> In other words, if there were two remotes in the configuration file,
> you cannot tell which one was given to --origin when you made the
> repository with "git clone".

I'm not sure why this matters (not saying it doesn't).
 
> But we'd end up treating them the same.  And something like
> remote.originName would help that.  Otherwise, we'd end up sending
> this message:
> 
>     Even if we give "--bare --origin yourfavouritename" to you now,
>     unlike how 'origin' is treated in the default case, in the
>     resulting repository, 'yourfavouritename' is not special at all.

Isn't that the case in non-bare repositories as well?
BTW I don't like special cases but realize that the "origin" ship has
sailed long ago.
 
> Some people may want to treat yourfavouritename is not special at
> all, while some people may want to treat yourfavouritename truly as
> a replacement for 'origin' that is the default.  The message we
> would be sending is that we'd ignore the latter folks.
 
Can't they just continue doing what they've been doing so far,
that is leave it at "origin"?  I'm not sure this would be my concern
as a user of this feature.

-- 
roman

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-06 20:23           ` Roman Neuhauser
@ 2021-08-06 22:13             ` Junio C Hamano
  2021-08-07 11:18               ` Roman Neuhauser
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2021-08-06 22:13 UTC (permalink / raw)
  To: Roman Neuhauser; +Cc: Øystein Walle, git

Roman Neuhauser <rn+git@sigpipe.cz> writes:

>> But we'd end up treating them the same.  And something like
>> remote.originName would help that.  Otherwise, we'd end up sending
>> this message:
>> 
>>     Even if we give "--bare --origin yourfavouritename" to you now,
>>     unlike how 'origin' is treated in the default case, in the
>>     resulting repository, 'yourfavouritename' is not special at all.
>
> Isn't that the case in non-bare repositories as well?

You have branches that are checked out.  The first branch that you'd
presumably be using as the primary (traditionally called 'master')
knows that the nickname used to call the remote it integrates with
as the value of branch.master.remote

In a bare repository, there is no such clue.

> Can't they just continue doing what they've been doing so far,
> that is leave it at "origin"?  I'm not sure this would be my concern
> as a user of this feature.

That answer can be thrown back at you.  You can leave it at "origin"
when using "--bare" ;-).

The posted patch is a good first step to allow both options to be
used at the same time.  Without the first step, these two options
cannot coexist.

But I am also saying that the first step alone is an inadequate
solution that goes only halfway.  If you can get yourfavouritename,
while others cannot use their favourite names, that is not a
satisfying solution.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-06 22:13             ` Junio C Hamano
@ 2021-08-07 11:18               ` Roman Neuhauser
  0 siblings, 0 replies; 13+ messages in thread
From: Roman Neuhauser @ 2021-08-07 11:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Øystein Walle, git

# gitster@pobox.com / 2021-08-06 15:13:35 -0700:
> Roman Neuhauser <rn+git@sigpipe.cz> writes:
> 
> >> But we'd end up treating them the same.  And something like
> >> remote.originName would help that.  Otherwise, we'd end up sending
> >> this message:
> >> 
> >>     Even if we give "--bare --origin yourfavouritename" to you now,
> >>     unlike how 'origin' is treated in the default case, in the
> >>     resulting repository, 'yourfavouritename' is not special at all.
> >
> > Isn't that the case in non-bare repositories as well?
> 
> You have branches that are checked out.  The first branch that you'd
> presumably be using as the primary (traditionally called 'master')
> knows that the nickname used to call the remote it integrates with
> as the value of branch.master.remote

aha, i see that as a special (heh) case, an exception. :)
i spend most of my time on branches with no upstram.  sure, they're
extensions of master and such, but they have no upstream themselves.
and since there's no "origin" remote in my repos:

  git checkout -b fix-this-or-that master
  # tadaa, git fetch does nothing[1]

git fetch losing the hardcoded "origin" in favor of a configurable
value would be an improvement, yes.

> > Can't they just continue doing what they've been doing so far,
> > that is leave it at "origin"?  I'm not sure this would be my concern
> > as a user of this feature.

hm, that last sentence came out wrong.  i meant to say that as a user
of this feature, i would not mind having to provide and explicit remote.

> That answer can be thrown back at you.  You can leave it at "origin"
> when using "--bare" ;-).

how would that help the people who yearn for clone --bare --origin
but wouldn't use it if it meant fetch with explicit remotes?

> The posted patch is a good first step to allow both options to be
> used at the same time.  Without the first step, these two options
> cannot coexist.

i agree.
 
> But I am also saying that the first step alone is an inadequate
> solution that goes only halfway.  If you can get yourfavouritename,
> while others cannot use their favourite names, that is not a
> satisfying solution.

i don't see how the patch in its current form prevents anyone from
naming --origin whatever they want (within the accepted syntax).

---

i think a step back is in order.  git fetch --all would work,
git remote update would work.  if the issue is the imaginary
guy's ability to update the bare repo without peeking inside
config, either of these commands has him covered.

if the goal is to enable git fetch w/o --all or any other remote
specification then i'd say remote.fetchDefault would be a nice
mirror to remote.pushDefault.  this glaring asymmetry would go away:

  If no remote is configured, or if you are not on any branch,
  it defaults to origin for fetching and remote.pushDefault
  for pushing.

if you want the repo to remember where it was cloned from,
then again, remote.fetchDefault can fill that role.  obviously
mutable, but any setting would be, and i just don't see a problem
with that.

coming back to a question that fell below the radar:

# gitster@pobox.com / 2021-08-04 10:06:31 -0700:
> In other words, if there were two remotes in the configuration file,
> you cannot tell which one was given to --origin when you made the
> repository with "git clone".

when does this matter?

---

looking over the earlier emails, i'd like to reiterate one thing:

> We cannot tell between 'somewhere' and 'elsewhere', which one is
> what those who use the default configuration would refer to
> 'origin'---presumably, 'somewhere' being the --origin's argument
> when "git clone" was run, has some significance over 'elsewhere' in
> the user's mind, even after the latter is added to the repository.

i can't speak for others, but with me, this assumption is flat out
wrong.  half my "working copies" get cloned from upstream sources
and i add a remote to publish my changes from later, while the other
half happens the other way around.  the urls given to git clone
don't mean... much[2].

finally, this notion that --origin in a regular clone works just like
"origin" is generally false.  relevant to bare repos, if you don't
have any branch checked out, it goes to "origin".  iow if symmetry
between regular and bare clones is the goal, then mission accomplished,
they already behave the same.


[1] not only does it do nothing, it does it without a beep, which,
    aside from the runtime, looks just like a successful fetch from
    a remote i'm up-to-date with.

[2] https://www.youtube.com/watch?v=WO2q1iQX2UA

-- 
roman; btw, git-pull is backwards

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re* [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-04 17:06         ` Junio C Hamano
  2021-08-06 20:23           ` Roman Neuhauser
@ 2021-08-07 22:08           ` Junio C Hamano
  2021-08-08  2:03             ` Roman Neuhauser
  1 sibling, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2021-08-07 22:08 UTC (permalink / raw)
  To: Øystein Walle; +Cc: git, rn+git

Junio C Hamano <gitster@pobox.com> writes:

>>> 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".
>> ...
> But we'd end up treating them the same.  And something like
> remote.originName would help that.  Otherwise, we'd end up sending
> this message:
>
>     Even if we give "--bare --origin yourfavouritename" to you now,
>     unlike how 'origin' is treated in the default case, in the
>     resulting repository, 'yourfavouritename' is not special at all.
>
> Some people may want to treat yourfavouritename is not special at
> all, while some people may want to treat yourfavouritename truly as
> a replacement for 'origin' that is the default.  The message we
> would be sending is that we'd ignore the latter folks.

So, let's illustrate one of the things that is needed after the good
first step to allow --bare --origin=yourfavouritename used together.

There may be other things that needs fixing, of course, but we need
to start from somewhere.

---- >8 -------- >8 -------- >8 -------- >8 -------- >8 -------- >8 ----
Subject: [PATCH] remote: fall back on the sole remote when unspecified

Historically, we used hardcoded "origin" as the fallback default for
commands that take a remote (e.g. "git fetch") when the user did not
tell us otherwise.  Since the "--origin=name" option was taught to
"git clone", however, we may not have a remote whose name is
"origin" at all.

Which means that the name given to "git clone --origin" does not
truly replace the hardcoded "origin". An example of such limitation
is that "git fetch" (no other parameters) would fetch happily from
the "origin" repository, but in a repository cloned with the custom
name using "--origin=name", "git fetch" would not fetch from anywhere
and instead fail.

We can fix this by noticing that the repository has one and only one
remote defined, and use that as a replacement for the hardcoded
"origin".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

This matters for automation for those who want to use --origin
option.  Imagine you have multiple bare clones and you wanted to use
custom names for 'origin'.  And you want a cron job that goes over
these repositories and run "git fetch" from their upstream before
you come in for work, so that these bare clones can be used as
close-by mirrors of their upstream projects.

Unfortunately, that would not work.  If these repositories use
their own nicknames for their upstream that are not "origin",

	for repo in a b c
	do
		git -C $repo fetch
	done

would just fail.  Of course, you can somehow out-of-band know the
origin's name for each repo, e.g.

	for repoorigin in a:xyzzy b:frotz c:nitfol
	do
		repo=${repoorigin%:*}
                origin=${repoorigin#*:}
		git -C $repo fetch $origin
	done

but that is solving a problem that arises only because we are not
treating the name given to "git clone --origin=name" as a true
replacement for the default "origin".

 remote.c             | 10 +++++++++-
 t/t5512-ls-remote.sh | 10 ++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git c/remote.c w/remote.c
index dfb863d808..8a2fd1ccc9 100644
--- c/remote.c
+++ w/remote.c
@@ -39,6 +39,8 @@ static int remotes_alloc;
 static int remotes_nr;
 static struct hashmap remotes_hash;
 
+static const char *default_remote_name;
+
 static struct branch **branches;
 static int branches_alloc;
 static int branches_nr;
@@ -460,6 +462,12 @@ static void read_config(void)
 	}
 	git_config(handle_config, NULL);
 	alias_all_urls();
+	if (remotes_nr == 1 &&
+	    remotes[0]->configured_in_repo &&
+	    remotes[0]->url)
+		default_remote_name = remotes[0]->name;
+	else
+		default_remote_name = "origin";
 }
 
 static int valid_remote_nick(const char *name)
@@ -483,7 +491,7 @@ const char *remote_for_branch(struct branch *branch, int *explicit)
 	}
 	if (explicit)
 		*explicit = 0;
-	return "origin";
+	return default_remote_name;
 }
 
 const char *pushremote_for_branch(struct branch *branch, int *explicit)
diff --git c/t/t5512-ls-remote.sh w/t/t5512-ls-remote.sh
index f53f58895a..aa6f14e8fd 100755
--- c/t/t5512-ls-remote.sh
+++ w/t/t5512-ls-remote.sh
@@ -83,8 +83,14 @@ test_expect_success 'ls-remote --sort="-refname" --tags self' '
 	test_cmp expect actual
 '
 
-test_expect_success 'dies when no remote specified and no default remotes found' '
-	test_must_fail git ls-remote
+test_expect_success 'ls-remote falls back to the only remote' '
+	generate_references \
+		refs/tags/mark1.2 \
+		refs/tags/mark1.10 \
+		refs/tags/mark1.1 \
+		refs/tags/mark >expect &&
+	git ls-remote --sort="-refname" --tags >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'use "origin" when no remote specified' '

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: Re* [PATCH v2] clone: Allow combining --bare and --origin
  2021-08-07 22:08           ` Re* " Junio C Hamano
@ 2021-08-08  2:03             ` Roman Neuhauser
  0 siblings, 0 replies; 13+ messages in thread
From: Roman Neuhauser @ 2021-08-08  2:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Øystein Walle, git

# gitster@pobox.com / 2021-08-07 15:08:02 -0700:
> Subject: [PATCH] remote: fall back on the sole remote when unspecified
> 
> Historically, we used hardcoded "origin" as the fallback default for
> commands that take a remote (e.g. "git fetch") when the user did not
> tell us otherwise.  Since the "--origin=name" option was taught to
> "git clone", however, we may not have a remote whose name is
> "origin" at all.
> 
> Which means that the name given to "git clone --origin" does not
> truly replace the hardcoded "origin". An example of such limitation
> is that "git fetch" (no other parameters) would fetch happily from
> the "origin" repository, but in a repository cloned with the custom
> name using "--origin=name", "git fetch" would not fetch from anywhere
> and instead fail.

hey, i'm all for all this pre-existing lossage getting fixed if you
can do it.  all i'm saying is that since this combination of options
wasn't possible before there won't be any pre-existing uses of git
suddenly breaking.

> This matters for automation for those who want to use --origin
> option.  Imagine you have multiple bare clones and you wanted to use
> custom names for 'origin'.  And you want a cron job that goes over
> these repositories and run "git fetch" from their upstream before
> you come in for work, so that these bare clones can be used as
> close-by mirrors of their upstream projects.

imagine that you wanted to use git clone --bare --origin with
any git version released so far.  this is not snark, i'm pointing
out that git git has a history of things not working where one
would expect them to.
 
> Unfortunately, that would not work.  If these repositories use
> their own nicknames for their upstream that are not "origin",
> 
> 	for repo in a b c
> 	do
> 		git -C $repo fetch
> 	done

  for repo in a b c; do
    git -C $repo fetch --all # or git -C remote update
  done

all it takes to mitigate this is to point this out in the release
notes and man page.  what you sketched out above is analogous to my
initial encounter with git clone --bare --origin not working:
where were you when the half-assed implementation was landing?  :)
why was there no one to champion for people who'd want to use those
two together? :)) (j/k)
 
> would just fail.  Of course, you can somehow out-of-band know the
> origin's name for each repo, e.g.

even if i accept the premise that git fetch --all can't be used
and the explicit name is necessary, isn't that magical out-of-band
wand called git-config?

  origin=$(
    git config --file $repo \
    --name-only --get-regexp \
    '^remote\.[^.]*.url' |
    sed -E 's/^remote\.([^.]+).url$/\1/'
  )
  git -C $repo fetch $origin
 
i'm not skilled enough in git-config to simplify that.


i think it'd be prudent to pause this thread for now because it's
only distracting you from fixing the --origin fallout, and as long
as you talk about how it *should* be while i bring up available
workarounds, it's just noise.

> but that is solving a problem that arises only because we are not
> treating the name given to "git clone --origin=name" as a true
> replacement for the default "origin".

and i'm really grateful that you're tying the loose ends, as long
as this whole thing doesn't fizzle out on account of being too much,
and the partial improvement doesn't get swept with it!
 
i think i said in earlier that i'm a big fan of stripping "origin"
of its special standing.  huge kudos if you can see this through.

-- 
roman

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-08-08  2:03 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-01  8:25 [PATCH] clone: Remove constraint on --bare and --origin Øystein Walle
2021-08-02  2:18 ` Junio C Hamano
2021-08-02  8:53 ` Ævar Arnfjörð Bjarmason
2021-08-02 17:49   ` [PATCH v2] clone: Allow combining " Øystein Walle
2021-08-03 21:28     ` Junio C Hamano
2021-08-04 13:30       ` Øystein Walle
2021-08-04 17:06         ` Junio C Hamano
2021-08-06 20:23           ` Roman Neuhauser
2021-08-06 22:13             ` Junio C Hamano
2021-08-07 11:18               ` Roman Neuhauser
2021-08-07 22:08           ` Re* " Junio C Hamano
2021-08-08  2:03             ` Roman Neuhauser
2021-08-04  1:16     ` Junio C Hamano

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.