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

Use Khronos asciidoctor-spec Docker image in CI #1196

Merged
merged 8 commits into from
Jul 9, 2024
Merged

Conversation

oddhack
Copy link
Contributor

@oddhack oddhack commented Jul 1, 2024

Per discussion with @bashbaug

N.b. at present the CI script has less parallelism than it could, at least as I understand Actions. Some of the 'steps' could be split off into 'jobs'. Might try that next once the basic build is working. Net performance is still somewhat faster than current CI since it's generally faster to load the container than to add needed packages at each invocation, and the spec build is pretty fast, so there's not much to be gained - might reduce runtime from 3 to 1.5 minutes, at best.

There was odd error behavior from shifting to the container which I have never seen in Vulkan CI, having to do with mixed ownership of files in the checked-out repository. I inserted a brute-force workaround right after the checkout action.

Closes #1192

Per discussion with $bashbaug

N.b. at present the CI script has less parallelism than it could, at
least as I understand Actions. Some of the 'steps' could be split off
into 'jobs'. Might try that next once the basic build is working. Net
performance is still somewhat faster than current CI since it's
generally faster to load the container than to add needed packages at
each invocation, and the spec build is pretty fast, so there's not much
to be gained.

There was odd error behavior from shifting to the container which I have
never seen in Vulkan CI, having to do with mixed ownership of files in
the checked-out repository. I inserted a brute-force workaround right
after the checkout action.
@oddhack
Copy link
Contributor Author

oddhack commented Jul 1, 2024

@bashbaug BTW, building the spec with this toolchain reports some errors in tables with the wrong numbers of cells on some rows, and maybe some others. Not sure if those were present before, but the updated asciidoctor apparently does do some more comprehensive error reporting as @gmlueck noticed the same thing in SYCL-Docs.

Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one question about the docker SHA.

.github/workflows/presubmit.yml Outdated Show resolved Hide resolved
.github/workflows/presubmit.yml Show resolved Hide resolved
Since the Docker build image runs a python virtual environment now.

Also added 'scripts/runDocker' script which will invoke docker locally
with the same image used in Github CI, for testing.

Note this script will pull over a GB of Docker stuff onto the machine
it's invoked on, if the image is not already cached.
... which appears sporadic, not easily replicable.
…file

For future reference, some of the git operations in CI and the Makefile
appear to *sporadically* fail in CI because of different checked-out
repo configurations.

I modified the 'git symbolic-ref' and 'git log' operations invoked from
the Makefile to detect errors and substitute a placeholder message,
based on similar changes to the Vulkan Makefile a while back. This
(appears) to eliminate the sporadic 'fatal' messages. We may need to do
that to the 'git describe' as well.

None of this reads on the generated artifacts, except that they may or
may not contain accurate tag / commit comments.
@neiltrevett neiltrevett merged commit c75e07f into main Jul 9, 2024
3 checks passed
@bashbaug bashbaug deleted the 1192-container branch July 9, 2024 15:54
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.

Presubmit workflow fails on Install required packages stage
3 participants