-
Notifications
You must be signed in to change notification settings - Fork 21
upgrade to tar 1.29 #238
base: master
Are you sure you want to change the base?
upgrade to tar 1.29 #238
Conversation
Travis, the current CI system, uses a very old Ubuntu distribution whose package 'coreutils' does not include 'realpath'. Or, 'coreutils' has had it but the CI image maintainer has stripped it from the final image for consistency's sake. Anyway, this gets us realpath in one way or another.
can you squash please |
aci-builder/bin-run/builder.go
Outdated
return errs.WithEF(err, b.fields, "Failed to tar aci") | ||
if err2 := common.ExecCmd(buildersTar, params...); err2 != nil { | ||
// If that failed, output the original error nevertheless. | ||
logs.WithFields(b.fields).WithField("params", params).Error("Parameters to 'tar' within the builder") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should not log in error here, you should log as debug. or better add fields to errs just under so it will be propagated to where the error will end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It returns in the very next line. But I will add this to the fields.
aci-builder/build.sh
Outdated
@@ -16,26 +16,53 @@ mkdir -p ${rootfs}/dgr ${rootfs}/usr/bin | |||
GOOS=linux GOARCH=amd64 go build --ldflags '-s -w -extldflags "-static"' -o ${rootfs}/dgr/builder/stage1/run ${dir}/bin-run | |||
upx ${rootfs}/dgr/builder/stage1/run | |||
|
|||
sudo tar xf ${dir}/rootfs.tar.xz -C ${rootfs}/dgr/ | |||
: ${tar:="$(realpath "${dir}/files/dgr/usr/bin/tar")"} | |||
if [[ ! -x "${tar}" ]] || ! "${tar}" --help | grep -q -F sort; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that, we should update buildroot instead since 1.29 is in here.
But since it's up to me to do it, I can accept it for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this approach. I've had the binary tar, as easily reversible commit, already in a separat commit.
Don't add tar binary to repository. Please remove it and add it as ignored until we update buildroot |
squash, change the log stuff and remove binary from repo and it's good to me |
I wish we could do this entirely without resorting to the tar binary. But all implementations in Go are by far not on its level feature-wise. And indeed, won't be in the next months: GNU tools can and use Perl regular expressions, Go's implementation a reduced set by design. So even if works on a Go implementation started today (which sounds trivial), that'll be a complex roadblock for whoever fails to plan for this. An upgraded tar in the buildroot won't do it: We already need to have a defined tar on the host to build dgr. Except perhaps you want us to extract it from the buildroot as part of I will squash this into three commits:
|
it's true that it would be better with a go version Also like I sayed, it's a temporary fix and will be included in the buildroot later. |
038d56f
to
c9ebe9c
Compare
In order to have the most recent 'tar' – which is necessary for reproducible builds – we build it ourselves. Future commits will use the henceforth available options `--sort`, `--clamp-mtime`, `--exclude`, as well as `--transform`.
Enables dgr to work on hosts that don't have any tar. (See also blablacar#217.) Sorts the contents to have a reproducible order, but pulls the manifest file to the front for fast access. Sorted contents of ACIs allow for easier comparison, and usage of tools such as zsync, and deduplication on the server. The price, sorting by 'tar', is cheap. zap the "chdir-acrobatique", use `tar -C`: dgr changes paths and performs needless renames, which result in mayhem if the process quits prematurely or the timing were off. The solution is to use tar's `-C` param and transform the filenames. closes blablacar#210
Addressed. Most of the time went into pleasing Travis. |
I'm looking at your PR to try to merge it but I see you are mixing 2 things. Building dgr as reproductible and building aci as reproductible. I merged the first PR that is using latest version of tar to build reproductible aci during the build. I did not included the rest for the moment since it's building tar to use it to build dgr. on another PR @nyodas updated buildroot with latest version of tar so you should be able to go ahead with building reproductible aci. Now about building dgr as reproductible, instead of downloading tar and using it, we should build dgr inside a container to solve this, like @nyodas did to do buildroot. |
We will need the following options to tar for reproducible builds. Therefore this PR upgrades the version dgr ships with to 1.29, but still calls the host's tar by default.
--sort
--clamp-mtime
--exclude
The first two commits prepare the CI and the required version of tar.
Any later commits progressively use more of the new features.
Commits are – following the industry best practices – split into logical units. That is, every commit motivates its own use-case, and/or can be reverted independently later.
Please note that Github (the web frontend) could display the commits in the wrong order.