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

[WIP] Allow updating all plugins when building the lockfile #22719

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Sep 28, 2023

The lockfile generator is very conservative and doesn't update the lockfile refs for git repos (i.e. plugins). As such, when gem updates happen in plugins, they aren't automatically pulled into the lockfile. This new option allows me to see which specific plugins are making changes. I can then choose to just update them all, or pick and choose.

@bdunne Please review.

The lockfile generator is very conservative and doesn't update the
lockfile refs for git repos (i.e. plugins). As such, when gem updates
happen in plugins, they aren't automatically pulled into the lockfile.
This new option allows me to see which specific plugins are making
changes. I can then choose to just update them all, or pick and choose.
@Fryguy
Copy link
Member Author

Fryguy commented Sep 28, 2023

I've also considered pushing does that weird repo path scanning logic into Vmdb::Plugins - if that's preferable, let me know. I only didn't do it because I couldn't decide on the key/method name.

────────────────────────────────────────────────────────
modified: lib/tasks/release.rake
────────────────────────────────────────────────────────
@ lib/tasks/release.rake:190 @ namespace :release do
     update_gems = ENV["UPDATE_GEMS"].to_s.split(" ")
     if ENV["UPDATE_PLUGINS"]
       require "vmdb/plugins"
-      update_gems += Vmdb::Plugins.paths.values.map { |p| p.match(%r{/gems/(.+)-\h+$}).captures.first }
+      update_gems += Vmdb::Plugins.repos.values
     end

     root = Pathname.new(__dir__).join("../..")
────────────────────────────────────────────────────────
modified: lib/vmdb/plugins.rb
────────────────────────────────────────────────────────
@ lib/vmdb/plugins.rb:37 @ def details
         hash[engine] = {
           :name    => engine.name,
           :version => version(engine),
-          :path    => engine.root.to_s
+          :path    => engine.root.to_s,
+          :repo    => engine.root.to_s.match(%r{/gems/(.+)-\h+$}).captures.first
         }
       end
     end
@ lib/vmdb/plugins.rb:51 @ def versions
       details.transform_values { |v| v[:version] }
     end

+    def repos
+      details.transform_values { |v| v[:repo] }
+    end
+
     # Ansible content (roles) that come out-of-the-box, for use by both Automate
     #   and ansible-runner
     def ansible_content

@@ -185,6 +185,10 @@ namespace :release do
end

update_gems = ENV["UPDATE_GEMS"].to_s.split(" ")
if ENV["UPDATE_PLUGINS"]
require "vmdb/plugins"
update_gems += Vmdb::Plugins.paths.values.map { |p| p.match(%r{/gems/(.+)-\h+$}).captures.first }
Copy link
Member Author

@Fryguy Fryguy Sep 28, 2023

Choose a reason for hiding this comment

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

This regex is checking the on-disk path (e.g. /Users/jfrey/.gem/ruby/3.0.6/bundler/gems/manageiq-providers-workflows-a7c24c7cdd32), then pulling out the gem/repo name (e.g. manageiq-providers-workflows)

I'm not sure if there's a better way? I guess I could also take the engine name and underscorize it, but I think there's some inconsistencies like manageiq-consumption provides ManageIQ::Showback1 (i.e. "/Users/jfrey/.gem/ruby/3.0.6/bundler/gems/manageiq-consumption-561f74d85a02" -> ManageIQ::Showback::Engine)

Footnotes

  1. https://github.com/ManageIQ/manageiq-consumption/tree/561f74d85a026dc82b3b8bfa4a53f25edfc609bb/lib/manageiq

@Fryguy Fryguy changed the title Allow updating all plugins when building the lockfile [WIP] Allow updating all plugins when building the lockfile Sep 29, 2023
@miq-bot miq-bot added the wip label Sep 29, 2023
@miq-bot
Copy link
Member

miq-bot commented Sep 29, 2023

Checked commits Fryguy/manageiq@c30ce70~...b58e156 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 👍

@Fryguy
Copy link
Member Author

Fryguy commented Sep 29, 2023

Moved to WIP because when you are running from a plug-in the path is your local dev path. Need to handle that case and be more defensive here in general.

@miq-bot
Copy link
Member

miq-bot commented Nov 6, 2023

This pull request is not mergeable. Please rebase and repush.

@miq-bot
Copy link
Member

miq-bot commented Feb 19, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

2 similar comments
@miq-bot
Copy link
Member

miq-bot commented May 20, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

@miq-bot
Copy link
Member

miq-bot commented Aug 26, 2024

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s).

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

Successfully merging this pull request may close these issues.

3 participants