-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cirrus: use fastvm for builds #24120
cirrus: use fastvm for builds #24120
Conversation
This seems to safe around 3mins, not a lot but at least something. I will look to see what parts of the build jobs are slow to see if there are some ways to make it faster because AFAICT most time does not seem to be spend compiling so I think there is some room in our docs/validation process. |
898332b
to
5812f8e
Compare
5812f8e
to
5d2390e
Compare
Cockpit tests failed for commit 5d2390e07c1bfde20bc48295974897e66f371e06. @martinpitt, @jelly, @mvollmer please check. |
8ac4299
to
d9e0107
Compare
Cockpit tests failed for commit d9e0107f9610b01eb890196e64e8276be63182c6. @martinpitt, @jelly, @mvollmer please check. |
d9e0107
to
2e5ed0b
Compare
Windows does not have zstd by default so we need to install it. In particular I am looking at switching the repo archive to zstd as this makes things much faster (over 1min in podman)[1] but the windows testing is unable to extract that. While archiver added zstd support a while back it is not in the version that is on chocolatey which seems a bit out of date. [1] containers/podman#24120 Signed-off-by: Paul Holzinger <[email protected]>
258f33f
to
3f3f597
Compare
@edsantiago PTAL (low prio) I think this is good to review, of course this will fail CI until #24227 is merged then I will rebase and mark as ready. |
Builds now take over 10 mins, given golang compilation is parallelized by default we can give more cores to speed it up. Signed-off-by: Paul Holzinger <[email protected]>
This target runs several scripts in serial but they do not have any dependencies so we can split them all into their own target so that make -j can run the targets in parallel to speed this up. Signed-off-by: Paul Holzinger <[email protected]>
The doc generation and the validate-binaries target can be run in parallel as they do not depend on each other and a specific ordering. As such we pass -j $(nproc) but also --output-sync=target to ensure the output is not intermixed between several targets which could be harder to read in case of errors. Hower dus the complex podman-release target we can run podman-release and validate-binaries at the same time as the dependencies are not right and we run podman-release first in order to get the correct binaires build. Signed-off-by: Paul Holzinger <[email protected]>
The current podman-release-%.tar.gz target does a lot more then just checking if we can build for the given arch, in particular it first builds a local podman-remote for the remote-docs.sh script. This makes things slow as we compile several things and then builda and package the docs. Given the docs are not arch specific there is realy no point in doing all that work. All we care about is if the bianries can build on other arches to catch compile issue for otherwise untested arches. This should make the CI Alt Arch. tasks much faster. Signed-off-by: Paul Holzinger <[email protected]>
In particular the main build task already did a make vendor and a regeneration of the completion scripts. This means the first tre_status would pick up both changes so the suggestion would be off. And rerunning the same thing again here just makes thing slower than they need to be. In particular there was the bug that make completion even rebuild podman because generate-bindings obviously updates the timestamps of the files as they are overwritten. We do however must run generate-bindings as it was not run before. Signed-off-by: Paul Holzinger <[email protected]>
The script for aarch is exactly the same so there doesn't seem to be a reason to duplciate it. Signed-off-by: Paul Holzinger <[email protected]>
The repo tar process took over 1:20 min, with zstd it takes less than 10s so we safe over a minute by doing this. Signed-off-by: Paul Holzinger <[email protected]>
3f3f597
to
9e35fea
Compare
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.
LGTM
.cirrus.yml
Outdated
time tar xjf /tmp/repo.tbz -C $GOSRC | ||
echo "$ARTCURL/Build%20for%20${DISTRO_NV}/repo/repo.tar.zst" | ||
time $ARTCURL/Build%20for%20${DISTRO_NV}/repo/repo.tar.zst | ||
time tar --zstd -xf /tmp/repo.tar.zst -C $GOSRC |
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.
From info tar
:
Reading compressed archive is even simpler: you don't need to specify
any additional options as GNU ‘tar’ recognizes its format automatically.
I have a slight predilection for eliminating the --zstd
, and just letting tar
figure it out itself. WDYT?
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 can try that and see if it works, the one thing that really messed me up is that we extract on macos as well and there is no gnu tar there so I am not sure if that can figure it out too. If not we end up with a case where one tar command has it and the other does not which might cause more confusion.
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.
Oh good point. I hate macs so much.
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.
add this as extra commit let's see if it works or not, bsd tar (used on macos) can do that to per google search in theory
I tried to use tar on my local mac to test it but I just got this error and did not want to spend more time on it to figure out what is wrong there...
tar: Can't launch external program: zstd --no-check -3
If doesn't work I can just revert the commit and if it does work I can squash it into the zstd change if you prefer that.
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.
LGTM. I'm curious about the tree_status
changes, especially how much we can rely on the state being what you describe, but will go along with it.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, lsm5, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tar should be smart enough to check the magic byte and use the correct decompression algo based on that so there is no need to spell it out explictly. Signed-off-by: Paul Holzinger <[email protected]>
/lgtm no need to squash. Thank you! |
Builds now take over 10 mins, given golang compilation is parallelized by default we can give more cores to speed it up.
Does this PR introduce a user-facing change?