commaai / agnos-builder

Build AGNOS, the operating system for your comma 3/3X
33 stars 54 forks source link

github actions/checkout clean function removing files #366

Closed ParsaNobahari closed 3 weeks ago

ParsaNobahari commented 3 weeks ago

looking at checkout workflow running, https://github.com/ParsaNobahari/agnos-builder/actions/runs/10748426412/job/29812960368#step:2:38 at lines between 40 to 50, because it was a re-run, we can see that it is trying to clean repository which is the default behavior for all untracked files and directories (git clean -ffdx && git reset --hard HEAD).

but whilst looking at the files that it is deleting, it could turn into a real bottleneck for re-runs: Removing tools/SecImage/ Removing tools/aarch64-linux-android-4.9/ Removing tools/aarch64-linux-gnu-gcc/ Removing tools/llvm-arm-toolchain-ship/ Removing ubuntu-base-24.04.1-base-arm64.tar.gz

This turns into processing bottleneck for aarch64-linux-android-4.9.tar.gz aarch64-linux-gnu-gcc.tar.gz llvm-arm-toolchain-ship.tar.gz llvm-arm-toolchain-ship.tar.gz since we have to extract them for every re-run. and it specially doesn't make any sense since these files are at this point 3 years old and I don't think they are changing anytime soon. so why don't have them cached and extracted already?

It's much worse for ubuntu-base-24.04.1-base-arm64.tar.gz because we have to download it everytime. why? should we not delete it because we have the checksum stored in env variable so we are not going to download something else everytime we curl it, unless to curl the checksum from UBUNTU_BASE_URL from checksum file and run it through regex if we are downloading a different version everytime? and if going to delete it, why checking for it if it already has been cloned or not: line 39: # Download Ubuntu Base if not done already line 40: if [ ! -f $UBUNTU_FILE ]; then

I propose that we add ubuntu base to repo like other archives metioned above. it would make a lot of it more seemless at 30MB.

for now, I just changed build.yaml to https://github.com/commaai/agnos-builder/compare/master...ParsaNobahari:agnos-builder:build-checkout-caching by disabling default cleaning and adding custom one. I only tested if it runs at all for now: https://github.com/ParsaNobahari/agnos-builder/actions/runs/10841905792/job/30086668511 getting a full run and re-run would be more wise but as long as there being 5 exceptions, it shouldn't matter. It's basically up to you.

andiradulescu commented 3 weeks ago
ParsaNobahari commented 3 weeks ago

tools are not extracted on arm64 builders used in CI

sorry for my ignorance here. I just assumed that commaai is cross-compiling their softwares on x86 and flashing it to their devices. aren't they using x86 runners for CI?

on CI builders there is no clean since those files are not there in the first place

Yes. I've looked at workflow runs on agnos repo before posting this. I thought that would be because of starting a new runner for each run. on first run there is no cleanup being done.

the ubuntu image is downloaded on every run, so we get a fail when the 24.04 image is getting updated

makes sense. me and adeebshihadeh have the same idea: https://github.com/commaai/agnos-builder/pull/332#issuecomment-2325161018 if you agree with this approach, you mind I make a pr to download the checksum file and do the checking?

andiradulescu commented 3 weeks ago

sorry for my ignorance here. I just assumed that commaai is cross-compiling their softwares on x86 and flashing it to their devices. aren't they using x86 runners for CI?

no, using arm64 runners in CI - explained here

if you agree with this approach, you mind I make a pr to download the checksum file and do the checking?

why would this be needed? straight answer, no