-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Consolidate gem dependencies and metadata #9306
Comments
Migrated issue to PUP-12031 |
Hi, |
Agreed, we're investigating this approach. It also means the |
Fixed in #9325 The And there is a rake task ( |
Change was backported to 7.x in #9337 |
TL;DR
We should remove the gem metadata contained in
ext/project_data.yaml
for thepuppet gem, instead use the gemspec as the source of truth
when building it.
Background
Puppet (and Facter's) gemspec files are only used when running under bundler,
but not when building the gems!! Since there are multiple sources of truth, we
have frequently updated one but not the other. Most recently, the puppet 8.0.0
gem listed the wrong minimum required ruby version, forcing us to release 8.0.1
One complexity is we publish platform-specific gems for puppet:
https://rubygems.org/downloads/puppet-8.5.1.gem
https://rubygems.org/downloads/puppet-8.5.1-x86-mingw32.gem
https://rubygems.org/downloads/puppet-8.5.1-x64-mingw32.gem
https://rubygems.org/downloads/puppet-8.5.1-universal-darwin.gem
The reason is because puppet depends on CFPropertyList on macOS and ffi on
Windows. By publishing platform specific gems, we can add runtime dependencies
that only affect that platform.
You might be asking why not just add CFPropertyList and ffi for all platforms?
The reason is those gems contain native extensions, and so would require a
compiler and ruby-dev packages to be installed in order to
gem install puppet
.Technically, CFPropertyList itself doesn't contain native extensions, but it has
at various times had a runtime dependency on nokogiri, which does.
Proposal
Move and reconcile all of the common metadata from
ext/project_data.yaml
tothe gemspec, such as author, description, required ruby version, etc.
Move
.gemspec
back topuppet.gemspec
, since it should be used to build thegem, see commit 8f81b522a98472
For the platform-specific gems, create corresponding
puppet-<platform>.gemspec
files. Each gemspec can append platform specificdependencies like:
Modify our build process to build the generic gem:
Optionally, build platform specific gems in a data driven way. In other
words, the build automation should not have a list of platforms to build. It
should be based on the names of the gemspec files. For example, we could
conceivably have different platform-specific gemspec in 7.x vs main branches.
One downside of this approach is
Gemfile
will need to contain conditional logic to include the rightgemspec
file, taking into account the bundler and rubygems use different platform names:Alternatives
It's possible to add conditional logic in the gemspec, to include additional runtime
dependencies based on the platform we're packaging for:
And then specify the platform to build for
gem build --platform x64-mingw32 puppet.gemspec
. One nice thing is theGemfile
would not need to change. However, the list of the platforms would need to be stored somewhere, and could be different for 7.x vs main.Additional Context
1342839
2707fd8
https://puppet.atlassian.net/browse/PA-1077
https://puppet.atlassian.net/browse/PA-2058
https://puppet.atlassian.net/browse/PUP-8685
The text was updated successfully, but these errors were encountered: