-
Notifications
You must be signed in to change notification settings - Fork 34
Migrate clang/llvm to package build image #101
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
Conversation
|
I believe fedora-37-test is currently used for all EDK 2 CI https://github.com/tianocore/edk2/blob/master/.azurepipelines/templates/defaults.yml#L12 - specifically, for the CI which runs as Ubuntu-GCC5 - so possibly only fedora-40 would need this update? |
|
Here is the PR that switches the CI to the Fedora 40 image: tianocore/edk2#6261 |
|
The default gcc for Ubuntu 22.04 is 11, so I guess the |
|
It was just to match the version of GCC in the Fedora images. |
Thanks for the response. Possibly it is more 'useful' (e.g. for CI), in that case, to find out whether things build or not in the default current gcc in Ubuntu? (In fact, they do!) |
|
"Building" and "working" are two different things. I don't recall any specific issues with the default GCC on Ubuntu22, but I know we (NVIDIA) couldn't use the default GCC-9 in Ubuntu20. That's probably where the habit of using update-alternative began. The code originated with Ubuntu20 and was carried forward to Ubuntu22. Anyway, I recommend:
Of course, that means deviating from Fedora. But from what I've seen, Fedora is a faster moving target and Ubuntu won't keep up anyway. |
|
Any intention to work on this again? Or can I close this PR? |
|
Hi @osteffenrh - Actually I am hoping to return to this shortly. |
e3c8b06 to
27551c9
Compare
|
@jgarver @osteffenrh - Hi! I'm back on this again. Initial attempt at integrating CLANGDWARF/PDB builds into CI is finding new genuine code errors tianocore/edk2#11785, pretty much as expected and - hopefully - confirming the justification for this work. The above is a new one from actually trying to start to integrate CLANGXXX into EDK2 CI, but note that these less serious issues tianocore/edk2#11738 were found while running clang (including but not only XCODE5) builds, as part of this work. (And we have continued to find and submit fixes for similar bugs and issues going back over a few years now, from building on CLANGXXX and XCODE5 toolchains.) |
Signed-off-by: Mike Beaton <[email protected]>
27551c9 to
939ac91
Compare
|
For now, I've removed the clang change from the Ubuntu image, since AFAICT the only CI step in the build host which notices |
|
What is your plan? Replace gcc with clang in all Linux CI jobs? Yeah, the Linux CI still uses the GCC5 toolchain option (although with a gcc15 now).
Which ones do you mean? |
Not at all. The plan is to clone GCC5 CI to CLANGDWARF and CLANGPDB, and run it as well as GCC5 CI. Likely, this will be far to much to include, so at least minimal CLANGXXX CI clones of .azurepipelines/Ubuntu-GCC.yml, ArmVirtPkg/PlatformCI/.azurepipelines/Ubuntu-GCC.yml and OvmfPkg/PlatformCI/.azurepipelines/Ubuntu-GCC.yml, or similar. The reason being, we have found many many real bugs by building with clang toolchains (CLANGPDB, CLANGDWARF, XCODE5), which we have been submitting back to EDK2 for some years, and which we have to fix for our CI (including CLANGDWARF, CLANGPDB and our own fully working fork of XCODE5 (including relocations, using some additional non-standard tools)) to complete. But it is not just for our sake, or just to run XCODE5, these are real bugs which would not be entering the EDK2 codebase if at least basic clang builds were in CI. (In addition, of course, this also keeps these officially supported CLANG toolchains building.) I am open to discussion as to exactly which CI steps would be cloned to also run in CLANG, but I would very much like it to be some of them
I may be wrong about why some of my CLANG package builds are stopping during CI, with cryptic, low-information errors, even though some are building and completing. I was under the impression that it might be to do with the CI unit test runner (which I believe runs in the CI Ubuntu 'host' vm, not the CI Fedora 'guest' vm, and which I believe only supports GCC). I am still investigating this. It might help to clarify that the Acidanthera CI that I am referring to (which includes CLANG) uses GitHub runners, with scripts controlling traditional EDK2 builds, not the Here are some example bugfixes: tianocore/edk2#11794 The ones attributed to being found in XCODE5 are also found in CLANG builds (but are not found in GCC), if the warnings are turned back on tianocore/edk2#11761. Oh, and by the way, I am finding more, now, as I clone and enable CLANG CI in my own private runners, while working on this (e.g. tianocore/edk2#11785, but there are others I have not had time to try to make fixes for yet). From past and current experience, some of these when I check will turn out to be fixed but never reported in Acidanthera, some will be new (where your CI hits any code which ours doesn't) |
|
Sounds good, nice work. |
|
Merged. You probably want diff --git a/.azurepipelines/templates/defaults.yml b/.azurepipelines/templates/defaults.yml
index e94e6351cbb3..e67bb9bed441 100644
--- a/.azurepipelines/templates/defaults.yml
+++ b/.azurepipelines/templates/defaults.yml
@@ -9,4 +9,4 @@
variables:
default_python_version: "3.12"
- default_linux_image: "ghcr.io/tianocore/containers/fedora-43-test:bde74f8"
+ default_linux_image: "ghcr.io/tianocore/containers/fedora-43-test:b562821"since the CI does nut run on the "latest" tag. |
|
Tyvm - I'm currently running in my own Azure Pipelines with my own fork of both |
Description
This PR proposes moving clang+llvm from -dev to -build, with a view to making a future PR offering CLANGDWARF and CLANGPDB CI to EDK 2.
I note thatgccin the Ubuntu image usesupdate-alternativesto link to a fixed version - while this is potentially somewhat fragile (i.e. there is unfortunately no automatic way to link all and only the bins provided by a given version), it does allow a fixed version. On the other hand the Fedora image doesn't use anything similar.I haven't added a fixed version withupdate-alternativesforclangin the Ubuntu image, though am happy to update the PR do so; with either the bins which appear to be needed to build, or all bins which seem to be provided with the given version - although this is quite an extensive list in the case of llvm-14 (75 bins).EDIT: Strikethoughs as per this comment.
Issue #99
Containers Affected
ubuntu-22-build
ubuntu-22-test
ubuntu-22-dev
fedora-40-build
fedora-40-test
fedora-40-dev