download/git: fix basename for files inside tarballs

RFC PATCH New Ricardo Martincoski 3 participants 5 messages Apr 5, 2018 – Apr 7, 2018

Share & tools

Ricardo Martincoski

author

2018-04-05 03:16 UTC #0 lore raw

RFC PATCH 1/1 download/git: fix basename for files inside tarballs

Commit "6d938bcb52 download: git: introduce cache feature" introduced a typo that makes the tarball to contain files without the package basename:

$ tar -tvf good-a238b1dfcd825d47d834af3c5223417c8411d90d.tar.gz
-rw-r--r-- 0/0               8 2017-10-14 02:10 ./file

Historically, all tarballs are generated with the basename:

$ tar -tvf good-a238b1dfcd825d47d834af3c5223417c8411d90d.tar.gz
-rw-r--r-- 0/0               8 2017-10-14 02:10 good-a238b1dfcd825d47d834af3c5223417c8411d90d/file

The hashes in the tree were calculated with the basename.

In the most common scenario, after the download ends the tarball is generated, the hash mismatches and the download mechanism falls back to use the tarball from http://sources.buildroot.net .

The problem can be reproduced by forcing the download of any git package PKG that has a hash file to check against:

$ make defconfig
$ ./utils/config --set-str BR2_BACKUP_SITE ""
$ BR2_DL_DIR=$(mktemp -d) make PKG-dirclean PKG-source

Fix the typo so the basename is really added to the files, that was clearly the intention of the code.

Signed-off-by Ricardo Martincoski <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Maxime Hadjinlian <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Yann E. MORIN <[email protected]>

---
NOTICE: linux-firmware is NOT fixed by this. I don't know why. I tried in my local computer a few other (other than the ones tested in Gitlab CI) git packages and they are fixed by this patch as well:
libuci
squashfs
ubus
x264
Could it be related to my setup?
tar (GNU tar) 1.29
git version 2.14.1

I detected this while debugging why this automated test now fails:
http://patchwork.ozlabs.org/patch/831742/

Before this patch:
https://gitlab.com/RicardoMartincoski/buildroot/pipelines/19997839

After this patch:
https://gitlab.com/RicardoMartincoski/buildroot/pipelines/19998472

---
 support/download/git | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

support/download/git +1 -1

index 58dbcd211c..f07195b0d1 100755
@@ -111,7 +111,7 @@ LC_ALL=C sort <"${output}.list" >"${output}.list.sorted"
 
 # Create GNU-format tarballs, since that's the format of the tarballs on
 # sources.buildroot.org and used in the *.hash files
-tar cf - --transform="s/^\.$/${basename}/" \
+tar cf - --transform="s/^\./${basename}/" \
 	--numeric-owner --owner=0 --group=0 --mtime="${date}" --format=gnu \
          -T "${output}.list.sorted" >"${output}.tar"
 gzip -6 -n <"${output}.tar" >"${output}"

Signature · 1 line

2.14.1

Peter Korsgaard

2018-04-05 06:30 UTC #1 lore raw

RFC PATCH 1/1 download/git: fix basename for files inside tarballs

Quoted 18 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Ricardo" == Ricardo Martincoski <[email protected]> writes:
Commit "6d938bcb52 download: git: introduce cache feature" introduced a typo that makes the tarball to contain files without the package basename:
$ tar -tvf good-a238b1dfcd825d47d834af3c5223417c8411d90d.tar.gz
-rw-r--r-- 0/0               8 2017-10-14 02:10 ./file
Historically, all tarballs are generated with the basename:
$ tar -tvf good-a238b1dfcd825d47d834af3c5223417c8411d90d.tar.gz
-rw-r--r-- 0/0               8 2017-10-14 02:10 good-a238b1dfcd825d47d834af3c5223417c8411d90d/file
The hashes in the tree were calculated with the basename.
In the most common scenario, after the download ends the tarball is generated, the hash mismatches and the download mechanism falls back to use the tarball from http://sources.buildroot.net .

Quoted 8 lines

The problem can be reproduced by forcing the download of any git package PKG that has a hash file to check against:
$ make defconfig
$ ./utils/config --set-str BR2_BACKUP_SITE ""
$ BR2_DL_DIR=$(mktemp -d) make PKG-dirclean PKG-source
Fix the typo so the basename is really added to the files, that was clearly the intention of the code.

Argh, indeed. Verified on the tremor package by moving the .hash file and my old tarball. Without the patch I get:

tar -tvf tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8.tar.gz | head
-rw-r--r-- 0/0 467 2018-03-19 21:08 ./CHANGELOG

-rw-r--r-- 0/0            1466 2018-03-19 21:08 ./COPYING
-rw-r--r-- 0/0            1553 2018-03-19 21:08 ./Makefile.am
-rw-r--r-- 0/0            1941 2018-03-19 21:08 ./README
-rw-r--r-- 0/0            1186 2018-03-19 21:08 ./Version_script.in
-rw-r--r-- 0/0            6534 2018-03-19 21:08 ./asm_arm.h
-rwxr-xr-x 0/0            3657 2018-03-19 21:08 ./autogen.sh
-rw-r--r-- 0/0            4639 2018-03-19 21:08 ./backends.h

-rw-r--r-- 0/0 14547 2018-03-19 21:08 ./block.c
-rw-r--r-- 0/0 1109 2018-03-19 21:08 ./block.h

And with the patch:

tar -tvf tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8.tar.gz | head
-rw-r--r-- 0/0 467 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/CHANGELOG

-rw-r--r-- 0/0            1466 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/COPYING
-rw-r--r-- 0/0            1553 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/Makefile.am
-rw-r--r-- 0/0            1941 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/README
-rw-r--r-- 0/0            1186 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/Version_script.in
-rw-r--r-- 0/0            6534 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/asm_arm.h
-rwxr-xr-x 0/0            3657 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/autogen.sh
-rw-r--r-- 0/0            4639 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/backends.h

-rw-r--r-- 0/0 14547 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/block.c
-rw-r--r-- 0/0 1109 2018-03-19 21:08 tremor-7c30a66346199f3f09017a09567c6c8a3a0eedc8/block.h

And the sha256sum matches our hash file.

Committed, thanks.

Quoted 6 lines

Signed-off-by Ricardo Martincoski <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
Cc Maxime Hadjinlian <[email protected]>
Cc Peter Korsgaard <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Yann E. MORIN <[email protected]>

Quoted 11 lines

---
NOTICE: linux-firmware is NOT fixed by this. I don't know why. I tried in my local computer a few other (other than the ones tested in Gitlab CI) git packages and they are fixed by this patch as well:
libuci
squashfs
ubus
x264
Could it be related to my setup?
tar (GNU tar) 1.29
git version 2.14.1

Hmm, it works here. I again moved my old tarball and .hash file:

Quoted 1 line

Quoted 1 line
Quoted 1 line
linux-firmware 65b1c68c63f974d72610db38dfae49861117cae2 Downloading

Initialized empty Git repository in /var/lib/downloads/linux-firmware/git/.git/ Fetching all references
remote: Counting objects: 6321, done.
remote: Compressing objects: 100% (23/23), done.
remote: Total 6321 (delta 13), reused 0 (delta 0)
Receiving objects: 100% (6321/6321), 167.76 MiB | 1.80 MiB/s, done. Resolving deltas: 100% (3925/3925), done.
From http://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware
* [new branch] master -> origin/master
warning: refname '65b1c68c63f974d72610db38dfae49861117cae2' is ambiguous. Git normally never creates a ref that ends with 40 hex characters because it will be ignored when you just specify 40-hex. These refs may be created by mistake. For example,

  git checkout -b $br $(git rev-parse ...)

where "$br" is somehow empty and a 40-hex ref is created. Please
examine these refs and maybe delete them. Turn this message off by
running "git config advice.objectNameWarning false"
WARNING: no hash file for linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2.tar.gz

Quoted 1 line

Quoted 1 line
Quoted 1 line
linux-firmware 65b1c68c63f974d72610db38dfae49861117cae2 Extracting

gzip -d -c /var/lib/downloads/linux-firmware/linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2.tar.gz | tar --strip-components=1 -C /home/peko/source/buildroot/output-git/build/linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2 -xf -

sha256sum ~download/linux-firmware/linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2.tar.gz 246a0de9ef9de5076e7695c9e9399b5358bb77278ef375ed9360466d00a1a39b /var/lib/downloads/linux-firmware/linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2.tar.gz

Which matches the .hash file and my old tarball.

So I guess there is something else going on. I'm using tar 1.29 and git 2.11.0.

Signature · 1 line

Bye, Peter Korsgaard

Ricardo Martincoski

author

2018-04-06 02:21 UTC #2 lore raw

RFC PATCH 1/1 download/git: fix basename for files inside tarballs

Hello,

Quoted 1 line · Peter Korsgaard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Ricardo" == Ricardo Martincoski writes:

[snip]

Quoted 1 line

Quoted 1 line
NOTICE: linux-firmware is NOT fixed by this. I don't know why.

[snip]

Quoted 5 lines

Quoted 3 lines
Could it be related to my setup?
tar (GNU tar) 1.29
git version 2.14.1
Hmm, it works here. I again moved my old tarball and .hash file:

[snip]

Quoted 1 line

So I guess there is something else going on. I'm using tar 1.29 and git 2.11.0.

It seems related to tar options. And we use the same version, so I don't understand the reason we get different results.

At the very end of 'tar --help' I see:
*This* tar defaults to:
--format=gnu -f- -b20 --quoting-style=escape --rmt-command=/usr/lib/tar/rmt --rsh-command=/usr/bin/rsh

I am using Ubuntu 17.10.

Inspecting the tarball, what makes linux-firmware uncommon is that it has symlinks to upper directories.
Based on my tests, it seems '--transform=' operates on the symlink destination as well. See at the end:
..."bin -> linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2./mrvl/"...

Based only on this, assuming there are no other variables, my tweak to the transform regexp still does not cover all cases. Sorry.

My tests:
I also moved the tarball and hash file and downloaded using 2018.02 and current master (e509e04dedd0). Sorry about the long lines, it's the raw output.

$ sha256sum 2018.02/linux-firmware-* master/linux-firmware/linux-firmware-*
d253f7abb32e0407bcc840a5bddba1dc20190bb4a4bb6d40512f279fde384671  2018.02/linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2.tar.gz
246a0de9ef9de5076e7695c9e9399b5358bb77278ef375ed9360466d00a1a39b  master/linux-firmware/linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2.tar.gz

$ tar -tvf 2018.02/linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2.tar.gz > 2018.02/log.txt
$ tar -tvf master/linux-firmware/linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2.tar.gz > master/log.txt

$ diff -U3 2018.02/log.txt master/log.txt | head
--- 2018.02/log.txt     2018-04-05 20:48:31.408302590 -0300
+++ master/log.txt      2018-04-05 20:48:49.031955640 -0300
@@ -893,8 +893,8 @@
 -rw-r--r-- 0/0            2464 2018-01-04 13:06 linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2/libertas/sd8686_v8_helper.bin
 -rw-r--r-- 0/0          124252 2018-01-04 13:06 linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2/libertas/sd8686_v9.bin
 -rw-r--r-- 0/0            2516 2018-01-04 13:06 linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2/libertas/sd8686_v9_helper.bin
-lrwxrwxrwx 0/0               0 2018-01-04 13:06 linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2/libertas/sd8688.bin -> ../mrvl/sd8688.bin
-lrwxrwxrwx 0/0               0 2018-01-04 13:06 linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2/libertas/sd8688_helper.bin -> ../mrvl/sd8688_helper.bin
+lrwxrwxrwx 0/0               0 2018-01-04 13:06 linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2/libertas/sd8688.bin -> linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2./mrvl/sd8688.bin
+lrwxrwxrwx 0/0               0 2018-01-04 13:06 linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2/libertas/sd8688_helper.bin -> linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2./mrvl/sd8688_helper.bin

Thoughts?

Regards,
Ricardo

Yann E. MORIN

2018-04-07 21:27 UTC #3 lore raw

RFC PATCH 1/1 download/git: fix basename for files inside tarballs

Ricardo, All,

On 2018-04-05 23:21 -0300, Ricardo Martincoski spake thusly:
[--SNIP--]

Quoted 5 lines

Inspecting the tarball, what makes linux-firmware uncommon is that it has symlinks to upper directories.
Based on my tests, it seems '--transform=' operates on the symlink destination as well. See at the end:
..."bin -> linux-firmware-65b1c68c63f974d72610db38dfae49861117cae2./mrvl/"...

I sent a simple fix:

    https://patchwork.ozlabs.org/patch/895967/

Care to give it a spin? ;-)

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

2018-04-05 17:09 UTC #4 lore raw

RFC PATCH 1/1 download/git: fix basename for files inside tarballs

In reply to Ricardo Martincoski

Ricardo, All,

On 2018-04-05 00:16 -0300, Ricardo Martincoski spake thusly:

Quoted 16 lines

Commit "6d938bcb52 download: git: introduce cache feature" introduced a typo that makes the tarball to contain files without the package basename:
$ tar -tvf good-a238b1dfcd825d47d834af3c5223417c8411d90d.tar.gz
-rw-r--r-- 0/0               8 2017-10-14 02:10 ./file
Historically, all tarballs are generated with the basename:
$ tar -tvf good-a238b1dfcd825d47d834af3c5223417c8411d90d.tar.gz
-rw-r--r-- 0/0               8 2017-10-14 02:10 good-a238b1dfcd825d47d834af3c5223417c8411d90d/file
The hashes in the tree were calculated with the basename.
In the most common scenario, after the download ends the tarball is generated, the hash mismatches and the download mechanism falls back to use the tarball from http://sources.buildroot.net .

Quoted 8 lines

The problem can be reproduced by forcing the download of any git package PKG that has a hash file to check against:
$ make defconfig
$ ./utils/config --set-str BR2_BACKUP_SITE ""
$ BR2_DL_DIR=$(mktemp -d) make PKG-dirclean PKG-source
Fix the typo so the basename is really added to the files, that was clearly the intention of the code.

Yes, definitely the intention.

I had that one fix pending locally and did not have time to send it yesterday evenong, so thinaks for fixing it faster! :-)

It's already been applied, otherwise I'd have given my ACK.

Regards,
Yann E. MORIN.

Quoted 46 lines

Signed-off-by Ricardo Martincoski <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
Cc Maxime Hadjinlian <[email protected]>
Cc Peter Korsgaard <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Yann E. MORIN <[email protected]>
---
NOTICE: linux-firmware is NOT fixed by this. I don't know why. I tried in my local computer a few other (other than the ones tested in Gitlab CI) git packages and they are fixed by this patch as well:
libuci
squashfs
ubus
x264
Could it be related to my setup?
tar (GNU tar) 1.29
git version 2.14.1
I detected this while debugging why this automated test now fails:
http://patchwork.ozlabs.org/patch/831742/
Before this patch:
https://gitlab.com/RicardoMartincoski/buildroot/pipelines/19997839
After this patch:
https://gitlab.com/RicardoMartincoski/buildroot/pipelines/19998472
---
 support/download/git | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
support/download/git +1 -1
index 58dbcd211c..f07195b0d1 100755
@@ -111,7 +111,7 @@ LC_ALL=C sort <"${output}.list" >"${output}.list.sorted"
 
 # Create GNU-format tarballs, since that's the format of the tarballs on
 # sources.buildroot.org and used in the *.hash files
-tar cf - --transform="s/^\.$/${basename}/" \
+tar cf - --transform="s/^\./${basename}/" \
 	--numeric-owner --owner=0 --group=0 --mtime="${date}" --format=gnu \
          -T "${output}.list.sorted" >"${output}.tar"
 gzip -6 -n <"${output}.tar" >"${output}"
Signature · 1 line
2.14.1

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'