Skip to content

Conversation

OptimumCode
Copy link
Collaborator

A workflow that rebuilds all available versions on demand. It uses tags to find previous versions plus it uses the main branch to build that latest available version

@OptimumCode OptimumCode requested a review from Julian March 1, 2025 12:14
- name: Install qemu
run: |
sudo apt-get update
sudo apt-get install -y qemu-user-static
Copy link
Member

Choose a reason for hiding this comment

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

May as well ask: do you know if Kotlin supports cross-compiling natively rather than needing to use QEMU? (some languages like rust, c# and golang I know do and we already use that functionality for some of those)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know honestly. I can dig into that (I am not familiar with that topic - will learn something new 😃 ).

But Kotlin here is used as a JVM language. If Java supports cross-compiling so does Kotlin (I think so at least)

Copy link
Collaborator Author

@OptimumCode OptimumCode Mar 5, 2025

Choose a reason for hiding this comment

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

Actually, I think the answer would be yes because we compile kotlin into java classes which don't depend on the platform. So, we can remove this step I think

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or not...something went wrong once I removed the qemu
https://github.com/bowtie-json-schema/kotlin-kmp-json-schema-validator/actions/runs/13722121362/job/38379776771?pr=2

[1/2] STEP 6/8: RUN gradle --no-daemon --parallel dependencies > /dev/null
exec container process /bin/sh: Exec format error
Error: building at STEP "RUN gradle --no-daemon --parallel dependencies > /dev/null": while running runtime: exit status 1

I will try to debug it locally

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, so it looks like the problem is not in Java/Kotlin but in Gradle which uses sh to execute scripts. And apparently, this is not possible without emulation which is provided by QEMU

Copy link
Member

Choose a reason for hiding this comment

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

Huh, that's... strange. Shell scripts are text, so they shouldn't be arch-specific?? And I have to assume gradle isn't installing its own sh binary... is it????? If I can ask for one more time could you try to find someone who's maybe written a blog post on compiling cross-arch JVM images and maybe just confirm they ended up doing so via qemu? If so it's all fine to leave, but I'm just honestly curious.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it doesn't not install its own shell. I was looking into something like that but had no luck. I will try doing it one more time. Something doesn't add up here...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have not found an article about creating multiarch image with Java app. However, I did a few experiments and everything works just fine without any modifications with docker (I don't believe it installs QEMU automatically)
https://github.com/bowtie-json-schema/kotlin-kmp-json-schema-validator/actions/runs/13725912881/job/38392187019

But with podman and buildah we are getting the same error...
Will continue looking.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

CI is green without QEMU, however I am not yet sure how it happened (exactly the same configuration failed before...). Will dig deeper tomorrow

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have figured out why the same changes did not work before)
I used a build-all workflow to test them - it collects tags, checkouts them and builds images. And this is the part I was missing - Dockerfile on the tag does not contain the changes I did 😅 And the build failed

Re changes:

The only important thing is --platform=$BUILDPLATFORM in the Dockerfile. It checks out an image that is compatible with build platform and compiles Kotlin/Java classes. After that, the resulting files are moved to the container with target platform which can use those classes

- name: Install qemu
run: |
sudo apt-get update
sudo apt-get install -y qemu-user-static
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know honestly. I can dig into that (I am not familiar with that topic - will learn something new 😃 ).

But Kotlin here is used as a JVM language. If Java supports cross-compiling so does Kotlin (I think so at least)

- name: Install qemu
run: |
sudo apt-get update
sudo apt-get install -y qemu-user-static
Copy link
Collaborator Author

@OptimumCode OptimumCode Mar 5, 2025

Choose a reason for hiding this comment

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

Actually, I think the answer would be yes because we compile kotlin into java classes which don't depend on the platform. So, we can remove this step I think

matrix:
revision: ${{ fromJson(needs.versions.outputs.revisions) }}
permissions:
id-token: write
Copy link
Member

Choose a reason for hiding this comment

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

We really need all 3 of these write perms? (Another thing I'd like to do going forward is make sure we run zizmor on all workflows, which will complain about this, but if it's needed it's needed)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As I understand we need them to publish provenance. But I will double check.

Re zizmor: I will look at it (probably in a separate PR). Never heard about that tool before

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

id-token and attestations permissions are required by actions/attest-build-provenance@v2
https://github.com/actions/attest-build-provenance?tab=readme-ov-file#usage

packages required to publish the image

Not sure about contents though. According to this link contents: read are default permission for github token. So, probably we can remove it

Copy link
Collaborator Author

@OptimumCode OptimumCode Mar 7, 2025

Choose a reason for hiding this comment

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

But maybe it explicitly restricts permissions to read, because there is a column Default access (permissive) which has read/write value for that permission type

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reg zizmor: did I get you right that you want to run zizmor as a part of CI (whenever workflow files are changed)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It actually also integrates with pre-commit hook:

https://woodruffw.github.io/zizmor/usage/#use-with-pre-commit

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I forget if I've added it to bowtie itself but I've definitely added that workflow to a bunch of other repos I maintain

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't find zizmor in bowtie. Have you added it as a pre-commit hook or as a service workflow in other repos? Just to make it consistent across them

@Julian
Copy link
Member

Julian commented Mar 7, 2025

I left some last comments, and pre-commit is probably complaining about the missing end-of-line at the end of the file (or other stuff, I didn't click through), but after this I think feel free to merge whenever!

@OptimumCode
Copy link
Collaborator Author

pre-commit is probably complaining about the missing end-of-line at the end of the file

It actually complains about missing pre-commit config) it is in another branch. I will merge them together to make sure everything is formatted correctly

@OptimumCode OptimumCode changed the base branch from bowtie-main to prep-ci March 7, 2025 16:26
@OptimumCode
Copy link
Collaborator Author

@Julian I will merge this PR into prep-ci branch and will apply changes to make zizmor happy in a separate PR

@OptimumCode OptimumCode merged commit 401d87b into prep-ci Mar 8, 2025
9 checks passed
@OptimumCode OptimumCode deleted the rebuild-all branch March 8, 2025 07:54
OptimumCode added a commit that referenced this pull request Mar 15, 2025
…ication (#1)

* Add pre-commit config file

* Add dependabot config

* Add license

* Add build workflow

* Add automerge for dependabot PRs

* Add missing bracket

* Correct quotes

* Remove dotnet condition

* Remove --from when installing bowtie

* Use correct package to install bowtie

* Version is always set in the matrix

* Try extract version from latest and current version and prepare a the tag when needed

* Move part of tags to build image step

* Print collected versions

* Execute pre-commit hook

* Use raw value in jq to get rid of quotes

* Trigger image build only on a push to main branch

* Correct which version is used for a tag

* Add tag with version to the final image

* Use gh cli to create tag and release

* Add missing GH_TOKEN env variable

* Use personal access token to create a tag and release

* Use default github token for release creation

* Use bowtie action

* Add workflow to rebuild all available old version and the latest one (#2)

This commit also adds `--platform=$BUILDPLATFORM` into Dockerfile to enable multi-arch builds without QEMU

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Add zizmor pre-commit hook and update workflow to comply with zizmor rules (#3)

* Cleanup workflows

* Use user.login to identify whether the PR is from dependabot

* Remove manual tag creation. Use release API to create a tag

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

2 participants