threads / patch / 42995

patchimport-tars: support hard links

Subject: [PATCH] import-tars: support hard links

## tl;dr

3 messages between Aug 3, 2016 and Aug 4, 2016. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Johannes Schindelin· Aug 3, 2016, 13:30 UTC · lore

Previously, we simply treated hard links as if they were plain files with size 0, ignoring the link type "1" and hence the link target.

What we should do instead, of course, is to use the link target to get at the import mark for the contents, even if we cannot recreate the hard link per se, as Git has no concept of hard links.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/import-tars-hardlink-v1
 contrib/fast-import/import-tars.perl | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)
Show changes to contrib/fast-import/import-tars.perl +20 −11
diff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl
index 95438e1..d60b431 100755
--- a/contrib/fast-import/import-tars.perl
+++ b/contrib/fast-import/import-tars.perl
@@ -96,18 +96,21 @@ foreach my $tar_file (@ARGV)
 		$mtime = oct $mtime;
 		next if $typeflag == 5; # directory
 
-		print FI "blob\n", "mark :$next_mark\n";
-		if ($typeflag == 2) { # symbolic link
-			print FI "data ", length($linkname), "\n", $linkname;
-			$mode = 0120000;
-		} else {
-			print FI "data $size\n";
-			while ($size > 0 && read(I, $_, 512) == 512) {
-				print FI substr($_, 0, $size);
-				$size -= 512;
+		if ($typeflag != 1) { # handle hard links later
+			print FI "blob\n", "mark :$next_mark\n";
+			if ($typeflag == 2) { # symbolic link
+				print FI "data ", length($linkname), "\n",
+					$linkname;
+				$mode = 0120000;
+			} else {
+				print FI "data $size\n";
+				while ($size > 0 && read(I, $_, 512) == 512) {
+					print FI substr($_, 0, $size);
+					$size -= 512;
+				}
 			}
+			print FI "\n";
 		}
-		print FI "\n";
 
 		my $path;
 		if ($prefix) {
@@ -115,7 +118,13 @@ foreach my $tar_file (@ARGV)
 		} else {
 			$path = "$name";
 		}
-		$files{$path} = [$next_mark++, $mode];
+
+		if ($typeflag == 1) { # hard link
+			$linkname = "$prefix/$linkname" if $prefix;
+			$files{$path} = [ $files{$linkname}->[0], $mode ];
+		} else {
+			$files{$path} = [$next_mark++, $mode];
+		}
 
 		$author_time = $mtime if $mtime > $author_time;
 		$path =~ m,^([^/]+)/,;

-- 
2.9.0.281.g286a8d9

base-commit: f8f7adce9fc50a11a764d57815602dcb818d1816
Junio C Hamano· Aug 3, 2016, 16:45 UTC · re: Johannes Schindelin · lore

Re: [PATCH] import-tars: support hard links

Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> Previously, we simply treated hard links as if they were plain files
> with size 0, ignoring the link type "1" and hence the link target.
Nicely spotted and explained.
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Published-As: https://github.com/dscho/git/releases/tag/import-tars-hardlink-v1

A link to a page that lets you download entire source tarball is not very useful to most people, except for those who want "this exact change on top of some unknown base which may or may not have other things they need", which I think is a minority.

Can you also (or instead) point at a branch/tag that people can do
	git fetch $repo $branch
more easily?
Show 36 quoted lines
>  contrib/fast-import/import-tars.perl | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl
> index 95438e1..d60b431 100755
> --- a/contrib/fast-import/import-tars.perl
> +++ b/contrib/fast-import/import-tars.perl
> @@ -96,18 +96,21 @@ foreach my $tar_file (@ARGV)
>  		$mtime = oct $mtime;
>  		next if $typeflag == 5; # directory
>  
> -		print FI "blob\n", "mark :$next_mark\n";
> -		if ($typeflag == 2) { # symbolic link
> -			print FI "data ", length($linkname), "\n", $linkname;
> -			$mode = 0120000;
> -		} else {
> -			print FI "data $size\n";
> -			while ($size > 0 && read(I, $_, 512) == 512) {
> -				print FI substr($_, 0, $size);
> -				$size -= 512;
> +		if ($typeflag != 1) { # handle hard links later
> +			print FI "blob\n", "mark :$next_mark\n";
> +			if ($typeflag == 2) { # symbolic link
> +				print FI "data ", length($linkname), "\n",
> +					$linkname;
> +				$mode = 0120000;
> +			} else {
> +				print FI "data $size\n";
> +				while ($size > 0 && read(I, $_, 512) == 512) {
> +					print FI substr($_, 0, $size);
> +					$size -= 512;
> +				}
>  			}
> +			print FI "\n";
>  		}
> -		print FI "\n";

The resulting if/else cascade initially looked a bit unnatural (naively I would have expected that a new "elsif ($typeflag == 1)" would be inserted before the final "else" currently is). Because you have to do avoid giving a new mark to hardlink entries, and that fact would not change regardless of what values of $typeflag other than 1 exists in the imported tars, so I think the resulting code structure makes a lot of sense. We may want to add more elsif to notice and ignore/warn/substitute things like CHRTYPE/BLKTYPE by enhancing the if/else cascade inside the "if ($typeflag != 1)" introduced by this patch, but the code structure does not have to change when such an enhancement happens.

Nicely done.
Show 17 quoted lines
>  		my $path;
>  		if ($prefix) {
> @@ -115,7 +118,13 @@ foreach my $tar_file (@ARGV)
>  		} else {
>  			$path = "$name";
>  		}
> -		$files{$path} = [$next_mark++, $mode];
> +
> +		if ($typeflag == 1) { # hard link
> +			$linkname = "$prefix/$linkname" if $prefix;
> +			$files{$path} = [ $files{$linkname}->[0], $mode ];
> +		} else {
> +			$files{$path} = [$next_mark++, $mode];
> +		}
>  
>  		$author_time = $mtime if $mtime > $author_time;
>  		$path =~ m,^([^/]+)/,;
Johannes Schindelin· Aug 4, 2016, 14:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] import-tars: support hard links

Hi Junio,
On Wed, 3 Aug 2016, Junio C Hamano wrote:
Show 9 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > ---
> > Published-As: https://github.com/dscho/git/releases/tag/import-tars-hardlink-v1
> 
> A link to a page that lets you download entire source tarball is not
> very useful to most people, except for those who want "this exact
> change on top of some unknown base which may or may not have other
> things they need", which I think is a minority.

True. I added a second line that describes how to fetch it (see the t5533 patch I just sent out).

Ciao, Dscho

← back to recent threads