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

[Junio C Hamano <gitster@pobox.com>]

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?

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

>  		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,^([^/]+)/,;