Skip to content
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

Migrate clang/llvm to build images #101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mikebeaton
Copy link

@mikebeaton mikebeaton commented Oct 2, 2024

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 that gcc in the Ubuntu image uses update-alternatives to 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 with update-alternatives for clang in 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).

Issue #99

Containers Affected

ubuntu-22-build
ubuntu-22-test
ubuntu-22-dev
fedora-40-build
fedora-40-test
fedora-40-dev

@mikebeaton
Copy link
Author

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?

@osteffenrh
Copy link
Contributor

Here is the PR that switches the CI to the Fedora 40 image: tianocore/edk2#6261

@mikebeaton
Copy link
Author

mikebeaton commented Oct 2, 2024

The default gcc for Ubuntu 22.04 is 11, so I guess the update-alternatives links are required, if gcc 12 is required. On the other hand, AFACIT gcc 11 builds everything in EDK 2 fine... The default clang for Ubuntu 22.04, version 14, also builds EDK 2 fine, so I can't see a need to use update-alternatives there - but then again, I cannot yet clearly see what the original need was for update-alternatives for gcc. I don't know if @jgarver can comment?

@jgarver
Copy link
Contributor

jgarver commented Oct 2, 2024

It was just to match the version of GCC in the Fedora images.

@mikebeaton
Copy link
Author

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!)

@jgarver
Copy link
Contributor

jgarver commented Oct 2, 2024

"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:

  • Keeping GCC as is in Ubuntu22.
  • Not using update-alternative for clang, until we find a reason to not like the default version.
  • Trying to move to the default GCC when we add Ubuntu24 support.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants