[PATCH] clone: Remove constraint on --bare and --origin

Subsystems: the rest

STALE1834d

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

[PATCH] clone: Remove constraint on --bare and --origin

From: Øystein Walle <hidden>
Date: 2021-08-01 08:25:57

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

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

Re: [PATCH] clone: Remove constraint on --bare and --origin

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-08-02 08:54:17

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

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.

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

From: Øystein Walle <hidden>
Date: 2021-08-02 17:49:59

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

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