Add zeitwerk loader and teach it about this gem #551

jrafanie · 2022-12-10T00:55:56Z

Depends on:

Core using classic loader (just this PR) - green cross repo: [BOT] Cross repo test for ManageIQ/manageiq-gems-pending#551 manageiq-cross_repo-tests#800

specify load path directories
inform of organizational directories that aren't namespaces
specific files that must be required and not autoloaded
custom inflectors where we deviate from convention
remove requires for autoload-able files (caution must be taken when we need to monkey patch as we need to require or we may never get our monkey patch loaded)
use require_relative for ones that must be manually required

TODO:

Extract commits that are completely backward compatible Zeitwerk friendly changes #559
Keep WIP the remaining parts that depend on having zeitwerk
Merge PRs removing dependency on lib/manageiq-gems-pending.rb file
- Use namespaced gem require instead of hyphenated name manageiq#22548
- Use namespaced gem require instead of hyphenated name amazon_ssa_support#96

jrafanie · 2022-12-10T00:57:07Z

lib/manageiq-gems-pending.rb

+loader.setup
+
+# TODO: consider turning on eager_load and only disable really slow/fat requires
+# loader.eager_load


This is what I enabled locally to get everything to fail so I could then fix or ignore.

jrafanie · 2022-12-10T00:57:54Z

manageiq-gems-pending.gemspec

  s.add_runtime_dependency "winrm",                   "~> 2.1"
  s.add_runtime_dependency "winrm-elevated",          "~> 1.0.1"
+  s.add_runtime_dependency "zeitwerk"


probably want to lock this down to a minor version

lib/gems/pending/util/miq_winrm.rb

lib/manageiq-gems-pending.rb

See: ManageIQ/manageiq#22279 I noticed this while looking to add zeitwerk in gems pending, see: ManageIQ/manageiq-gems-pending#551 I used the following to find these: git grep -iE "win_rm|winrm|scvmm|hyperv"

See: ManageIQ#22279 Related: ManageIQ/manageiq-smartstate#172 Related: ManageIQ/manageiq-gems-pending#551

See: ManageIQ/manageiq#22279 Related: ManageIQ/manageiq#22413 ManageIQ/manageiq-smartstate#172 Found while looking at: ManageIQ#551

See: ManageIQ#22279 Related: ManageIQ/manageiq-smartstate#172 Related: ManageIQ/manageiq-gems-pending#551

kbrock · 2023-03-23T19:49:59Z

lib/gems/pending/util/miq-xml.rb

@@ -41,7 +41,7 @@ def self.xml_document(xmlClass)
      when :xmlhash


tangent: do we have enough coverage to know if we can drop :xmlhash or :rexml?

aah, default is :rexml

I had assumed we only used this in a few rare places, but besides smart state (and specs), looks like we use it in kubernetes, vmware, ui classic (only in 1-2 files each) along with a dozen core models.

good question... I haven't looked recently but in the past we had no coverage of this area so it's hard to know if you can just drop it.

kbrock · 2023-03-23T22:45:27Z

We used to require util/ for all of these. Happy that you are getting rid of these.

Fryguy · 2023-06-09T15:44:24Z

lib/manageiq/gems/pending/zeitwerk.rb

+lib = File.expand_path("#{__dir__}../../../../")
+loader.push_dir("#{lib}/gems/pending/util")
+
+# This tells the loader to not expect Mount or ObjectStorage namspaces...


🦅 👁️

Suggested change

# This tells the loader to not expect Mount or ObjectStorage namspaces...

# This tells the loader to not expect Mount or ObjectStorage namespaces...

Fryguy · 2023-06-09T15:44:47Z

lib/manageiq/gems/pending/zeitwerk.rb

+# These files skip zeitwerk and must be required manually
+##### TODO: check for requires outside of this gem to these files and util/ in requires
+loader.ignore("#{lib}/gems/pending/util/miq-extensions.rb")       # loader file, no class
+loader.ignore("#{lib}/gems/pending/util/require_with_logging.rb") # file has no class ;-)


We should move this out into a diagnostics thing or maybe move it into core.

See: ManageIQ#22279 Related: ManageIQ/manageiq-smartstate#172 Related: ManageIQ/manageiq-gems-pending#551

* specify load path directories * organizational directories that aren't namespaces * specific files that must be required and not autoloaded * custom inflectors where we deviate from convention

Unfortunately, miq_rexml is different because we need to monkey patch it early.

…them Once we eager load, we can remove these.

Use for_gem_extension provided in zeitwerk 2.6.8

