-
Notifications
You must be signed in to change notification settings - Fork 95
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
pipelines/ruby: add new test pipelines #1483
base: main
Are you sure you want to change the base?
Conversation
b4554e1
to
7b51ab9
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.
This is great. Left a few comments. Can we also document the usage of the pipeline in melange's docs?
@@ -0,0 +1,14 @@ | |||
name: Test a Ruby package |
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.
I think we should get rid of this. We're not doing anything inherently specific to Ruby
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.
It seems we have name:
field defined on each pipeline, do you say we should omit this field?
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.
Sorry, the pipeline. It looks like it does the same thing a runs
would do
|
||
needs: | ||
packages: | ||
- wolfi-base |
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.
Wolfi base is implicit. We don't need to define it
name: Test a Ruby package require, with optional load clause | ||
|
||
needs: | ||
packages: |
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 should add Ruby here, passed as an input. See go/build for what I'm talking about
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.
I just checked the other Ruby pipelines and it seems we didn't include the Ruby
, was wondering if there was any reason. 🤔
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.
I don't think it'd ever been considered that pipeline should fetch the toolchain. go/build
didn't offer this functionality until a few months ago either. We should provide it because it's helpful, and we can implicitly pull in ruby without defining it in the environment
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.
That makes sense, I've added Ruby here and couldn't pass the CI error: https://github.com/chainguard-dev/melange/actions/runs/10788637898/job/29919880644?pr=1483#step:9:1071 - for some reason, it doesn't find ruby-3.2
even its there.
|
||
pipeline: | ||
- runs: | | ||
set +x |
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.
I don't think this applies to the rest of the pipeline
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.
This is a copy-paste thing from python/import
pipeline actually. I'm checking the CI logs here but couldn't see the instructions either.
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.
Yeah, happy to be wrong here, but I don't believe it's transient to the rest of the pipeline. Will let someone that knows for sure chime in
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.
it does "apply", in that it turns off '-x' option (making it not spew bash debug to stderr).
f6c177d
to
71843e5
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.
with regard to the error you're seeing [here]:
2024/09/10 09:27:54 INFO running step "Test a Ruby package require, with optional load clause"
2024/09/10 09:27:54 WARN /bin/sh: ruby-3.2: not found
2024/09/10 09:27:54 INFO ruby-3.2 -e "require 'json'": FAIL
2024/09/10 09:27:54 INFO ERROR: failed to test package. the test environment has been preserved:
2024/09/10 09:27:54 INFO workspace dir: /tmp/melange-workspace-1987892197
2024/09/10 09:27:54 INFO guest dir: /tmp/melange-guest-1360058723
A couple things:
-
ruby-3.2 does not provide an executable named 'ruby-3.2'. The package is
named ruby-3.2 but it only provides /usr/bin/ruby. So thats why you get
'ruby-3.2' command not found. -
If the 'inputs.ruby' is providing the package rather than an executable
name (in python it provides an executable name), to install, and we are assuming that a ruby (ruby-3.X) package will always install /usr/bin/ruby, then you can ditch the searching for a binary (lines 43-57 entirely), and make 'RUBY='ruby' on line 29. -
in e2e-tests/ruby3.2-json-test.yaml you should actually drop the 'packages:' section, because the 'needs' should fulfill that.
Signed-off-by: Dentrax <furkan.turkal@chainguard.dev>
71843e5
to
ae4bafc
Compare
Thank you so much @smoser! I just updated the PR and hit a new issue:
That's probably due to I set |
This PR adds new pipelines for Ruby:
ruby/test
ruby/require
Example usages: