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

(PUP-12031) Consolidate gem depenendencies & metadata into puppet.gem… #9325

Merged
merged 2 commits into from
Apr 23, 2024

Conversation

AriaXLi
Copy link
Contributor

@AriaXLi AriaXLi commented Apr 17, 2024

…spec

This PR:

  • Moves puppet gem metadata from ext/project_yaml.data and .gemspec into puppet.gemspec so puppet gems can be built using gem build puppet.gemspec
  • Adds conditional logic in puppet.gemspec for platform specific dependencies like ffi and CFPropertyList when building platform specific gems (using --platform)
  • Adds Rake tasks that builds these puppet gems

@AriaXLi AriaXLi added the enhancement New feature or request label Apr 17, 2024
@AriaXLi AriaXLi requested a review from a team as a code owner April 17, 2024 22:54
@AriaXLi
Copy link
Contributor Author

AriaXLi commented Apr 17, 2024

Not sure if enhancement or maintenance label is better?

@AriaXLi
Copy link
Contributor Author

AriaXLi commented Apr 17, 2024

Successfully built gem using 'gem build puppet.gemspec'

❯ gem build puppet.gemspec
  Successfully built RubyGem
  Name: puppet
  Version: 8.7.0
  File: puppet-8.7.0.gem
❯ gem build puppet.gemspec --platform universal-darwin
  Successfully built RubyGem
  Name: puppet
  Version: 8.7.0
  File: puppet-8.7.0-universal-darwin.gem
❯ gem build puppet.gemspec --platform x64-mingw32
  Successfully built RubyGem
  Name: puppet
  Version: 8.7.0
  File: puppet-8.7.0-x64-mingw32.gem
  ❯ gem build puppet.gemspec --platform x86-mingw32
  Successfully built RubyGem
  Name: puppet
  Version: 8.7.0
  File: puppet-8.7.0-x86-mingw32.gem

@AriaXLi AriaXLi marked this pull request as draft April 17, 2024 22:59
@AriaXLi AriaXLi force-pushed the PUP-12031 branch 3 times, most recently from b995285 to 212d41d Compare April 18, 2024 16:27
@AriaXLi AriaXLi marked this pull request as ready for review April 18, 2024 16:27
@joshcooper
Copy link
Contributor

Somehow bundle install on Linux used to install the CFPropertyList gem:

https://github.com/puppetlabs/puppet/actions/runs/8713544694/job/23902159199#step:4:90

I'm guessing it's due to the all platform in

universal-darwin: all
and
if bundle_platform == "all"
but not 100% sure.

To resolve this, I think we want to guard the directoryservice_spec tests so they only run when the CFPropertyList feature is present like we do in

describe Puppet::Util::Plist, :if => Puppet.features.cfpropertylist? do

Also add the CFPropertyList gem in the features group in the Gemfile like we do for diff-lcs

puppet/Gemfile

Line 22 in fb44fd9

gem 'diff-lcs', '~> 1.3', require: false

You might also need to append platforms: [:ruby] or possibly platforms: [:ruby, :darwin] to the gem 'CFPropertyList', so that the we don't bother installing the gem or running those tests on Windows or JRuby.

@AriaXLi
Copy link
Contributor Author

AriaXLi commented Apr 18, 2024

:if => Puppet.features.cfpropertylist? do

Thanks for the advice, quick q: what does adding CFPropertyList gem to the features group do specifically? Are those gems only installed on specific platforms?

@AriaXLi AriaXLi marked this pull request as draft April 19, 2024 16:07
@AriaXLi
Copy link
Contributor Author

AriaXLi commented Apr 22, 2024

$ bundle exec rake -T
...
rake pl_ci:gem_build                                       # Build puppet gems
rake rubocop                                               # run static analysis with rubocop
rake warnings                                              # verify that changed files are clean of Ruby warnings

@AriaXLi
Copy link
Contributor Author

AriaXLi commented Apr 22, 2024

❯ bundle exec rake pl_ci:gem_build
gem build puppet.gemspec --platform x86-mingw32
  Successfully built RubyGem
  Name: puppet
  Version: 8.7.0
  File: puppet-8.7.0-x86-mingw32.gem
gem build puppet.gemspec --platform x64-mingw32
  Successfully built RubyGem
  Name: puppet
  Version: 8.7.0
  File: puppet-8.7.0-x64-mingw32.gem
gem build puppet.gemspec --platform universal-darwin
  Successfully built RubyGem
  Name: puppet
  Version: 8.7.0
  File: puppet-8.7.0-universal-darwin.gem
gem build puppet.gemspec
  Successfully built RubyGem
  Name: puppet
  Version: 8.7.0
  File: puppet-8.7.0.gem

@AriaXLi
Copy link
Contributor Author

AriaXLi commented Apr 22, 2024

Diffs to verify that the gems are being built correctly:

❯ diff <(gem specification puppet-8.6.0.gem) <(gem specification puppet-8.7.0.gem)
4c4
<   version: 8.6.0
---
>   version: 8.7.0
11c11
< date: 2024-04-09 00:00:00.000000000 Z
---
> date: 2012-08-17 00:00:00.000000000 Z
151c151,153
< description: Puppet, an automated configuration management tool
---
> description: |
>   Puppet, an automated administrative engine for your Linux, Unix, and Windows systems, performs administrative tasks
>   (such as adding users, installing packages, and updating server configurations) based on a centralized specification.
❯ diff <(gem specification puppet-8.6.0-x86-mingw32.gem) <(gem specification puppet-8.7.0-x86-mingw32.gem)
4c4
<   version: 8.6.0
---
>   version: 8.7.0
11c11
< date: 2024-04-09 00:00:00.000000000 Z
---
> date: 2012-08-17 00:00:00.000000000 Z
179c179,181
< description: Puppet, an automated configuration management tool
---
> description: |
>   Puppet, an automated administrative engine for your Linux, Unix, and Windows systems, performs administrative tasks
>   (such as adding users, installing packages, and updating server configurations) based on a centralized specification.
❯ diff <(gem specification puppet-8.6.0-x64-mingw32.gem) <(gem specification puppet-8.7.0-x64-mingw32.gem)
4c4
<   version: 8.6.0
---
>   version: 8.7.0
11c11
< date: 2024-04-09 00:00:00.000000000 Z
---
> date: 2012-08-17 00:00:00.000000000 Z
179c179,181
< description: Puppet, an automated configuration management tool
---
> description: |
>   Puppet, an automated administrative engine for your Linux, Unix, and Windows systems, performs administrative tasks
>   (such as adding users, installing packages, and updating server configurations) based on a centralized specification.
❯ diff <(gem specification puppet-8.6.0-universal-darwin.gem) <(gem specification puppet-8.7.0-universal-darwin.gem)
4c4
<   version: 8.6.0
---
>   version: 8.7.0
11c11
< date: 2024-04-09 00:00:00.000000000 Z
---
> date: 2012-08-17 00:00:00.000000000 Z
155c155
<     - - "~>"
---
>     - - ">="
157c157,160
<         version: '2.2'
---
>         version: 3.0.6
>     - - "<"
>       - !ruby/object:Gem::Version
>         version: '4'
162c165
<     - - "~>"
---
>     - - ">="
164,165c167,173
<         version: '2.2'
< description: Puppet, an automated configuration management tool
---
>         version: 3.0.6
>     - - "<"
>       - !ruby/object:Gem::Version
>         version: '4'
> description: |
>   Puppet, an automated administrative engine for your Linux, Unix, and Windows systems, performs administrative tasks
>   (such as adding users, installing packages, and updating server configurations) based on a centralized specification.

The difference in the universal-darwin gem is because in this PR, the puppet.gemspec was updated to use a newer version of CFPropertyList for universal-darwin: gem 'CFPropertyList', ['>= 3.0.6', '< 4'], require: false

❯ gem specification puppet-8.6.0-universal-darwin.gem
- !ruby/object:Gem::Dependency
  name: CFPropertyList
  requirement: !ruby/object:Gem::Requirement
    requirements:
    - - "~>"
      - !ruby/object:Gem::Version
        version: '2.2'
❯ gem specification puppet-8.7.0-universal-darwin.gem
  name: CFPropertyList
 requirement: !ruby/object:Gem::Requirement
   requirements:
   - - ">="
     - !ruby/object:Gem::Version
       version: 3.0.6
   - - "<"
     - !ruby/object:Gem::Version
       version: '4'

@AriaXLi AriaXLi marked this pull request as ready for review April 22, 2024 22:16
@joshcooper
Copy link
Contributor

what does adding CFPropertyList gem to the features group do specifically? Are those gems only installed on specific platforms?

We generally add gems to the features group for optional dependencies. For example, puppet has an optional dependency on the rdoc gem, which is used by the puppet doc application. If the gem is present, then the puppet doc functionality is enabled and we want to the rspec tests to exercise those behaviors.

Usually the need for features occurs because the gem we want to add contains native extensions (or one of its transitive dependencies does). If a gem contains native extensions and is added as a (hard) runtime dependency, but the author doesn't ship precompiled binaries, then gem install puppet will only work if you have a compiler and ruby headers and libraries (from ruby-dev/ruby-devel) installed. So it won't generally work on Windows.

FFI is an example of a gem that does provide precompiled binaries, which we rely on for Windows platforms. But many other gems aren't as thorough.

Gemfile Show resolved Hide resolved
@AriaXLi AriaXLi force-pushed the PUP-12031 branch 2 times, most recently from b6548a2 to fe5265c Compare April 22, 2024 23:16
…spec

This commit moves the puppet gem metadata from ext/project_yaml.data and
.gemspec into puppet.gemspec. This change allows the puppet gem to be built
using `gem build puppet.gemspec`. Additionally, for platform specific gems
(e.g. puppet-universal-darwin.gem and puppet-x64-mingw32.gem), there is
conditional logic in puppet.gemspec so platform specific dependecies such as
ffi are appended when building those platform-specific gems.
@AriaXLi AriaXLi force-pushed the PUP-12031 branch 3 times, most recently from 3c18d2a to 5d66a5b Compare April 23, 2024 16:50
Rakefile Outdated Show resolved Hide resolved
@AriaXLi
Copy link
Contributor Author

AriaXLi commented Apr 23, 2024

When rake command fails:

❯ bundle exec rake pl_ci:gem_build
bundler: failed to load command: rake (/Users/aria.li/.rbenv/versions/3.1.1/bin/rake)
/Users/aria.li/.rbenv/versions/3.1.1/lib/ruby/gems/3.1.0/gems/bundler-2.3.13/lib/bundler/rubygems_integration.rb:72:in `rescue in validate': The gemspec at /Users/aria.li/gh_repos/puppet/puppet.gemspec is not valid. Please fix this gemspec. (Gem::InvalidSpecificationException)
The validation error was 'puppet-8.7.0 contains itself (puppet-8.7.0.gem), check your files list'

Rakefile Outdated Show resolved Hide resolved
This commit adds a Rake task, gem:build, that will build all puppet
gems which includes the three platform specific puppet gems:
x86-mingw32, x64-mingw32, and universal-darwin
@AriaXLi AriaXLi merged commit fa213f5 into puppetlabs:main Apr 23, 2024
9 checks passed
@AriaXLi AriaXLi deleted the PUP-12031 branch April 23, 2024 22:07
@AriaXLi AriaXLi added the backport 7.x Generate a backport PR to 7.x label May 7, 2024
Copy link

github-actions bot commented May 7, 2024

Backport failed for 7.x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin 7.x
git worktree add -d .worktree/backport-9325-to-7.x origin/7.x
cd .worktree/backport-9325-to-7.x
git checkout -b backport-9325-to-7.x
ancref=$(git merge-base fb44fd90c64ee11f0a29fb8924adbab5b0695555 e58bc11b60c6781deb5b172e674cac405d4b4d6d)
git cherry-pick -x $ancref..e58bc11b60c6781deb5b172e674cac405d4b4d6d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 7.x Generate a backport PR to 7.x enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants