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

Issue with gem loading with rails/zeitwerk on linux #24

Closed
cecilian opened this issue Feb 26, 2021 · 8 comments
Closed

Issue with gem loading with rails/zeitwerk on linux #24

cecilian opened this issue Feb 26, 2021 · 8 comments

Comments

@cecilian
Copy link

Hello,

the fact that the gem name contains two uppercase characters makes it unavailable under case-sensitive OSs like linux because autoloading with zeitwerk doesn't manage to pick it up.

Manual require fails too:

irb(main):001:0> require 'IPinfo'
LoadError (cannot load such file -- IPinfo)

but it works using the file name in lowercase:

irb(main):002:0> require 'ipinfo'
=> true

In my case all was working in my development environment (OSX) but failed on circleCI.
At the moment the only workaround I've found is adding the following line to application.rb:

require 'ipinfo' unless defined?(IPinfo)

Would it be possible to address this issue so that the gem can be required automatically also under linux?
Thank you

gem version: 1.0.1
rails version: 6.0.3.2

@TheJohnzo
Copy link

I'm also seeing this issue. require 'IPinfo' works fine locally in OSX but fails on our linux servers. Would like to see an official patch for this to smooth things out. Otherwise, a handy gem to use.

Ruby 2.7.2
Rails 5.2.4.4
Gem: 1.0.1

@UmanShahzad
Copy link
Contributor

Hey guys, thanks for the report.

This gem was named a while ago and it seems it didn't follow the recommendation made at https://guides.rubygems.org/name-your-gem/:

DON’T USE UPPERCASE LETTERS
OS X and Windows have case-insensitive filesystems by default. Users may mistakenly require files from a gem using uppercase letters which will be non-portable if they move it to a non-windows or OS X system. While this will mostly be a newbie mistake we don’t need to be confusing them more than necessary.

So I'll recommend importing manually using lowercase, always, and apologize for the mistake made here in the first place.

For the specific case mentioned by @cecilian, we'll have to think of something else, because it seems the "zeitwerk" tool can't auto-discover the gem (is that right?). Renaming the gem itself at this point seems improbable but not off the table. You can possibly file an issue with zeitwerk to support this case somehow, if that's even technically possible.

I'll come back to you when we conclude on what to do specifically. I'm personally leaning on renaming the gem to match rubygem's recommendations.

@UmanShahzad
Copy link
Contributor

Looked into seeing what we can do here, and there's a roadblock: rubygems doesn't allow uploading ipinfo (without uppercase) unless we delete the entirety of IPinfo (uppercase) first, which is definitely not going to happen.

So we'd be forced to have a new gem named ipinfoio or something similar, but this just means all users would have to fix all their imports going forward.

So for your fix @cecilian, I think you'll have to stick to the manual import in application.rb, and/or file an issue with zeitwerk to work with this properly.

For @TheJohnzo, as the rubygems docs mention, you should always just import with lowercase letters, regardless of what the gem name actually is on rubygems.

Unless there are any objections, I'll close this issue soon.

@cecilian
Copy link
Author

cecilian commented Mar 6, 2021

Hello @UmanShahzad , thank you for looking into this.

So zeitwerk is the default code loader for Rails 6 upwards. Since the gem loading was working prior to its introduction, I think it is the case that the loading process was made stricter in zeitwerk. For this reason I think it's unlikely the rails/zeitwerk maintainers will be inclined to make an allowance in this case, also because the gem naming didn't follow conventions in the first place.

I can live with my manual require, it's not a problem, but for the reason stated above (loading with Zeitwerk will be the default for rails going forward) I would recommend that you address the issue (my guess is a breaking change is necessary, but after all we have semver for that).

I didn't investigate it but it's possible that the problem is not the gem name itself but rather the naming of the module.

In any case no problem for me about closing this issue.

@UmanShahzad
Copy link
Contributor

@cecilian I looked more into Rails6+Zeitwerk specifically and it's likely a module-name issue and not a gem name issue: https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#customizing-inflections

Zeitwerk expects the imported constants to be a camellized version of the file name, so e.g. asdf_b == AsdfB, or ipinfo == Ipinfo. We use IPinfo so this doesn't work.

irb(main):001:0> "ipinfo".camelize()
=> "Ipinfo"

After adding this to config/initializers/inflections.rb:

ActiveSupport::Inflector.inflections(:en) do |inflect|
  inflect.acronym 'IPinfo'
end

The camelize function works for IPinfo:

irb(main):001:0> "ipinfo".camelize
=> "IPinfo"

I'm not 100% certain this will fix your issue, but your fix is certainly much simpler than whatever else we'd have to do.

@juliandicks
Copy link

My quick workaround

  if RUBY_PLATFORM =~ /linux/
    require 'ipinfo'
  else
    require 'IPinfo'
  end

@cecilian
Copy link
Author

cecilian commented Mar 9, 2021

Hey there, I just wanted to let you know that I tried adding the inflection as you suggested but it doesn't work, i.e. the gem is still working fine on OSX but not working on linux.

The only thing that's making it work is the optional manual require (mine or the one suggested by @juliandicks ).

If you don't intend to look into an official fix I think it would be better to add a line to the Readme detailing what has to be done to have the gem working under linux with Rails >=6

@UmanShahzad
Copy link
Contributor

Sure @cecilian , made 161c84c.

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

No branches or pull requests

4 participants