miq-bot · 2023-08-21T17:06:16Z

Checked commits jrafanie/manageiq-gems-pending@d23ca2f~...d560e92 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
45 files checked, 4 offenses detected

lib/gems/pending/util/miq_file_storage.rb

⚠️ - Line 51, Col 7 - Lint/Void - Variable MiqGlusterfsSession used in void context.
⚠️ - Line 52, Col 7 - Lint/Void - Variable MiqLocalMountSession used in void context.
⚠️ - Line 53, Col 7 - Lint/Void - Variable MiqNfsSession used in void context.
⚠️ - Line 54, Col 7 - Lint/Void - Variable MiqSmbSession used in void context.

jrafanie · 2023-08-21T19:53:44Z

ok, this is green other than the already existing constants:

The cross repo is green other than ui service

jrafanie · 2023-08-21T19:54:52Z

I think this is ready to go. Once in and backported to quinteros, we can decide about about the core PR. They can be independent upgrades.

Fixes an issue hit with bin/update but not bundle update. The hypenated manageiq-gems-pending gem name file was removed as bundler/rubygems wants you to use manageiq/gems/pending file layout. Due to ManageIQ/manageiq-gems-pending#551

Fixes an issue hit with bin/update but not bundle update. The hyphenated manageiq-gems-pending gem name file was removed as bundler/rubygems wants you to use manageiq/gems/pending file layout. Due to ManageIQ/manageiq-gems-pending#551

Fryguy · 2023-10-27T15:57:19Z

Skipping backport to quinteros, because it is already in the branch.

jrafanie commented Dec 10, 2022

View reviewed changes

miq-bot added the wip label Dec 10, 2022

jrafanie commented Dec 10, 2022

View reviewed changes

jrafanie force-pushed the zeitwerk branch 2 times, most recently from ce8f964 to dd9cbc0 Compare December 12, 2022 15:08

jrafanie mentioned this pull request Dec 12, 2022

Move manageiq and plugins to use zeitwerk autoloader instead of classic ManageIQ/manageiq#22000

Closed

55 tasks

jrafanie force-pushed the zeitwerk branch from c589e87 to 6c718f8 Compare December 15, 2022 20:48

Fryguy reviewed Dec 15, 2022

View reviewed changes

lib/gems/pending/util/miq_winrm.rb Outdated Show resolved Hide resolved

Fryguy reviewed Dec 15, 2022

View reviewed changes

lib/manageiq-gems-pending.rb Outdated Show resolved Hide resolved

jrafanie force-pushed the zeitwerk branch from 6c718f8 to 738b44a Compare December 16, 2022 21:20

jrafanie mentioned this pull request Mar 16, 2023

Remove scvmm and hyperv since core dropped it ManageIQ/manageiq-smartstate#172

Merged

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Mar 16, 2023

Drop scvmm/hyperv references since it was dropped in ManageIQ#22279

c46d231

See: ManageIQ#22279 Related: ManageIQ/manageiq-smartstate#172 Related: ManageIQ/manageiq-gems-pending#551

jrafanie mentioned this pull request Mar 16, 2023

Drop scvmm/hyperv references since it was dropped in #22279 ManageIQ/manageiq#22413

Merged

jrafanie mentioned this pull request Mar 16, 2023

Drop winrm/scvmm/hyperv support since core dropped it #556

Merged

jrafanie added a commit to jrafanie/manageiq that referenced this pull request Mar 16, 2023

Drop scvmm/hyperv references since it was dropped in ManageIQ#22279

8e47670

See: ManageIQ#22279 Related: ManageIQ/manageiq-smartstate#172 Related: ManageIQ/manageiq-gems-pending#551

jrafanie force-pushed the zeitwerk branch from 738b44a to f5779db Compare March 16, 2023 21:34

miq-bot added the unmergeable label Mar 21, 2023

jrafanie force-pushed the zeitwerk branch 2 times, most recently from c06c761 to 0aebbc0 Compare March 22, 2023 20:26

kbrock reviewed Mar 23, 2023

View reviewed changes

miq-bot removed the unmergeable label Apr 28, 2023

jrafanie force-pushed the zeitwerk branch from eaa10c8 to f76b8cb Compare May 2, 2023 17:45

jrafanie mentioned this pull request May 2, 2023

Clarification about usage with nested subgems fxn/zeitwerk#138

Closed

jrafanie force-pushed the zeitwerk branch from f76b8cb to 850bc82 Compare May 2, 2023 18:36

jrafanie mentioned this pull request May 3, 2023

Zeitwerk friendly changes #559

