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

Update omnibus-overrides, switch to ruby 2.7 #1123

Merged
merged 21 commits into from
May 6, 2020
Merged

Conversation

clintoncwolfe
Copy link
Collaborator

@clintoncwolfe clintoncwolfe commented Apr 23, 2020

Description

Switches from ruby 2.6.5 to 2.7.1 and updates bundler and rubygems to match Chef Infra Client.

Related Issue

Part of #1059

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

Aha! Link: https://chef.aha.io/features/SH-1656

omnibus_overrides.rb Outdated Show resolved Hide resolved
@clintoncwolfe clintoncwolfe requested a review from a team as a code owner April 29, 2020 01:28
@netlify
Copy link

netlify bot commented Apr 29, 2020

Deploy preview for chef-workstation ready!

Built with commit 9b27255

https://deploy-preview-1123--chef-workstation.netlify.app

@tyler-ball tyler-ball force-pushed the cw/switch-to-ruby-2.7 branch 3 times, most recently from 42ab716 to de7942e Compare April 29, 2020 03:01
tyler-ball added a commit to tyler-ball/rb-fsevent that referenced this pull request Apr 29, 2020
We are currently in the [process](chef/chef-workstation#1123) of updating our package to use Ruby 2.7 which switched to using Bundler 2.x. We don't want to keep including bundler 1.x inside the package we generate so we have started going through and removing development dependencies on bundler.

This works for us because all recent versions of Ruby include bundler (`gem install bundler` is unnecessary). `bundle install` in this directory still works. Our companies Ruby development has started removing bundler as a dev dependency to make the upgrade to Ruby 2.7 easier.

Another option would be to remove the version specification, but that does cause a warning when doing `gem build`:
```
$ gem build rb-fsevent.gemspec
WARNING:  open-ended dependency on bundler (>= 0, development) is not recommended
  use a bounded requirement, such as '~> x.y'
WARNING:  See http://guides.rubygems.org/specification-reference/ for help
```
This is just a warning, but it is an annoying one.

What would you think of either of these solutions?
@@ -394,6 +394,9 @@ GEM
ffi (1.12.2)
ffi (1.12.2-x64-mingw32)
ffi (1.12.2-x86-mingw32)
ffi-compiler (1.0.1)
Copy link
Contributor

Choose a reason for hiding this comment

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

@tas50 @lamont-granquist Is this going to bite me in the ass?

Copy link
Contributor

Choose a reason for hiding this comment

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

🤷

Copy link
Contributor

Choose a reason for hiding this comment

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

The new library which builds native extensions is https://github.com/cotag/http-parser which pulls in https://github.com/nodejs/http-parser/tree/77310eeb839c4251c07184a5db8885a572a08352 which is a C extension so... hopefully not?

tenor-129399655

Copy link
Contributor

Choose a reason for hiding this comment

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

Turns out the answer is YES

@tyler-ball tyler-ball force-pushed the cw/switch-to-ruby-2.7 branch 3 times, most recently from 9b17d95 to 7ce372d Compare May 6, 2020 00:30
clintoncwolfe and others added 13 commits May 5, 2020 20:59
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Signed-off-by: Clinton Wolfe <clintoncwolfe@gmail.com>
Signed-off-by: tyler-ball <tball@chef.io>
Also cleans up a rake error

Signed-off-by: tyler-ball <tball@chef.io>
This has support for ed25519 keys correctly

Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
Ruby 2.7 ships with these, and we fixed issues in appbundler / the
omnibus-toolchain to prevent installing multiple copies.

Also bumped deps.

Signed-off-by: tyler-ball <tball@chef.io>
…test versions

Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
This is an idea for potentially what we need to upgrade NodeJS

Signed-off-by: tyler-ball <tball@chef.io>
Notes: chef/chef pulls nokogiri in using *only* an omnibus software
definition. It is not in the Gemfile. So they are manually compiling the
gem on all systems we distribute Chef to. We want to do the same thing.

In Nokogiri 1.11.0 they started distributing a pre-compiled linux
package (https://rubygems.org/gems/nokogiri/versions/1.11.0.rc2-x86_64-linux).
We don't want to use that, or any of their pre-compiled gems.
Unfortunately I cannot figure out how to get bundler to force Nokogiri
to compile but also allow OTHER gems with pre-compiled extensions to
be installed (like the -mingw32 gems).

So, for now, after bundling I manually uninstall all versions of
Nokogiri and then manually install the version that forces compilation.

Maybe we could fix this by having a Windows-specific Gemfile that
includes the -mingw32 gems, so all non-Windows systems force extension
compilation for all gems via `BUNDLE_FORCE_RUBY_PLATFORM`?

Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
Non-Windows worked correctly with only specifying
`env["NOKOGIRI_USE_SYSTEM_LIBRARIES"] = "true"` but Windows requires
more. It doesn't hurt the other systems to do this, it is just unecessary
there.

Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
Signed-off-by: tyler-ball <tball@chef.io>
@tyler-ball tyler-ball merged commit fae3f19 into master May 6, 2020
@tyler-ball tyler-ball deleted the cw/switch-to-ruby-2.7 branch May 6, 2020 06:01
@@ -48,7 +47,6 @@
ed25519.png
example
examples
ext
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this for nokogiri or the http gem mess. This is going to bloat the builds pretty significantly.

Copy link
Contributor

Choose a reason for hiding this comment

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

This was for the http gem - specifically without this it is missing http-parser-ext.

@jonsmorrow jonsmorrow added the Epic label Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants