-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add non-ghr version of cpubuilder dockerfile. #4
Conversation
packages: write | ||
steps: | ||
- name: Checkout repository | ||
uses: actions/checkout@v3 |
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.
We probably want to pin the action to a specific version
uses: actions/checkout@v3 | |
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 |
(will also increase the score if we want to run OpenSSF scorecard, see https://github.com/ossf/scorecard/blob/main/docs/checks.md#pinned-dependencies)
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.
Oh, I started switching back to using tags in iree-org/iree#17703
We have been using commit hashes instead of tags, since they are more stable and that was a policy recommendation for Google-managed GitHub repos: https://opensource.google/documentation/reference/github/services#actions
When using a third-party action (one not hosted in a Google-managed org), a fixed version of the action MUST be used by specifying a specific commit, rather than a branch like "master", or a tagged release, which can be overwritten by any maintainer of the action. Docker images should always be run at a fixed version rather than "latest".
Using release tags is easier and more common, so switching back to that style.
Anyways, let's leave that for follow-up work if we want it and apply such a change to all files in the repo.
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.
Oops. Needed to use an uppercase D
in the file extension 🤦
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.
Fixed: de9012a
…es. (#18355) Progress on #15332. This uses a new `cpubuilder_ubuntu_jammy_x86_64` dockerfile from iree-org/base-docker-images#4. If this looks good, I'll continue to switch other workflows from `gcr.io/iree-oss/base` to this image, installing new deps and pushing new package versions as needed. Once all the workflows are changed, we can delete https://github.com/iree-org/iree/tree/main/build_tools/docker. skip-ci: does not affect other builds
Progress on #15332. This uses a new `cpubuilder_ubuntu_jammy_x86_64` dockerfile from iree-org/base-docker-images#4. Once all the workflows are changed, we can delete https://github.com/iree-org/iree/tree/main/build_tools/docker. Logs: * byollvm: https://github.com/iree-org/iree/actions/runs/10567230124/job/29275748653?pr=18356 * gcc: https://github.com/iree-org/iree/actions/runs/10567230125/job/29275748639?pr=18356 skip-ci: does not affect other builds
I believe we can replace https://github.com/iree-org/iree/blob/main/build_tools/docker/dockerfiles/base.Dockerfile with this. At least, I tested the
linux_x64_clang_debug
workflow in IREE locally and was able to get it working using this Dockerfile and only minor edits to the workflow:Command history for future-me (could go in docs):
For now the Dockerfiles for ghr and non-ghr versions both have large sections of code copy/pasted. Hopefully these will be edited infrequently and we don't need to overcomplicate that with multistage builds. The publish workflow also has duplicated code. That could use a matrix or reusable workflow.