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

Float 3.0 is misparsed as integer 3 in workflow YAML #849

Closed
eregon opened this issue Dec 5, 2020 · 10 comments
Closed

Float 3.0 is misparsed as integer 3 in workflow YAML #849

eregon opened this issue Dec 5, 2020 · 10 comments
Labels
Service Feature Feature scope to the pipelines service and launch app

Comments

@eregon
Copy link

eregon commented Dec 5, 2020

Describe the bug

3.0 in a workflow is misparsed as integer 3.
Or at the very least, when it's parsed and then later converted to a string (with ${{ matrix.ruby }}), it's 3 and not the expected 3.0.

To Reproduce

jobs:
  test:
    strategy:
      matrix:
        os: [ ubuntu-latest ]
        ruby: [ 2.7, 3.0 ]
    name: ${{ matrix.os }} ${{ matrix.ruby }}
    runs-on: ${{ matrix.os }}
    steps:
    - uses: ruby/setup-ruby@v1
      with:
        ruby-version: ${{ matrix.ruby }}

Expected behavior
The input log and the job name should indicate 3.0, but we see 3 instead:
https://github.com/eregon/setup-ruby/runs/1503368422?check_suite_focus=true#step:3:3

Run ./
  with:
    ruby-version: 3
    bundler-cache: true
    bundler: default
    working-directory: .

This is important because ruby-version: 3.0 should pick version 3.0 but not version 3.1.
Due to this bug it would pick version 3.1.

I think in the YAML specification it's clear that floats and integers are different, so they should be kept that way too in workflow YAMLS, including when passing through ${{ matrix.ruby }}.

Using quotes [ 2.7, '3.0' ] is a workaround, but it should be unnecessary and it's much more verbose with many versions.

Runner Version and Platform

Version of your runner? 2.274.2
OS of the machine running the runner? Applies to all

@eregon eregon added the bug Something isn't working label Dec 5, 2020
@maxim-lobanov
Copy link

@eregon I have faced with the same issue in my own action https://github.com/maxim-lobanov/setup-xcode and decided that the best solution is updating my action's readme to recommend customers to use quotes in inputs.

I agree that the behavior that you have described looks a bit strange and it could be a bug. But I have found another problem if inputs are not quoted:

with:
    python-version: 3.10

This input will be parsed as 3.1 and it is definitely not that you expect to see. But based on YAML specification it is expected behavior because 3.10 is float value and trailing 0 can be removed.

A few words from https://symfony.com/doc/current/components/yaml/yaml_format.html

Strings in YAML can be wrapped both in single and double quotes. In some cases, they can also be unquoted
...
Finally, there are other cases when the strings must be quoted, no matter if you’re using single or double quotes:
...

  • When the string looks like a number, such as integers (e.g. 2, 14, etc.), floats (e.g. 2.6, 14.9) and exponential numbers (e.g. 12e7, etc.) (otherwise, it would be treated as a numeric value);

So looks like that quoting is the best approach to save input as it is since strings can't be transformed somehow.
Also, All first-party actions recommend to wrap version input to quotes: https://github.com/actions/setup-node#usage

(Please do not take my answer as official position of actions/runner owners 😃 . I don't have context how YAML parsing works under hood in GitHub Actions and just say from position of third-party action's owner who faced with the same issue)

@eregon
Copy link
Author

eregon commented Dec 12, 2020

I agree for 3.10 it's a problem and there is no other way to fix that than to use quotes ('3.10').
I am taking advantage here that CRuby versions never had and most likely never will have a minor > 9 (i.e., it was 1.9 then 2.0).

When I started the action I considered both options, and I chose the unquoted variant because it worked as far as I could see just as good, and it's a lot more concise.
Compare:

jobs:
  test:
    strategy:
      fail-fast: false
      matrix:
        os: [ubuntu, macos]
        ruby: [2.5, 2.6, 2.7, head, debug, jruby, jruby-head, truffleruby, truffleruby-head]

and

jobs:
  test:
    strategy:
      fail-fast: false
      matrix:
        os: [ubuntu, macos]
        ruby: ['2.5', '2.6', '2.7', 'head', 'debug', 'jruby', 'jruby-head', 'truffleruby', 'truffleruby-head']

I find the second one much harder to read.
A practical consequence is the quoted variant actually doesn't fit on one line in the README.

An example in another project: https://github.com/jeremyevans/sequel/pull/1742/files

Also, as we can still see today, the unquoted versions were also used in TravisCI, which a lot of Ruby users come from:
https://docs.travis-ci.com/user/languages/ruby/

language: ruby
rvm:
  - 2.5
  - 2.6
  - jruby
  - truffleruby

richardmcmillen added a commit to richardmcmillen/reline that referenced this issue Dec 27, 2020
Wrap 3.0 version in single quotes. This is to avoid issues with
interpreting the version identifier as `3` instead of `3.0`.

Some more details here: actions/runner#849
richardmcmillen added a commit to richardmcmillen/reline that referenced this issue Dec 28, 2020
Wrap 3.0 version in single quotes. This is to avoid issues with
interpreting the version identifier as `3` instead of `3.0`.

Some more details here: actions/runner#849
dentarg added a commit to dentarg/middleman that referenced this issue Jan 1, 2021
To avoid unexpectedly stop testing Ruby 3.0 when Ruby 3.1 is released.

See actions/runner#849

At https://github.com/middleman/middleman/runs/1631689419?check_suite_focus=true#step:3:3 we can see that the setup-ruby action ran with just 3 as the input:

```
Run ruby/setup-ruby@v1
  with:
    ruby-version: 3
```

If we quote the version it works as expected, example at https://github.com/ruby/setup-ruby/runs/1617122299?check_suite_focus=true#step:3:3

```
Run ./
  with:
    ruby-version: 3.0
```
tdreyno pushed a commit to middleman/middleman that referenced this issue Jan 3, 2021
To avoid unexpectedly stop testing Ruby 3.0 when Ruby 3.1 is released.

See actions/runner#849

At https://github.com/middleman/middleman/runs/1631689419?check_suite_focus=true#step:3:3 we can see that the setup-ruby action ran with just 3 as the input:

```
Run ruby/setup-ruby@v1
  with:
    ruby-version: 3
```

If we quote the version it works as expected, example at https://github.com/ruby/setup-ruby/runs/1617122299?check_suite_focus=true#step:3:3

```
Run ./
  with:
    ruby-version: 3.0
```
dentarg added a commit to dentarg/raven-ruby that referenced this issue Jan 27, 2021
To avoid unexpectedly stop testing Ruby 3.0 when Ruby 3.1 is released.

See actions/runner#849

You can see that it says just `3`, not `3.0` at https://github.com/getsentry/sentry-ruby/runs/1775107213?check_suite_focus=true#step:4:4
st0012 pushed a commit to getsentry/sentry-ruby that referenced this issue Jan 27, 2021
To avoid unexpectedly stop testing Ruby 3.0 when Ruby 3.1 is released.

See actions/runner#849

You can see that it says just `3`, not `3.0` at https://github.com/getsentry/sentry-ruby/runs/1775107213?check_suite_focus=true#step:4:4
dentarg added a commit to dentarg/graphql-batch that referenced this issue Feb 24, 2021
To avoid unexpectedly stop testing Ruby 3.0 when Ruby 3.1 is released.

See actions/runner#849

At https://github.com/Shopify/graphql-batch/runs/1691443768?check_suite_focus=true#step:3:4 we can see that the setup-ruby action ran with just `3` as the input and not `3.0`.
@eregon
Copy link
Author

eregon commented Mar 5, 2021

The YAML parser seems at https://github.com/actions/runner/blob/main/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/YamlObjectReader.cs
It seems to both parse doubles and integers as NumberToken, which contains a double value, which is probably why later on it cannot make the difference between an integer and a double with no decimal part.

Probably NumberToken should keep a boolean flag for whether it's an integer or double, or to keep a String value if it's a double with no decimal part so it knows that converting it to string should be 3.0 and not 3.
In general I think all double values should always show they are floating point by always having a decimal point/dot, so the boolean seems best.

okuramasafumi added a commit to okuramasafumi/setup-ruby that referenced this issue Mar 5, 2021
As described in the comment, due to a bug discussed on actions/runner#849,
we currently have to use quotes for  '3.0' as a workaround.
This change makes it easy for people not to remove quotes as they misunderstand it's a typo.
eregon pushed a commit to ruby/setup-ruby that referenced this issue Mar 5, 2021
As described in the comment, due to a bug discussed on actions/runner#849,
we currently have to use quotes for  '3.0' as a workaround.
This change makes it easy for people not to remove quotes as they misunderstand it's a typo.
dentarg added a commit to dentarg/rack that referenced this issue Mar 14, 2021
To avoid unexpectedly stop testing Ruby 3.0 when Ruby 3.1 is released.

See actions/runner#849

At https://github.com/rack/rack/runs/2041788658?check_suite_focus=true#step:3:3 we can see that the setup-ruby action ran with just `3` as the input and not `3.0`.
ioquatix pushed a commit to rack/rack that referenced this issue Mar 14, 2021
To avoid unexpectedly stop testing Ruby 3.0 when Ruby 3.1 is released.

See actions/runner#849

At https://github.com/rack/rack/runs/2041788658?check_suite_focus=true#step:3:3 we can see that the setup-ruby action ran with just `3` as the input and not `3.0`.
dentarg added a commit to dentarg/climate_control that referenced this issue Mar 16, 2021
To avoid unexpectedly stop testing Ruby 3.0 when Ruby 3.1 is released.

See actions/runner#849

At https://github.com/thoughtbot/climate_control/runs/2049131377?check_suite_focus=true#step:3:3 we can see that the setup-ruby action ran with just `3` as the input and not `3.0`.
joshuaclayton pushed a commit to thoughtbot/climate_control that referenced this issue Mar 17, 2021
To avoid unexpectedly stop testing Ruby 3.0 when Ruby 3.1 is released.

See actions/runner#849

At https://github.com/thoughtbot/climate_control/runs/2049131377?check_suite_focus=true#step:3:3 we can see that the setup-ruby action ran with just `3` as the input and not `3.0`.
@eregon
Copy link
Author

eregon commented Jan 24, 2023

Another approach to solve this: actions/toolkit#1320
That would even work for 3.10, there is no CRuby version N.10 but nevertheless it would be a general solution to this problem, as @maxim-lobanov mentioned above.

As one can see, there are hundreds of links to this issue, all workarounds for it, it'd be great to fix it, one way or another.

@ruvceskistefan
Copy link
Contributor

This issue does not seem to be a feature for the runner application, it concerns the GitHub actions platform more generally. Could you please post your feedback on the GitHub Community Support Forum which is actively monitored. Using the forum ensures that we route your feature request to the correct team. 😃

tinahollygb added a commit to growthbook/growthbook-ruby that referenced this issue Apr 18, 2023
y-yagi added a commit to y-yagi/credit_card_validations that referenced this issue Jun 12, 2023
Fivell pushed a commit to didww/credit_card_validations that referenced this issue Jun 16, 2023
* CI against latest Rubies

* Unlock `rake` version

To support Ruby 3.2.

* Show Ruby version in a Job name

* `3.0` need quotes

Ref: actions/runner#849
tjleing pushed a commit to aws-amplify/amplify-android that referenced this issue Dec 22, 2023
WarlyGroup pushed a commit to WarlyGroup/net-imap that referenced this issue Jul 4, 2024
See actions/runner#849 for details about the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Service Feature Feature scope to the pipelines service and launch app
Projects
None yet
Development

No branches or pull requests

6 participants