pipelines/ruby: add new test pipelines #1483

Dentrax · 2024-09-09T12:14:30Z

This PR adds new pipelines for Ruby:

ruby/test
ruby/require

Example usages:

- uses: ruby/test
  with:
    command: ruby -e "require 'json'"

- uses: ruby/require
  with:
    require: json

- uses: ruby/require
  with:
    requires: |
      require 'json'

EyeCantCU

This is great. Left a few comments. Can we also document the usage of the pipeline in melange's docs?

EyeCantCU · 2024-09-09T18:11:32Z

pkg/build/pipelines/ruby/test.yaml

@@ -0,0 +1,14 @@
+name: Test a Ruby package


I think we should get rid of this. We're not doing anything inherently specific to Ruby

It seems we have name: field defined on each pipeline, do you say we should omit this field?

Sorry, the pipeline. It looks like it does the same thing a runs would do

so drop this?

EyeCantCU · 2024-09-09T18:12:33Z

pkg/build/pipelines/ruby/require.yaml

+
+needs:
+  packages:
+    - wolfi-base


Wolfi base is implicit. We don't need to define it

EyeCantCU · 2024-09-09T18:13:05Z

pkg/build/pipelines/ruby/require.yaml

+name: Test a Ruby package require, with optional load clause
+
+needs:
+  packages:


We should add Ruby here, passed as an input. See go/build for what I'm talking about

I just checked the other Ruby pipelines and it seems we didn't include the Ruby, was wondering if there was any reason. 🤔

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

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.

So , yeah... several of the pipelines that I've added have this flaw in that they don't declare needs that it'd be nice if they did. py/pip-build-install for example needs bot a pip and a python, but doesn't declare either.

The reason is that it takes python as an input (the python version to use).

I don't think you can use a input to a pipeline in its needs. It would be cleaner here if you could. This specific case would be nice in that you could then:

needs: packages: - ruby-${{inputs.version}}

And then have a inputs that declared 3.2 as the default or something.

If that happens to work, (including with a range section like we're doing in python) then I guess I'd suggest doing that, but other wise I'd not bother.

Another option would be to have a pipeline per ruby-version (ruby/require-3.2, ruby/require-3.3). I suspect that would work, and you could reduce copy/paste code between ruby/require-3.2 and ruby/require-3.3 with nested pipelines. But if you were doing per-ruby versions i'd want that in wolfi-dev/os rather than melange proper.

So...
that was all long winded way of saying "I think this is fine".

EyeCantCU · 2024-09-09T18:13:42Z

pkg/build/pipelines/ruby/require.yaml

+
+pipeline:
+  - runs: |
+      set +x


I don't think this applies to the rest of the pipeline

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.

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

it does "apply", in that it turns off '-x' option (making it not spew bash debug to stderr).

smoser

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.

Dentrax · 2024-09-11T13:26:29Z

Thank you so much @smoser! I just updated the PR and hit a new issue:

ruby-3.2-3.2.4-r1.apk disqualified because ruby-3.3-3.3.5-r1.apk already provides cmd:erb

That's probably due to I set ${{inputs.ruby}}=ruby so it installs the ruby-3.3 (latest) whereas I have ruby-3.2 in the test pipeline. But I'd like to set ruby-3.2 as a default package since most of Wolfi packages using this version. That's why I had to set default: ruby-3.2.

smoser · 2024-10-29T12:27:37Z

pkg/build/pipelines/ruby/require.yaml

+        fi
+      }
+
+      if [ "$RUBY" = "ruby" ]; then


I think the default 'DEFAULT' or some other non-usable string is prefereable here.
The caller could very legitmately have ruby in the PATH and want that to be tested. At least I can't think of a good reason to "fix" it for them.

Did you have one?

Signed-off-by: Dentrax <[email protected]>

Dentrax · 2024-10-29T13:53:34Z

Thank you @smoser! I made some changes according to your reviews here, could you PTAL when possible? It seems the CI is now green but not sure whether it's false-positive.

smoser · 2024-10-30T00:46:41Z

pkg/build/pipelines/ruby/test.yaml

@@ -0,0 +1,14 @@
+name: Test a Ruby package


so drop this?

smoser · 2024-10-30T00:50:10Z

pkg/build/pipelines/ruby/require.yaml

+    default: DEFAULT
+  version:
+    description: Which Ruby version to use
+    default: 3.2


maybe if you're letting it declare version, then don't let it declare 'ruby' ?
because what happens if i do:

ruby: ruby3.3 version: ruby3.2

I tihnk one or the other.
Either take ruby and just don't bother with needs.
or take version , set needs, and use either explicitly use /usr/bin/ruby${{inputs.version}} or just assume in PATH as ruby${{inputs.version}}.

thoughts?

I'm kind of thinking out loud here. I think its wierd to take both, but it was very useful to in the python ones to use DEFAULT and pick the right /usr/bin/python3.XX when there was only one, and error in other cases.

Dentrax changed the title ~~pipelines/ruby: add new pipelines to ruby packages~~ pipelines/ruby: add new test pipelines for Ruby Sep 9, 2024

Dentrax changed the title ~~pipelines/ruby: add new test pipelines for Ruby~~ pipelines/ruby: add new test pipelines Sep 9, 2024

Dentrax force-pushed the pipelines-ruby-test branch 6 times, most recently from b4554e1 to 7b51ab9 Compare September 9, 2024 17:43

Dentrax marked this pull request as ready for review September 9, 2024 17:58

EyeCantCU requested changes Sep 9, 2024

View reviewed changes

Dentrax force-pushed the pipelines-ruby-test branch 4 times, most recently from f6c177d to 71843e5 Compare September 10, 2024 09:22

smoser requested changes Sep 10, 2024

View reviewed changes

Dentrax force-pushed the pipelines-ruby-test branch from 71843e5 to ae4bafc Compare September 11, 2024 12:55

smoser reviewed Oct 29, 2024

View reviewed changes

Dentrax added 2 commits October 29, 2024 15:39

pipelines/ruby: add new pipelines to ruby packages

7f52c3a

Signed-off-by: Dentrax <[email protected]>

apply reviews

67bf807

Signed-off-by: Dentrax <[email protected]>

Dentrax force-pushed the pipelines-ruby-test branch from ae4bafc to 67bf807 Compare October 29, 2024 13:35

smoser requested changes Oct 30, 2024

View reviewed changes

justinvreeland mentioned this pull request Nov 6, 2024

Draft: First pass at automated ruby test wolfi-dev/os#33418

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pipelines/ruby: add new test pipelines #1483

pipelines/ruby: add new test pipelines #1483

Dentrax commented Sep 9, 2024

EyeCantCU left a comment

EyeCantCU Sep 9, 2024

Dentrax Sep 9, 2024

EyeCantCU Sep 9, 2024

smoser Oct 30, 2024

EyeCantCU Sep 9, 2024

EyeCantCU Sep 9, 2024

Dentrax Sep 9, 2024

EyeCantCU Sep 9, 2024 •

edited

Loading

Dentrax Sep 10, 2024

smoser Oct 29, 2024

EyeCantCU Sep 9, 2024

Dentrax Sep 9, 2024

EyeCantCU Sep 9, 2024

smoser Sep 10, 2024

smoser left a comment

Dentrax commented Sep 11, 2024

smoser Oct 29, 2024

Dentrax commented Oct 29, 2024

smoser Oct 30, 2024

smoser Oct 30, 2024

smoser Oct 30, 2024

pipelines/ruby: add new test pipelines #1483

Are you sure you want to change the base?

pipelines/ruby: add new test pipelines #1483

Conversation

Dentrax commented Sep 9, 2024

EyeCantCU left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EyeCantCU Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smoser left a comment

Choose a reason for hiding this comment

Dentrax commented Sep 11, 2024

Choose a reason for hiding this comment

Dentrax commented Oct 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EyeCantCU Sep 9, 2024 •

edited

Loading