-
Notifications
You must be signed in to change notification settings - Fork 116
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
fix: gradle builds #3250
fix: gradle builds #3250
Conversation
This reverts commit b8fdd83. Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
6622847
to
ca401d7
Compare
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.
Thanks. Can you send a PR. to slsa-framework/example-packages with a new workflow to test this fix?
Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>
it seems like reusing the build directory (a gradle concept) is a bit odd here? What we select as the slsa artifacts directory is arbitrary (and this isn't this PRs issue really). I guess the question is what should be adding to the slsa artifacts directory? Everything that the build used to build the final artifact? or just the final artifact? |
Can you explain a bit more what you mean? You want me to submit and merge a PR that has a new workflow name that references my fork of slsa-github-generator? |
@loosebazooka I also thought it odd to only generate provenance for a select few files of all the files that are served as artifacts. But for this PR I thought to not break existing working behavior. |
The In this case something like The test should reference the
In general, the tests in |
@ianlewis So you're saying you want a new "pre-submit" workflow that runs as a PR Check, where the new workflow is meant to check the gradle and maven functionality? |
It's not a "pre-submit" since it won't block PRs. The e2e test workflows in example-package run daily on a schedule and create issues in this repo when the workflow fails. I believe that @laurentsimon means that we need to create a new workflow that checks the functionality you are fixing in this PR. We won't need it until after the PR is merged, so I think you can just note it, and not take it as a PR blocker. |
@ianlewis Ok I understand now. But the only way to test this fix properly in exaple-package is to make the gradle "project" directory also be the root of the repository. noted in the issue #2727 (comment) |
I'll wait until you have verified that changes fix the problems by testing your fork. Please comment when you've verified it works |
@laurentsimon I did actually test with my own of both slsa-github-generator and example-package my fork of slsa-github-generator running the original e2e of example-package with my fork of slsa-github-generator running the builder with my own sample gradle project with my own fork slsa-github-generator |
I'm not sure about the exact problem, but if directory structure is an issue, you can create a copy of the gradle project in example-package specifically for your test. |
yes, that's the next step after merging this P. Seems like you've done due diligence, so let's merge this PR. Please send a pr to example-package to close the loop and close #3255 |
This reverts commit b097318.
This reverts commit b097318.
This reverts commit b097318.
This reverts commit b097318. Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
Fixes the Gradle builds #2727
I think the first attempt to fix (now reverted) was mostly correct, but in this PR I correct the directory comparison conditional.
Also adds some documentation for handling multi-project builds, which seem to now be the default when initializing a new Gradle app.
Testing
Tested against my own sample project
Modified the
slsa-framwork/example-package
e2e tests against my own fork. The actual builds and provenance generation succeed, except for the verify stage, which should fail because my forkhttps://github.com/ramonpetgrave64/slsa-github-generator/.github/workflows/builder_gradle_slsa3.yml@refs/heads/main
is not a "trusted builder".