Skip to content

Add support for puppetfile resolver#73

Open
logicminds wants to merge 2 commits into
masterfrom
puppetfile_resolver
Open

Add support for puppetfile resolver#73
logicminds wants to merge 2 commits into
masterfrom
puppetfile_resolver

Conversation

@logicminds

Copy link
Copy Markdown
Contributor
  • This adds experimental support for the puppetfile resolver.
    Since the resolver is better, faster, stronger it might be
    useful to use this library in the future. A resolve task
    task has been added but it is not clear what that yet provides.

@codecov

codecov Bot commented Apr 22, 2022

Copy link
Copy Markdown

Codecov Report

Patch coverage: 80.00% and project coverage change: +0.76% 🎉

Comparison is base (eee5054) 65.44% compared to head (80c6504) 66.20%.

❗ Current head 80c6504 differs from pull request most recent head 09f6cef. Consider uploading reports for the commit 09f6cef to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
+ Coverage   65.44%   66.20%   +0.76%     
==========================================
  Files          12       13       +1     
  Lines         544      574      +30     
==========================================
+ Hits          356      380      +24     
- Misses        188      194       +6     
Files Changed Coverage Δ
lib/ra10ke/resolver.rb 77.77% <77.77%> (ø)
lib/ra10ke.rb 97.82% <100.00%> (+0.15%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@logicminds

logicminds commented Apr 22, 2022

Copy link
Copy Markdown
Contributor Author

Hey @glennsarti ^^

Not sure what exactly the output should look like. Currently looks broken.

This adds a r10k::resolver task which implements your example code.

Example output? https://github.com/voxpupuli/ra10ke/runs/6136277429?check_suite_focus=true

@logicminds logicminds force-pushed the puppetfile_resolver branch from ca90029 to 78a72bf Compare April 22, 2022 23:15
Comment thread lib/ra10ke/resolver.rb
class Instance
attr_reader :puppetfile

def initialize(puppetfile_path = File.expand_path(Dir.pwd))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Isn't this a directory, not a file? Cant read a dir

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. why yes that would fail.

Comment thread lib/ra10ke/resolver.rb
# Create the resolver
# - Use the document we just parsed (puppetfile)
# - Don't restrict by Puppet version (nil)
resolver = ::PuppetfileResolver::Resolver.new(puppetfile, nil)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

puppetfile should be @puppetfile

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is an attr_reader for puppetfile

@glennsarti

Copy link
Copy Markdown

The spec should be:

  it 'resolves the puppetfile' do
   expect(instance.resolve).to be_a(PuppetfileResolver::ResolutionResult)
  end

@glennsarti

glennsarti commented Apr 23, 2022

Copy link
Copy Markdown

There's a bunch of problems with your fixture Puppetfile (spec/fixtures/Puppetfile)

mod 'puppet-acl',
    git: 'https://github.com/dobbymoodge/puppet-acl.git',
    branch: 'master'

That git URL redirects to https://github.com/voxpupuli/puppet-posix_acl which is actually puppet-posix_acl module.

The gitlab module conflicts with choria, i.e. There is no version of apt that can satisfy both module requirements.

 - `puppetlabs-apt >=1.8.0 <3.0.0` required by `Git vshn-gitlab-1.4.1`
 - `puppetlabs-apt >= 4.5.1 < 9.0.0` required by `Forge choria-choria-0.26.2`

@logicminds

Copy link
Copy Markdown
Contributor Author

In the test result it shows a stacktrace. Additionally I was not expecting output instead I was hoping to just capture PuppetfileResolver::ResolutionResult and output the results later. So question is why is stuff being thrown to stdout? Guessing this is because of the thrown exception.

The fixtures have issues because they are used to show deprecations and other problems. So they work as designed but have the problems on purpose in order to tell the user to change their Puppetfile. I bet we don't have code that shows the module moved in github though. Would be a nice feature.

Regarding apt issue. I would like to programmatically detect this situation and suggest to the user to change something. I feel like the casual user won't be able to understand the output without some interpretation from Ra10ke.

Aside from this task what else can the resolver gem do that we should add?

1) Ra10ke::Resolver::Instance resolves the puppetfile
     Failure/Error: result = resolver.resolve(opts)

     PuppetfileResolver::Puppetfile::DocumentVersionConflictError:
       Puppetfile Resolver could not find compatible versions for possibility named "acl":
         In Puppetfile:
           puppet-acl >= 0

       Puppetfile Resolver could not find compatible versions for possibility named "inifile":
         In Puppetfile:
           -r10k >= 0 was resolved to Git zack-r10k-3.1.1, which depends on
             puppetlabs-inifile >= 1.0.0

           puppetlabs-inifile =2.2.0

       Puppetfile Resolver could not find compatible versions for possibility named "stdlib":
         In Puppetfile:
           -gms >= 0 was resolved to Git abrader-gms-1.0.1, which depends on
             puppetlabs-stdlib >=3.2.0 <5.0.0

           -r10k >= 0 was resolved to Git zack-r10k-3.1.1, which depends on
             puppetlabs-stdlib >= 4.6.0

           -gitlab >= 0 was resolved to Git vshn-gitlab-1.4.1, which depends on
             puppetlabs-stdlib 4.x

           puppet-archive =3.1.1 was resolved to Forge puppet-archive-3.1.1, which depends on
             puppetlabs-stdlib >= 4.13.1 < 5.0.0

           puppetlabs-ntp =6.4.1 was resolved to Forge puppetlabs-ntp-6.4.1, which depends on
             puppetlabs-stdlib >= 4.13.1 < 5.0.0

           puppetlabs-concat =4.0.0 was resolved to Forge puppetlabs-concat-4.0.0, which depends on
             puppetlabs-stdlib >= 4.13.1 < 5.0.0

           puppetlabs-stdlib =4.24.0

           puppetlabs-ruby =1.0.1 was resolved to Forge puppetlabs-ruby-1.0.1, which depends on
             puppetlabs-stdlib >= 4.13.1 < 7.0.0

           choria-choria =0.26.2 was resolved to Forge choria-choria-0.26.2, which depends on
             puppetlabs-stdlib >= 4.24.0 < 8.0.0
     # ./vendor/bundle/ruby/3.0.0/gems/molinillo-0.8.0/lib/molinillo/resolution.rb:317:in `raise_error_unless_state'
     # ./vendor/bundle/ruby/3.0.0/gems/molinillo-0.8.0/lib/molinillo/resolution.rb:299:in `block in unwind_for_conflict'
     # <internal:kernel>:90:in `tap'
     # ./vendor/bundle/ruby/3.0.0/gems/molinillo-0.8.0/lib/molinillo/resolution.rb:297:in `unwind_for_conflict'
     # ./vendor/bundle/ruby/3.0.0/gems/molinillo-0.8.0/lib/molinillo/resolution.rb:257:in `process_topmost_state'
     # ./vendor/bundle/ruby/3.0.0/gems/molinillo-0.8.0/lib/molinillo/resolution.rb:182:in `resolve'
     # ./vendor/bundle/ruby/3.0.0/gems/molinillo-0.8.0/lib/molinillo/resolver.rb:43:in `resolve'
     # ./vendor/bundle/ruby/3.0.0/gems/puppetfile-resolver-0.5.0/lib/puppetfile-resolver/resolver.rb:[37](https://github.com/voxpupuli/ra10ke/runs/6136277291?check_suite_focus=true#step:4:37):in `resolve'
     # ./lib/ra10ke/resolver.rb:[49](https://github.com/voxpupuli/ra10ke/runs/6136277291?check_suite_focus=true#step:4:49):in `resolve'
     # ./spec/ra10ke/resolver_spec.rb:18:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # Molinillo::VersionConflict:
     #   Unable to satisfy the following requirements:
     #   
     #   - `puppetlabs-stdlib =4.24.0` required by `Puppetfile`
     #   - `puppetlabs-stdlib >= 4.24.0 < 8.0.0` required by `Forge choria-choria-0.26.2`
     #   - `puppetlabs-stdlib >= 4.13.1 < 7.0.0` required by `Forge puppetlabs-ruby-1.0.1`
     #   - `puppetlabs-stdlib >= 4.13.1 < 5.0.0` required by `Forge puppetlabs-concat-4.0.0`
     #   - `puppetlabs-stdlib >= 4.13.1 < 5.0.0` required by `Forge puppetlabs-ntp-6.4.1`
     #   - `puppetlabs-stdlib >= 4.13.1 < 5.0.0` required by `Forge puppet-archive-3.1.1`
     #   - `puppetlabs-stdlib 4.x` required by `Git vshn-gitlab-1.4.1`
     #   - `puppetlabs-stdlib >= 4.6.0` required by `Git zack-r10k-3.1.1`
     #   - `puppetlabs-stdlib >=3.2.0 <5.0.0` required by `Git abrader-gms-1.0.1`
     #   - `puppetlabs-inifile =2.2.0` required by `Puppetfile`
     #   - `puppetlabs-inifile >= 1.0.0` required by `Git zack-r10k-3.1.1`
     #   - `puppet-acl >= 0` required by `Puppetfile`
     #   ./vendor/bundle/ruby/3.0.0/gems/molinillo-0.8.0/lib/molinillo/resolution.rb:317:in `raise_error_unless_state'

@glennsarti

Copy link
Copy Markdown

Ahh ok... that makes more sense...

As you guessed. The resolver is throwing an error PuppetfileResolver::Puppetfile::DocumentVersionConflictError and rspec is just dumping out the text.

I basically wrapped the original molinillo error (https://www.rubydoc.info/gems/molinillo/Molinillo/VersionConflict) so all the information is there... but detecting the apt version specification problem is ... tricky. It would probably be possible to detect, but navigating the molinillo trees etc. requires indepth work. No idea how long it would take to figure out.

As for the https://github.com/voxpupuli/puppet-posix_acl issue. That could be detected in the puppetfile resolver.

e.g. given a module name of 'x' and the metadata.json says the module name is 'y'. It could throw an error. In (https://github.com/glennsarti/puppetfile-resolver/blob/main/lib/puppetfile-resolver/models/module_specification.rb#L41)

@bastelfreak

Copy link
Copy Markdown
Member

@logicminds hey, any chance you could pick this up again? I think this would be a very awesome addition to ra10ke

  * This adds experimental support for the puppetfile resolver.
    Since the resolver is better, faster, stronger it might be
    useful to use this library in the future.  A resolve task
    task has been added but it is not clear what that yet provides.
@kenyon kenyon force-pushed the puppetfile_resolver branch from 78a72bf to 5f0f3d8 Compare September 11, 2023 22:44
@kenyon

kenyon commented Sep 11, 2023

Copy link
Copy Markdown
Member

Rebased on current master (commit eee5054), resolved merge conflicts, and resolved rubocop complaints. I was looking at this because it would be nice to have a Puppetfile dependency resolver that could restrict module versions based on Puppet version.

@bastelfreak

Copy link
Copy Markdown
Member

@kenyon thanks for the rebase. Any chance you can take a look at the failing unit test?

@kenyon

kenyon commented Sep 13, 2023

Copy link
Copy Markdown
Member

Well, I realized that since I read all of the module changelogs when I'm updating my Puppetfile, it's really a manual process so a rake task like this isn't useful enough to work on this PR at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants