[WIP,RFC,v2,3/5] clone: copy hidden paths at local clone
diff mbox series

Message ID 20190226051804.10631-4-matheus.bernardino@usp.br
State New
Headers show
Series
  • clone: dir iterator refactoring with tests
Related show

Commit Message

Matheus Tavares Bernardino Feb. 26, 2019, 5:18 a.m. UTC
Make the copy_or_link_directory function no longer skip hidden paths.
This function, used to copy .git/objects, currently skips all hidden
directories but not hidden files, which is an odd behaviour. The reason
for that could be unintentional: probably the intention was to skip '.'
and '..' only but it ended up accidentally skipping all directories
starting with '.'. Besides being more natural, the new behaviour is more
permissive to the user.

Also adjusted tests to reflect this change.

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/clone.c            | 2 +-
 t/t5604-clone-reference.sh | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

Comments

Duy Nguyen Feb. 26, 2019, 12:13 p.m. UTC | #1
On Tue, Feb 26, 2019 at 12:18 PM Matheus Tavares
<matheus.bernardino@usp.br> wrote:
>
> Make the copy_or_link_directory function no longer skip hidden paths.

It's actually only hidden directories because of the S_ISDIR check
right above. Not that it matters much...

> This function, used to copy .git/objects, currently skips all hidden
> directories but not hidden files, which is an odd behaviour. The reason
> for that could be unintentional:

This goes back to the very first version of clone.c in 8434c2f1af
(Build in clone - 2008-04-27). If you look at git-clone.sh back then,
which is the version before the C conversion, it does something like
this

    find objects -depth -print | cpio $cpio_quiet_flag -pumd$l "$GIT_DIR/"

and I'm pretty sure 'find' will not attempt to hide anything. So yes I
think this is just for skipping '.' and '..' and accidentally skips
more. From that view, it's actually a regresssion but nobody ever
bothers to hide anything in 'objects' directory to notice.

> probably the intention was to skip '.'
> and '..' only but it ended up accidentally skipping all directories
> starting with '.'. Besides being more natural, the new behaviour is more
> permissive to the user.
>
> Also adjusted tests to reflect this change.
>
> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/clone.c            | 2 +-
>  t/t5604-clone-reference.sh | 9 +++++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 50bde99618..cae069f03b 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -428,7 +428,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
>                         continue;
>                 }
>                 if (S_ISDIR(buf.st_mode)) {
> -                       if (de->d_name[0] != '.')
> +                       if (!is_dot_or_dotdot(de->d_name))
>                                 copy_or_link_directory(src, dest,
>                                                        src_repo, src_baselen);
>                         continue;
> diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
> index 6f9c77049e..f1a8e74c44 100755
> --- a/t/t5604-clone-reference.sh
> +++ b/t/t5604-clone-reference.sh
> @@ -262,16 +262,25 @@ test_expect_success SHA1,SYMLINKS 'clone repo with manually symlinked objects/*'
>         test_cmp expected actual &&
>         find S-* -name "*some*" | sort >actual &&
>         cat >expected <<-EOF &&
> +       S--dissociate/.git/objects/.some-hidden-dir
> +       S--dissociate/.git/objects/.some-hidden-dir/.some-dot-file
> +       S--dissociate/.git/objects/.some-hidden-dir/some-file
>         S--dissociate/.git/objects/.some-hidden-file
>         S--dissociate/.git/objects/some-dir
>         S--dissociate/.git/objects/some-dir/.some-dot-file
>         S--dissociate/.git/objects/some-dir/some-file
>         S--dissociate/.git/objects/some-file
> +       S--local/.git/objects/.some-hidden-dir
> +       S--local/.git/objects/.some-hidden-dir/.some-dot-file
> +       S--local/.git/objects/.some-hidden-dir/some-file
>         S--local/.git/objects/.some-hidden-file
>         S--local/.git/objects/some-dir
>         S--local/.git/objects/some-dir/.some-dot-file
>         S--local/.git/objects/some-dir/some-file
>         S--local/.git/objects/some-file
> +       S--no-hardlinks/.git/objects/.some-hidden-dir
> +       S--no-hardlinks/.git/objects/.some-hidden-dir/.some-dot-file
> +       S--no-hardlinks/.git/objects/.some-hidden-dir/some-file
>         S--no-hardlinks/.git/objects/.some-hidden-file
>         S--no-hardlinks/.git/objects/some-dir
>         S--no-hardlinks/.git/objects/some-dir/.some-dot-file
> --
> 2.20.1
>

Patch
diff mbox series

diff --git a/builtin/clone.c b/builtin/clone.c
index 50bde99618..cae069f03b 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -428,7 +428,7 @@  static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 			continue;
 		}
 		if (S_ISDIR(buf.st_mode)) {
-			if (de->d_name[0] != '.')
+			if (!is_dot_or_dotdot(de->d_name))
 				copy_or_link_directory(src, dest,
 						       src_repo, src_baselen);
 			continue;
diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
index 6f9c77049e..f1a8e74c44 100755
--- a/t/t5604-clone-reference.sh
+++ b/t/t5604-clone-reference.sh
@@ -262,16 +262,25 @@  test_expect_success SHA1,SYMLINKS 'clone repo with manually symlinked objects/*'
 	test_cmp expected actual &&
 	find S-* -name "*some*" | sort >actual &&
 	cat >expected <<-EOF &&
+	S--dissociate/.git/objects/.some-hidden-dir
+	S--dissociate/.git/objects/.some-hidden-dir/.some-dot-file
+	S--dissociate/.git/objects/.some-hidden-dir/some-file
 	S--dissociate/.git/objects/.some-hidden-file
 	S--dissociate/.git/objects/some-dir
 	S--dissociate/.git/objects/some-dir/.some-dot-file
 	S--dissociate/.git/objects/some-dir/some-file
 	S--dissociate/.git/objects/some-file
+	S--local/.git/objects/.some-hidden-dir
+	S--local/.git/objects/.some-hidden-dir/.some-dot-file
+	S--local/.git/objects/.some-hidden-dir/some-file
 	S--local/.git/objects/.some-hidden-file
 	S--local/.git/objects/some-dir
 	S--local/.git/objects/some-dir/.some-dot-file
 	S--local/.git/objects/some-dir/some-file
 	S--local/.git/objects/some-file
+	S--no-hardlinks/.git/objects/.some-hidden-dir
+	S--no-hardlinks/.git/objects/.some-hidden-dir/.some-dot-file
+	S--no-hardlinks/.git/objects/.some-hidden-dir/some-file
 	S--no-hardlinks/.git/objects/.some-hidden-file
 	S--no-hardlinks/.git/objects/some-dir
 	S--no-hardlinks/.git/objects/some-dir/.some-dot-file