dotnet / dotnet-buildtools-prereqs-docker

Used to maintain the Docker images hosted at the mcr.microsoft.com/dotnet-buildtools/prereqs image repository
MIT License
53 stars 102 forks source link

Update Ubuntu 22.04 crossdeps to install llvm 17 #944

Closed am11 closed 6 months ago

am11 commented 6 months ago

Context https://github.com/dotnet/runtime/pull/96349#issuecomment-1871886073.

dotnet-issue-labeler[bot] commented 6 months ago

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

am11 commented 6 months ago

@akoeplinger, thanks for merging this. Do you know why https://github.com/dotnet/versions/blob/0412008aa77369c93ae8e702e2a7029f818659b5/build-info/docker/image-info.dotnet-dotnet-buildtools-prereqs-docker-main.json#L3510 hasn't picked it up? Usually it takes an hour or two after the PR merge here when new tags are published. This seems to be stuck in some internal leg. Probably worth checking if one of the recent PR regressed the build?

akoeplinger commented 6 months ago

@am11 yup it was blocked internally (unrelated to the PRs), I fixed the issue and we should see the update in about an hour.

akoeplinger commented 6 months ago

@am11 btw. we should probably look into adding a cbl-mariner based cross image so it matches what we use for the main architectures (x64, arm64 etc)

Looks like only riscv64, ppc64le and s390x are still using the ubuntu-based image: https://github.com/dotnet/runtime/blob/44fd111025edf69907e2a1fd1de8b2ebbbe1e6b3/eng/pipelines/common/templates/pipeline-with-resources.yml#L67-L80

am11 commented 6 months ago

Latest staging tag ubuntu-22.04-cross-riscv64-20230215165123-0ea3466 still doesn't have clang-17 yet, so I guess it built the previous commit and not the latest one (yet), or something else went wrong? We should see /usr/bin/clang-17 instead of /usr/bin/clang-15 in the container.

btw. we should probably look into adding a cbl-mariner based cross image so it matches what we use for the main architectures (x64, arm64 etc)

True, I think we should definitely switch over ppc64le and s390x to cbl-mariner. The only reason for sticking the evolving riscv64 to Ubuntu base layer was that we needed the latest llvm at the time (and there was no cbl-mariner option). If we can update cbl-mariner to use llvm17, that would be good. I think llvm16 is also fine; risc-v is relatively a new architectures with interesting extensions still being ratified (e.g. https://github.com/dotnet/runtime/pull/92974#issuecomment-1829280267); llvm-toolchain adds support for new extensions only in the bleeding-edge version (new features aren't backported in llvm-project either 😀).

akoeplinger commented 6 months ago

@am11 ubuntu-22.04-cross-riscv64-20240109142831-3fc5553 should have the change now.

If we can update cbl-mariner to use llvm17, that would be good. I think llvm16 is also fine; risc-v is relatively a new architectures with interesting extensions still being ratified (e.g. https://github.com/dotnet/runtime/pull/92974#issuecomment-1829280267); llvm-toolchain adds support for new extensions only in the bleeding-edge version (new features aren't backported in llvm-project either 😀).

Looks like we're already using LLVM 16 since https://github.com/dotnet/dotnet-buildtools-prereqs-docker/pull/864, I don't know if upgrading to 17 would be a big deal (probably requires a bit of coordination with dotnet/runtime at least).

am11 commented 6 months ago

@am11 ubuntu-22.04-cross-riscv64-20240109142831-3fc5553 should have the change now.

Unfortunately not:

$ docker run --rm \
  mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-22.04-cross-riscv64-20240109142831-3fc5553 \
  sh -c 'find /usr/bin -name clang*'

/usr/bin/clangd-15
/usr/bin/clang-cpp-15
/usr/bin/clang-15
/usr/bin/clang++-15
akoeplinger commented 6 months ago

Hmm weird, the build log for the crossdeps image shows it getting installed:

#6 9.091 Get:6 https://apt.llvm.org/jammy llvm-toolchain-jammy-17/main amd64 clang-17 amd64 1:17.0.6~++20231209124227+6009708b4367-1~exp1~20231209124336.77 [144 kB]
am11 commented 6 months ago

Ah we probably need to touch the file under riscv directory too so it doesn’t skip the build. There is also a way to evict the cache and force build all the layers.

am11 commented 6 months ago

@mthalman, should I submit a PR with dummy change in https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/main/src/ubuntu/22.04/cross/riscv64/Dockerfile to trigger the build or could you force build it somehow (e.g. by evicting the cache)?

mthalman commented 6 months ago

I've kicked off a build with the cache disabled.

mthalman commented 6 months ago

There are many errors in various build legs of the pipeline that are preventing a successful publish: https://github.com/dotnet/dotnet-buildtools-prereqs-docker/issues/950 https://github.com/dotnet/dotnet-buildtools-prereqs-docker/issues/951 https://github.com/dotnet/dotnet-buildtools-prereqs-docker/issues/952 https://github.com/dotnet/dotnet-buildtools-prereqs-docker/pull/953

I haven't looked whether any of these issues are from the Dockerfile you care about. Please check up on that. If your Dockerfile is unaffected by these errors, then I would recommend making a change to the Dockerfile (such as whitespace) to cause it to trigger a build without needing to disable the cache (which is causing all these other failures).

am11 commented 6 months ago

Thanks. None of these are related to Ubuntu 22.04, so it's safe to assume that it will go through with a dummy change in riscv Dockerfile. I'll open a PR shortly.

am11 commented 6 months ago

New image is built, now we have clang-17:

$ docker run --rm \
  mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-22.04-cross-riscv64-20240110235310-34800da \
  sh -c 'find /usr/bin -name 'clang*''

/usr/bin/clang-17
/usr/bin/clang-cpp-17
/usr/bin/clangd-17
/usr/bin/clang++-17