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

Fixes #36913 - Set up GHA with matrix to run test on Ruby 2.7 #9900

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

ofedoren
Copy link
Member

This is the first part of #9871.

@ekohl
Copy link
Member

ekohl commented Nov 10, 2023

I wonder if it's updating Psych, or if libyaml on the Ubuntu-latest is newer than on our EL7 Jenkins nodes. That's the only thing I can think of

@ekohl
Copy link
Member

ekohl commented Nov 15, 2023

I decided to dive into the failure. As far as I can see it calls YAML.safe_load(value, permitted_classes: [Symbol]) to load the value. So some testing:

EL 8:

# rpm -qv ruby ; rpm -qv libyaml
ruby-2.5.9-111.module_el8+475+35a6c697.x86_64
libyaml-0.1.7-5.el8.x86_64
# ruby -ryaml -e 'puts YAML.safe_load("{a:test}").inspect'
Traceback (most recent call last):
	4: from -e:1:in `<main>'
	3: from /usr/share/ruby/psych.rb:314:in `safe_load'
	2: from /usr/share/ruby/psych.rb:350:in `parse'
	1: from /usr/share/ruby/psych.rb:402:in `parse_stream'
/usr/share/ruby/psych.rb:402:in `parse': (<unknown>): found unexpected ':' while scanning a plain scalar at line 1 column 2 (Psych::SyntaxError)

EL9

# rpm -qv ruby ; rpm -qv libyaml
ruby-3.0.4-160.el9.x86_64
libyaml-0.2.5-7.el9.x86_64
# ruby -ryaml -e 'puts YAML.safe_load("{a:test}").inspect'
{"a:test"=>nil}

Ubuntu 20.04

# dpkg -l | grep -E 'libruby|libyaml'
ii  libruby2.7:amd64        2.7.0-5ubuntu1.12            amd64        Libraries necessary to run Ruby 2.7
ii  libyaml-0-2:amd64       0.2.2-1                      amd64        Fast YAML 1.1 parser and emitter library
# ruby -ryaml -e 'puts YAML.safe_load("{a:test}").inspect'
{"a:test"=>nil}

So I'm going to say it's a difference between libyaml 0.1.x and 0.2.x.

@ekohl
Copy link
Member

ekohl commented Nov 17, 2023

I took the liberty of rebasing on #9908 and adding a commit that addresses the feedback to see if that works.

evgeni
evgeni previously approved these changes Nov 23, 2023
Copy link
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

three nitpicks

.github/workflows/foreman.yml Outdated Show resolved Hide resolved
.github/workflows/foreman.yml Outdated Show resolved Hide resolved
.github/workflows/foreman.yml Outdated Show resolved Hide resolved
@ofedoren
Copy link
Member Author

#9908 was merged, so this needs a rebase I guess.

@evgeni
Copy link
Member

evgeni commented Nov 23, 2023

#9908 was merged, so this needs a rebase I guess.

done and applied my nitpicks

Co-Authored-by: Ewoud Kohl van Wijngaarden <[email protected]>
Co-Authored-by: Evgeni Golov <[email protected]>
@evgeni evgeni merged commit b26b09f into theforeman:develop Nov 23, 2023
7 of 11 checks passed
@ekohl
Copy link
Member

ekohl commented Nov 23, 2023

And theforeman/actions#19 makes use of the new matrix.postgresql.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants