Re: [PATCH] archive: Store checksum correctly

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 24, 2019, 01:04 UTC
Message-ID: <20190724010445.GA164908@google.com>
In-Reply-To: <20190723025736.23036-1-mattst88@gmail.com>

(cc: René Scharfe, "git archive" expert) Matt Turner wrote:

Show 18 quoted lines

> tar2sqfs (part of https://github.com/topics/tar2sqfs) rejects tarballs
> made with git archive with the message
>
>     invalid tar header checksum!
>
> tar2sqfs recomputes the tarball's checksum to verify it. Its checksum
> implementation agrees with GNU tar, which contains a comment that states
>
>     Fill in the checksum field.  It's formatted differently from the
>     other fields: it has [6] digits, a null, then a space ...
>
> Correcting this allows tar2sqfs to correctly process the tarballs made
> by git archive.
>
> Signed-off-by: Matt Turner <mattst88@gmail.com>
> ---
>  archive-tar.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Nice. Is this something that can be covered in tests as well? (See t500* for existing "git archive" tests, and see test_lazy_prereq in case you'd like the test to use an external tool like tar2sqfs that not all users may have.)

Thanks, Jonathan

(patch left unsnipped for reference)

Show 15 quoted lines

> diff --git a/archive-tar.c b/archive-tar.c
> index 3e53aac1e6..f9a157bfd1 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -215,7 +215,9 @@ static void prepare_header(struct archiver_args *args,
>  	memcpy(header->magic, "ustar", 6);
>  	memcpy(header->version, "00", 2);
>  
> -	xsnprintf(header->chksum, sizeof(header->chksum), "%07o", ustar_header_chksum(header));
> +	xsnprintf(header->chksum, sizeof(header->chksum), "%06o", ustar_header_chksum(header));
> +	header->chksum[6] = '\0';
> +	header->chksum[7] = ' ';
>  }
>  
>  static void write_extended_header(struct archiver_args *args,