Merged

miq-bot added the unmergeable label May 4, 2023

jrafanie force-pushed the zeitwerk branch from 850bc82 to e08d30e Compare May 4, 2023 15:06

miq-bot mentioned this pull request Jun 8, 2023

[BOT] Cross repo test for ManageIQ/manageiq-gems-pending#551 ManageIQ/manageiq-cross_repo-tests#800

Open

Fryguy reviewed Jun 9, 2023

View reviewed changes

GilbertCherrie pushed a commit to GilbertCherrie/manageiq that referenced this pull request Jul 7, 2023

Drop scvmm/hyperv references since it was dropped in ManageIQ#22279

425ebd9

See: ManageIQ#22279 Related: ManageIQ/manageiq-smartstate#172 Related: ManageIQ/manageiq-gems-pending#551

jrafanie closed this Aug 10, 2023

jrafanie reopened this Aug 10, 2023

jrafanie added 5 commits August 21, 2023 13:00

Add zeitwerk loader and teach it about this gem

d23ca2f

* specify load path directories * organizational directories that aren't namespaces * specific files that must be required and not autoloaded * custom inflectors where we deviate from convention

Remove requires for autoload-able files.

16fbcb4

Unfortunately, miq_rexml is different because we need to monkey patch it early.

Use require_relative for ones that can't be autoloaded.

1b96e20

Temporarily eager load these since storage_interface_classes expects …

3fbfed1

…them Once we eager load, we can remove these.

Drop lib/manageiq-gems-pending, add zeitwerk.rb

d560e92

Use for_gem_extension provided in zeitwerk 2.6.8

jrafanie force-pushed the zeitwerk branch from 23a8665 to d560e92 Compare August 21, 2023 17:00

jrafanie changed the title ~~[WIP] Add zeitwerk loader and teach it about this gem~~ Add zeitwerk loader and teach it about this gem Aug 21, 2023

jrafanie added quinteros/yes? and removed wip labels Aug 21, 2023

jrafanie assigned kbrock Aug 21, 2023

agrare approved these changes Aug 23, 2023

View reviewed changes

agrare merged commit 943bd66 into ManageIQ:master Aug 23, 2023
3 checks passed

jrafanie deleted the zeitwerk branch August 23, 2023 18:09

jrafanie added the dependencies label Aug 23, 2023

jrafanie mentioned this pull request Aug 24, 2023

Let bundler do the require ManageIQ/manageiq#22682

Merged

Fryguy added quinteros/yes and removed quinteros/yes? labels Oct 27, 2023

Fryguy removed the quinteros/yes label Oct 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add zeitwerk loader and teach it about this gem #551

Add zeitwerk loader and teach it about this gem #551

jrafanie commented Dec 10, 2022 •

edited

Loading

jrafanie Dec 10, 2022

jrafanie Dec 10, 2022

kbrock Mar 23, 2023 •

edited

Loading

kbrock Mar 23, 2023

jrafanie Jun 5, 2023

kbrock commented Mar 23, 2023

Fryguy Jun 9, 2023

Fryguy Jun 9, 2023

miq-bot commented Aug 21, 2023

jrafanie commented Aug 21, 2023

jrafanie commented Aug 21, 2023

Fryguy commented Oct 27, 2023

		@@ -41,7 +41,7 @@ def self.xml_document(xmlClass)
		when :xmlhash

	# This tells the loader to not expect Mount or ObjectStorage namspaces...
	# This tells the loader to not expect Mount or ObjectStorage namespaces...

Add zeitwerk loader and teach it about this gem #551

Add zeitwerk loader and teach it about this gem #551

Conversation

jrafanie commented Dec 10, 2022 • edited Loading

jrafanie Dec 10, 2022

Choose a reason for hiding this comment

jrafanie Dec 10, 2022

Choose a reason for hiding this comment

kbrock Mar 23, 2023 • edited Loading

Choose a reason for hiding this comment

kbrock Mar 23, 2023

Choose a reason for hiding this comment

jrafanie Jun 5, 2023

Choose a reason for hiding this comment

kbrock commented Mar 23, 2023

Fryguy Jun 9, 2023

Choose a reason for hiding this comment

Fryguy Jun 9, 2023

Choose a reason for hiding this comment

miq-bot commented Aug 21, 2023

jrafanie commented Aug 21, 2023

jrafanie commented Aug 21, 2023

Fryguy commented Oct 27, 2023

jrafanie commented Dec 10, 2022 •

edited

Loading

kbrock Mar 23, 2023 •

edited

Loading