-
Notifications
You must be signed in to change notification settings - Fork 31
Add GitHub action to build and push Docker image #55
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
|
||
jobs: | ||
build: | ||
runs-on: ubuntu-latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pin this to ubuntu-24.04
. We've run into quite a few unexpected breakages due to the ubuntu version bumping on us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in cd731d0
runs-on: ubuntu-latest | ||
|
||
permissions: | ||
contents: read |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contents: read
should be a top level permission. See https://llvm.org/docs/CIBestPractices.html.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 9992e48
permissions: | ||
contents: read | ||
packages: write | ||
id-token: write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed? We do not use in the workflows in the monorepo https://github.com/llvm/llvm-project/blob/main/.github/workflows/build-ci-container.yml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7d7e489
id-token: write | ||
|
||
steps: | ||
- uses: actions/checkout@v5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these actions need to be hash pinned. See https://llvm.org/docs/CIBestPractices.html.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7ba9006
To help get the ball rolling with deploying LNT, this adds a GitHub action to build and push images to the GitHub container registry, which is free for public repos. This will push a new image to ghcr.io/llvm-project/llvm-lnt:latest whenever anything is pushed to the main branch.
It also includes a fix for the Dockerfile after the minimum python version was bumped to 3.8 in #48. 3.10 is as high as we can go before we need to upgrade psycopg to 3.x, which in turn requires upgrading sqlalchemy to god knows what. We also need to upgrade psycopg 2.x anyway to get it to build with the newer python bindings.