Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail

[Stefan Beller]

On 20.03.2016 12:40, Jeff King wrote:
> On Sun, Mar 20, 2016 at 06:05:34PM +0100, Lars Schneider wrote:
> 
 +test_expect_failure shallow-clone-recursive '
 +   URL="file://$(pwd | sed "s/[[:space:]]/%20/g")/repo" &&
>>>
>>> This would break if the test suite is in a path containing any other white 
>>> space
>>> than U+0020 such as a tab? (Not that I am encouraging using such paths...)
>> True. I wonder if we really need to use the "file://" URL format here. Maybe
>> a relative would be easier?!
> 
> I did not look closely at the patch or the tests, but generally we skip
> shallow-cloning for local-path repositories, as we don't do a regular
> object transfer at all (and we turn that optimization off for file://
> URLs). So presumably it would defeat the purpose of the test.
> 
> -Peff

Please see proposed patches at $gmane/288940

By passing along the --[no-]local option we can have tests, without
using file:// explicitely.

Thanks,
Stefan


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail

[Jeff King]

On Sun, Mar 20, 2016 at 06:05:34PM +0100, Lars Schneider wrote:

> >> +test_expect_failure shallow-clone-recursive '
> >> +   URL="file://$(pwd | sed "s/[[:space:]]/%20/g")/repo" &&
> > 
> > This would break if the test suite is in a path containing any other white 
> > space
> > than U+0020 such as a tab? (Not that I am encouraging using such paths...)
> True. I wonder if we really need to use the "file://" URL format here. Maybe
> a relative would be easier?!

I did not look closely at the patch or the tests, but generally we skip
shallow-cloning for local-path repositories, as we don't do a regular
object transfer at all (and we turn that optimization off for file://
URLs). So presumably it would defeat the purpose of the test.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail

[Lars Schneider]

On 15 Mar 2016, at 20:50, Stefan Beller  wrote:

> On Sun, Dec 20, 2015 at 3:19 PM,   wrote:
>> From: Lars Schneider 
>> 
>> "git clone --recursive --depth 1 --single-branch " clones the
>> submodules successfully. However, it does not obey "--depth 1" for
>> submodule cloning.
> 
> I am about to resend "[RFC/PATCH] clone: add `--shallow-submodules` flag"
> which would need tests and I thought about this series as tests.
Sounds good, thanks!


> I assume patch 2 (fixing a broken && chain in tests) made it through,
> but patch 1 and 3 did not? I may pick up ideas from here as it will be
> slightly different tests I'd guess.
Unfortunately patch 2 ($gmane/282776) did not made it in (I just checked 
next 104e649). How should we proceed?

> 
> 
>> +test_expect_failure shallow-clone-recursive '
>> +   URL="file://$(pwd | sed "s/[[:space:]]/%20/g")/repo" &&
> 
> This would break if the test suite is in a path containing any other white 
> space
> than U+0020 such as a tab? (Not that I am encouraging using such paths...)
True. I wonder if we really need to use the "file://" URL format here. Maybe
a relative would be easier?!


Thanks,
Lars--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail

[Junio C Hamano]

Stefan Beller  writes:

>> +test_expect_failure shallow-clone-recursive '
>> +   URL="file://$(pwd | sed "s/[[:space:]]/%20/g")/repo" &&
>
> This would break if the test suite is in a path containing any other white 
> space
> than U+0020 such as a tab? (Not that I am encouraging using such paths...)

Good spotting.

I thought use of [[:named character classes:]] is explicitly
disallowed by CodingGuidelines.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail

[Stefan Beller]

On Sun, Dec 20, 2015 at 3:19 PM,   wrote:
> From: Lars Schneider 
>
> "git clone --recursive --depth 1 --single-branch " clones the
> submodules successfully. However, it does not obey "--depth 1" for
> submodule cloning.

I am about to resend "[RFC/PATCH] clone: add `--shallow-submodules` flag"
which would need tests and I thought about this series as tests.

I assume patch 2 (fixing a broken && chain in tests) made it through,
but patch 1 and 3 did not? I may pick up ideas from here as it will be
slightly different tests I'd guess.


> +test_expect_failure shallow-clone-recursive '
> +   URL="file://$(pwd | sed "s/[[:space:]]/%20/g")/repo" &&

This would break if the test suite is in a path containing any other white space
than U+0020 such as a tab? (Not that I am encouraging using such paths...)

Thanks,
Stefan


> +   echo $URL &&
> +   git clone --recursive --depth 1 --single-branch $URL clone-recursive 
> &&
> +   (
> +   cd "clone-recursive" &&
> +   git log --oneline >lines &&
> +   test_line_count = 1 lines
> +   ) &&
> +   (
> +   cd "clone-recursive/submodule" &&
> +   git log --oneline >lines &&
> +   test_line_count = 1 lines
> +   )
> +'
> +
> +test_done
> --
> 2.5.1
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/3] submodule: add test to demonstrate that shallow recursive clones fail

[larsxschneider]

From: Lars Schneider 

"git clone --recursive --depth 1 --single-branch " clones the
submodules successfully. However, it does not obey "--depth 1" for
submodule cloning.

Signed-off-by: Lars Schneider 
---
 t/t7412-submodule-recursive.sh | 52 ++
 1 file changed, 52 insertions(+)
 create mode 100755 t/t7412-submodule-recursive.sh

diff --git a/t/t7412-submodule-recursive.sh b/t/t7412-submodule-recursive.sh
new file mode 100755
index 000..58b3eb9
--- /dev/null
+++ b/t/t7412-submodule-recursive.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='Test shallow cloning of repos with submodules'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+   git checkout -b master &&
+   echo file >file &&
+   git add file &&
+   test_tick &&
+   git commit -m "master commit 1" &&
+
+   git checkout -b branch &&
+   echo file >branch-file &&
+   git add branch-file &&
+   test_tick &&
+   git commit -m "branch commit 1" &&
+
+   git checkout master &&
+   git clone . repo &&
+   (
+   cd repo &&
+   git checkout master &&
+   git submodule add ../. submodule &&
+   (
+   cd submodule &&
+   git checkout branch
+   ) &&
+   git add submodule &&
+   test_tick &&
+   git commit -m "master commit 2"
+   )
+'
+
+test_expect_failure shallow-clone-recursive '
+   URL="file://$(pwd | sed "s/[[:space:]]/%20/g")/repo" &&
+   echo $URL &&
+   git clone --recursive --depth 1 --single-branch $URL clone-recursive &&
+   (
+   cd "clone-recursive" &&
+   git log --oneline >lines &&
+   test_line_count = 1 lines
+   ) &&
+   (
+   cd "clone-recursive/submodule" &&
+   git log --oneline >lines &&
+   test_line_count = 1 lines
+   )
+'
+
+test_done
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html