This is an archive of the discontinued LLVM Phabricator instance.

[libc++][CI] Uses LLVM 17 in Docker.
ClosedPublic

Authored by Mordante on Jan 31 2023, 11:52 AM.

Details

Reviewers
ldionne
philnik
Group Reviewers
Restricted Project
Commits
rGe4022b6b87bd: [libc++][CI] Uses LLVM 17 in Docker.
Summary

Updates the LLVM versions used in the Dockerfile. It also removes
obsolete symlinks. This doesn't update the Buildkite jobs, they need to
use the new Docker image before they can be updated.

Diff Detail

Event Timeline

Mordante created this revision.Jan 31 2023, 11:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2023, 11:52 AM
Herald added a subscriber: arichardson. · View Herald Transcript
Mordante requested review of this revision.Jan 31 2023, 11:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2023, 11:52 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik accepted this revision.Feb 2 2023, 1:09 PM
philnik added a subscriber: philnik.

LGTM assuming the Docker container builds.

libcxx/utils/ci/Dockerfile
79–80

Maybe also remove this?

This revision is now accepted and ready to land.Feb 2 2023, 1:09 PM
ldionne accepted this revision.Feb 3 2023, 2:19 PM

That's great, thanks!

Is LLVM_HEAD_VERSION=17 going to work yet?

jloser added a subscriber: jloser.Feb 3 2023, 6:28 PM

Is LLVM_HEAD_VERSION=17 going to work yet?

I don't think so based on looking at the code:

declare -A LLVM_VERSION_PATTERNS
LLVM_VERSION_PATTERNS[9]="-9"
LLVM_VERSION_PATTERNS[10]="-10"
LLVM_VERSION_PATTERNS[11]="-11"
LLVM_VERSION_PATTERNS[12]="-12"
LLVM_VERSION_PATTERNS[13]="-13"
LLVM_VERSION_PATTERNS[14]="-14"
LLVM_VERSION_PATTERNS[15]="-15"
LLVM_VERSION_PATTERNS[16]="-16"
LLVM_VERSION_PATTERNS[17]=""

if [ ! ${LLVM_VERSION_PATTERNS[$LLVM_VERSION]+_} ]; then
    echo "This script does not support LLVM version $LLVM_VERSION"
    exit 3
fi

since LLVM_VERSION_PATTERNS[17] is an empty string.

Thanks for the reviews!

Is LLVM_HEAD_VERSION=17 going to work yet?

I don't think so based on looking at the code:

declare -A LLVM_VERSION_PATTERNS
LLVM_VERSION_PATTERNS[9]="-9"
LLVM_VERSION_PATTERNS[10]="-10"
LLVM_VERSION_PATTERNS[11]="-11"
LLVM_VERSION_PATTERNS[12]="-12"
LLVM_VERSION_PATTERNS[13]="-13"
LLVM_VERSION_PATTERNS[14]="-14"
LLVM_VERSION_PATTERNS[15]="-15"
LLVM_VERSION_PATTERNS[16]="-16"
LLVM_VERSION_PATTERNS[17]=""

if [ ! ${LLVM_VERSION_PATTERNS[$LLVM_VERSION]+_} ]; then
    echo "This script does not support LLVM version $LLVM_VERSION"
    exit 3
fi

since LLVM_VERSION_PATTERNS[17] is an empty string.

AFAIK That part of the code is to make sure lines like

deb http://apt.llvm.org/bullseye/ llvm-toolchain-bullseye-15 main
deb http://apt.llvm.org/bullseye/ llvm-toolchain-bullseye main

work properly in sources.list files in Debian/Ubuntu. Note the the ToT version has no version suffix.

I have tested the Docker file locally with several Clang versions, including 17, before I created this review.

There is one fix queued to be backported to the LLVM-16 after that has landed I'll land this and upload a new Docker image.
(Without that fix the LLVM-16 branch modular build will fail.)

libcxx/utils/ci/Dockerfile
79–80

I prefer to do that separately; I'm not convinced this will work for the LLVM-16 release branch.

jloser added a comment.Feb 5 2023, 6:28 AM

Thanks for the reviews!

Is LLVM_HEAD_VERSION=17 going to work yet?

I don't think so based on looking at the code:

declare -A LLVM_VERSION_PATTERNS
LLVM_VERSION_PATTERNS[9]="-9"
LLVM_VERSION_PATTERNS[10]="-10"
LLVM_VERSION_PATTERNS[11]="-11"
LLVM_VERSION_PATTERNS[12]="-12"
LLVM_VERSION_PATTERNS[13]="-13"
LLVM_VERSION_PATTERNS[14]="-14"
LLVM_VERSION_PATTERNS[15]="-15"
LLVM_VERSION_PATTERNS[16]="-16"
LLVM_VERSION_PATTERNS[17]=""

if [ ! ${LLVM_VERSION_PATTERNS[$LLVM_VERSION]+_} ]; then
    echo "This script does not support LLVM version $LLVM_VERSION"
    exit 3
fi

since LLVM_VERSION_PATTERNS[17] is an empty string.

AFAIK That part of the code is to make sure lines like

deb http://apt.llvm.org/bullseye/ llvm-toolchain-bullseye-15 main
deb http://apt.llvm.org/bullseye/ llvm-toolchain-bullseye main

work properly in sources.list files in Debian/Ubuntu. Note the the ToT version has no version suffix.

I have tested the Docker file locally with several Clang versions, including 17, before I created this review.

There is one fix queued to be backported to the LLVM-16 after that has landed I'll land this and upload a new Docker image.
(Without that fix the LLVM-16 branch modular build will fail.)

Ah, I see — you're right after closer inspection of the llvm.sh script. This looks fine then — sorry about that!

Ah, I see — you're right after closer inspection of the llvm.sh script. This looks fine then — sorry about that!

No problem.

This revision was automatically updated to reflect the committed changes.