libbpf / ci

BPF CI
Other
7 stars 22 forks source link

Upgrade default llvm version to version 17 #82

Closed insearchoflosttime closed 1 year ago

insearchoflosttime commented 1 year ago

Currently, bpf CI is unable to locate llvm-16 on aarch64/gcc, aarch64/llvm-16, and s390x/gcc [0].

This change upgrades the default llvm version to 17.

[0] https://github.com/kernel-patches/bpf/actions/runs/4040302668

Signed-off by: Joanne Koong joannekoong@gmail.com

danielocfb commented 1 year ago

The change seems fine, but we should make sure that it actually fixes the issue. Also, I am not entirely clear (haven't looked) why x86_64 wasn't failing. Perhaps for that architecture the snapshot does still have LLVM 16 but not yet 17 available? That would be a royal mess.

Anyway, can you please schedule a run (unfortunately, that's not done yet when creating a pull request) with your changes? At a high level, I think you'll need something like https://github.com/danielocfb/vmtest/commit/c7d16da7823aaa89f3f3f0d4453e0f1838ae9140 that makes sure to use your fork of kernel-patches/ci with the new LLVM version. Then, apply that on top of your current kernel-patches/bpf checkout. I use something like:

#!/bin/sh

cp -a /data/users/muellerd/vmtest/.github .
git add -f .github
content=$(cd /data/users/muellerd/vmtest/; echo *)
cp -a /data/users/muellerd/vmtest/* .
git add --force ${content}
git commit -m 'Adding CI files'

Push that to your fork and create a pull request against kernel-patches/bpf-next. That should trigger a run.

Alternatively, it should be possible (and potentially easier, but I haven't tried that out myself) to directly adjust the paths in a fork of https://github.com/kernel-patches/bpf/tree/bpf-next_base (this branch has vmtest files automatically added to kernel-patches/bpf) and create a pull request.

insearchoflosttime commented 1 year ago

Also, I am not entirely clear (haven't looked) why x86_64 wasn't failing.

llvm-16 already is manually installed on x86_64 (according to the logs here)

Perhaps for that architecture the snapshot does still have LLVM 16 but not yet 17 available?

Interesting! is this possible that it won't have 17 available yet? My understanding is that a new llvm version will always be available for all architectures.

Awesome, I'll manually schedule a run. Thanks for the steps!

danielocfb commented 1 year ago

Perhaps for that architecture the snapshot does still have LLVM 16 but not yet 17 available?

Interesting! is this possible that it won't have 17 available yet? My understanding is that a new llvm version will always be available for all architectures.

Yeah, but I suspect they could have different "release" schedules and so I suppose we could end up in a situation where, say, s390x and arm snapshots have already migrated to LLVM 17 but x86_64 has not yet seen a new snapshot published and so it still contains only LLVM 16 bits. I am just hypothesizing here based on what I believe to see. I have no knowledge of how things work internally.

Edit: If that were the case (which was not anticipated...), we may have to disable some architectures temporarily, I suppose.

insearchoflosttime commented 1 year ago

The run has been scheduled here: https://github.com/kernel-patches/bpf/pull/4420

update: it succeeded!

danielocfb commented 1 year ago

I think we don't need to wait for all tests to finish. It can't make matters worse than they are right now :-) Merging.