Support gemification of `pathname` #4992

deivid-rodriguez · 2021-10-19T09:54:50Z

What was the end-user or developer problem that led to this PR?

The pathname library has been gemified, so users should be able to specify it inside their Gemfile's. However, bundler uses the pathname library internally for a lot of things. This causes the pathname gem to be activated too early, possibly conflicting with user's choice of the gem.

What is your fix for the problem, implemented in this PR?

My fix is:

Remove pathname usages when possible.
Vendor the pathname gem, together with a pure ruby implementation of its C extension.

The one concern I have here is that now some widely used methods, such as Bundler.root will return a Bundler::Pathname, not a Pathname. If people are using normal typing on the result, rather than duck typing, things will break since the result is an object of a different class now. If we can rely on people not doing this, I can fully implement Pathname's interface to avoid any other breakage (right now I have only implemented the subset of functionality that bundler uses internally).

Fixes #5016.
Fixes ruby/pathname#12.

Make sure the following tasks are checked

Describe the problem / feature
Write tests for features and bug fixes
Write code to solve the problem
Make sure you follow the current code style and write meaningful commit messages without tags

doodzik · 2021-10-22T16:47:49Z

bundler/spec/support/subprocess.rb

+        command_execution.exitstatus = if status.exited?
+          status.exitstatus
+        elsif status.signaled?
+          128 + status.termsig


\nit 128 is a magic number. I think it would add to maintainability if the number would live in a constant.

Sure, that can be improved, we should probably add a link to https://tldp.org/LDP/abs/html/exitcodes.html and move it to a constant/variable with some non magical name. But it's unrelated to this PR, here I'm just moving this code around, the magic number is already present in the current master branch.

doodzik · 2021-10-22T17:03:32Z

The one concern I have here is that now some widely used methods, such as Bundler.root will return a Bundler::Pathname, not a Pathname. If people are using normal typing on the result, rather than duck typing, things will break since the result is an object of a different class now.

I think it could be confusing for bundler's users if Bundler::Pathname isn't behaving like ruby's Pathname. I wonder if it would make more sense to return a string instead of exposing the internal implementation of Bundler::Pathname, which isn't subject to as much change. A user can still pass the string to their choice of Pathname implementation.

deivid-rodriguez · 2021-10-25T10:39:16Z

Bundler::Pathname should be behaving just like Pathname. Right now the interface is not yet complete because I only started by implementing the minimal subset of functionality used internally, but if we want to improve backwards compatibility of this PR, the whole thing should be reimplemented.

This way users should be able to use the whole set of Pathname methods on the result of Bundler.root. The problem is if users are using things like

root = Bundler.root

# stuff

if root.is_a?(Pathname)
  # stuff
end

or things like that, since that will change after this PR.

I think that should be weird enough to ignore it?

Another possibility could be to define some set of public bundler methods, and keep returning Pathname's for them, while defining equivalent ones that use Bundler::Pathname (like, Bundler.internal_root or whatever), and make bundler/setup call the internal ones.

But I tend to think the current approach is fine.

doodzik · 2021-10-26T04:55:17Z

I think that should be weird enough to ignore it?

Yeah, the current approach seems fine to me. The potential problem seems very niche and pretty easy to resolve for the developer. Not sure why someone would need to check if Bundler.root is a Pathname anyways.

andrewfader · 2021-11-16T00:11:44Z

Got a TypeError: unexpected @path /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/bootsnap-1.9.1/lib/bootsnap/load_path_cache/path.rb:103:in `to_s' on rails new with ruby 3.1-rc2 and rails 7-alpha

doodzik · 2021-11-16T03:39:51Z

@andrewfader Interesting error. There is an issue in bootsnap that suggests that the error is caused by multiple gem activation of pathname. Would you be able to test if this PR fixes your issue?

andrewfader · 2021-11-16T04:08:35Z

@doodzik I believe so, I ran setup.rb from this branch and then I was able to do it without error.

andrewfader · 2021-11-16T04:13:22Z

Actually it may still be broken. Here's the new error /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:49:in `basename': no implicit conversion of nil into String (TypeError) from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:49:in `chop_basename' from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:374:in `plus' from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:354:in `+' from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:417:in `block in join' from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:415:in `reverse_each' from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:415:in `join'

deivid-rodriguez · 2021-11-16T07:47:30Z

Is that all? It would seem to me that backtrace is missing some entries.

andrewfader · 2021-11-16T17:07:36Z

Here you go. Sorry about that:


❯ DISABLE_BOOTSNAP=1 rails s > foo.txt
/home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:20: warning: already initialized constant Pathname::TO_PATH
/home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/pathname-0.2.0/lib/pathname.rb:20: warning: previous definition of TO_PATH was here
/home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:22: warning: already initialized constant Pathname::SAME_PATHS
/home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/pathname-0.2.0/lib/pathname.rb:22: warning: previous definition of SAME_PATHS was here
/home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:34: warning: already initialized constant Pathname::SEPARATOR_LIST
/home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/pathname-0.2.0/lib/pathname.rb:34: warning: previous definition of SEPARATOR_LIST was here
/home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:35: warning: already initialized constant Pathname::SEPARATOR_PAT
/home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/pathname-0.2.0/lib/pathname.rb:35: warning: previous definition of SEPARATOR_PAT was here
/home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:41: warning: already initialized constant Pathname::ABSOLUTE_PATH
/home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/pathname-0.2.0/lib/pathname.rb:41: warning: previous definition of ABSOLUTE_PATH was here
/home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:49:in `basename': no implicit conversion of nil into String (TypeError)
	from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:49:in `chop_basename'
	from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:374:in `plus'
	from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:354:in `+'
	from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:417:in `block in join'
	from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:415:in `reverse_each'
	from /home/andrew/.rvm/rubies/ruby-3.1.0-preview1/lib/ruby/3.1.0/pathname.rb:415:in `join'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/application/configuration.rb:445:in `credentials_available_for_current_env?'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/application/configuration.rb:429:in `default_credentials_content_path'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/application/configuration.rb:71:in `initialize'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/application.rb:381:in `new'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/application.rb:381:in `config'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/railtie.rb:146:in `config'
	from /home/andrew/workspace/trident/config/application.rb:12:in `<class:Application>'
	from /home/andrew/workspace/trident/config/application.rb:11:in `<module:Trident>'
	from /home/andrew/workspace/trident/config/application.rb:9:in `<top (required)>'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/zeitwerk-2.5.1/lib/zeitwerk/kernel.rb:35:in `require'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/zeitwerk-2.5.1/lib/zeitwerk/kernel.rb:35:in `require'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/commands/server/server_command.rb:137:in `block in perform'
	from <internal:kernel>:90:in `tap'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/commands/server/server_command.rb:134:in `perform'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/thor-1.1.0/lib/thor/command.rb:27:in `run'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/thor-1.1.0/lib/thor/invocation.rb:127:in `invoke_command'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/thor-1.1.0/lib/thor.rb:392:in `dispatch'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/command/base.rb:87:in `perform'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/command.rb:48:in `invoke'
	from /home/andrew/.rvm/gems/ruby-3.1.0-preview1/gems/railties-7.0.0.alpha2/lib/rails/commands.rb:18:in `<top (required)>'
	from bin/rails:4:in `require'
	from bin/rails:4:in `<main>'

deivid-rodriguez · 2021-11-17T08:12:50Z

What are the contents of bin/rails?

andrewfader · 2021-11-17T15:47:19Z

@deivid-rodriguez I deleted my whole project and did rails new again and now rails s works fine. So I guess my issue is now solved. But here was bin/rails if it's at all helpful.

#!/usr/bin/env ruby
APP_PATH = File.expand_path("../config/application", __dir__)
require_relative "../config/boot"
require "rails/commands"

deivid-rodriguez · 2021-11-17T15:52:32Z

Mmm, alright. I was blind guessing that maybe your rails binstub was loading Pathname before bundler/setup is run. But no. Bundler generated binstubs were actually doing that before this PR so users using bundler generated binstubs will also need to regenerate them (not only to upgrade bundler) in order to get rid of that.

andrewfader · 2021-11-17T22:04:10Z

I'm guessing I didn't completely eradicate something - or something to do with rvm or bootsnap/spring running in the background

Not that I need it, but reads better.

When launching bundler subprocesses for end to end testing, all of them will load the `spec/support/rubygems_version_manager.rb` file passed as a ruby's `-r` flag. Unfortunately this file depends on `pathname`, so unless we drop that dependency, we can't really test support for including the `pathname` gem in the `Gemfile`. This commit implements some refactorings to avoid loading `pathname` inside `bundler` test subprocesses.

`Pathname#sub` returns a new `Pathname` already.

We can use strings earlier on.

For backwards compatibility.

deivid-rodriguez · 2021-11-18T20:36:19Z

Alright I implemented the whole Pathname API in ruby, except for Pathname#taint, Pathname#untaint, which are deprecated and seem fine to ignore, and Pathname#<=>, which explicitly makes sure Pathname objects can only be compared with Pathname objects. If the result of Bundler.root (now a Bundler::Patname) is sorted with other Pathname's, I don't think the sorting will work anyways, since Bundler.root won't be accepted as a valid element to compare to anyways.

I guess next step is to vendor the pathname test suite and make sure it passes against our pure ruby implementation.

doodzik · 2021-11-22T18:43:09Z

Looking at the linked PR in the pathname repo, pathname doesn't support rubies older than 2.7. We probably need to vendor an earlier version of the library to be used with rubygems, correct? Or would it make more sense to wait with vendoring pathname before we deprecated an older ruby version #3260.

deivid-rodriguez · 2021-11-22T18:57:04Z

That's a good point! Given that CI is fully green, my guess is that issues on old rubies are C-related (they were surfaced at gem compilation time) and that our pure ruby implementation doesn't suffer from them. But I think we'll find out for sure once I vendor pathname's test suite too, and see how our implementation behaves against it.

andrewfader · 2021-12-13T22:20:29Z

Maybe I could help or pair on this. Lmk?

deivid-rodriguez · 2021-12-14T07:33:50Z

This is only missing, I believe, to make sure that it's fully compatible with the real pathname library. For that, my idea was to import the pathname test suite, with the proper modifications to run against our ruby version, and run it as part of our CI. I guess I'll be able to come back to this in a couple of months, but in the mean time feel free to move that forward, I'm happy to help!

andrewfader · 2022-03-07T15:39:34Z

Thanks!

bundlerbot added the Bundler label Oct 19, 2021

deivid-rodriguez force-pushed the support_pathname_gem branch from e57cefa to ff77a3b Compare October 19, 2021 09:56

deivid-rodriguez mentioned this pull request Oct 19, 2021

A bunch of warnings showing up after migrating to version 0.2.0 ruby/pathname#12

Closed

deivid-rodriguez force-pushed the support_pathname_gem branch 3 times, most recently from 59c57c9 to 2ea0e8e Compare October 22, 2021 10:51

doodzik reviewed Oct 22, 2021

View reviewed changes

doodzik approved these changes Oct 22, 2021

View reviewed changes

casperisfine mentioned this pull request Nov 2, 2021

i get unexpected @path error. rails/bootsnap#374

Closed

andrewfader approved these changes Nov 17, 2021

View reviewed changes

deivid-rodriguez added 7 commits November 18, 2021 17:56

SharedHelpers.pwd doesn't need to return a Pathname

bc9ac8d

Don't activate the pathname gem from binstubs

fa9fd3b

The tmp test helper already joins internally

3b4a380

The bundled_app helper method already joins internally

a7c7b9b

The home helper method already joins internally

56607cb

The default_bundle_path helper method already joins internally

e68620d

The system_gem_path helper method already joins internally

1d108c1

deivid-rodriguez added 15 commits November 18, 2021 17:56

We should be checking raised exception, not status code here

6d7f7a0

These method should be returning a string

e0cafef

Check not having load system features also for successful runs

bfabc6c

Normalize Pathname instantiations

91eca89

Prefer Pathname#parent for simplicity

235e52e

Simplify check for ruby-core setup

6100a5d

Respect GEM_COMMAND in non ruby-core mode

1d382ad

Not that I need it, but reads better.

Reuse git helper when possible

0984a25

No need to instantiate here

3b3af0e

`Pathname#sub` returns a new `Pathname` already.

Use Pathname#sub for consistency

81828eb

Tweak assertion to give better error when it fails

4d2c059

Make rubygems source caches consistently use strings

f39c4de

Simplify unwritable files finding

668a971

We can use strings earlier on.

Support using the pathname gem in the Gemfile

d290d10

deivid-rodriguez force-pushed the support_pathname_gem branch from 2ea0e8e to d266753 Compare November 18, 2021 19:40

Backfill the rest of the Pathname API

f62cad4

For backwards compatibility.

deivid-rodriguez force-pushed the support_pathname_gem branch from d266753 to f62cad4 Compare November 18, 2021 20:20

deivid-rodriguez mentioned this pull request Nov 18, 2021

Officially drop support for ruby 2.6 or older ruby/pathname#13

Merged

deivid-rodriguez closed this Sep 24, 2025

eregon mentioned this pull request Oct 22, 2025

Bundler requires pathname eagerly and prevents using a newer version of the gem #9030

Open

Uh oh!

Support gemification of pathname #4992

Support gemification of pathname #4992

Uh oh!

Conversation

deivid-rodriguez commented Oct 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was the end-user or developer problem that led to this PR?

What is your fix for the problem, implemented in this PR?

Make sure the following tasks are checked

Uh oh!

doodzik Oct 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deivid-rodriguez Oct 25, 2021

Choose a reason for hiding this comment

Uh oh!

doodzik commented Oct 22, 2021

Uh oh!

deivid-rodriguez commented Oct 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

doodzik commented Oct 26, 2021

Uh oh!

andrewfader commented Nov 16, 2021

Uh oh!

doodzik commented Nov 16, 2021

Uh oh!

andrewfader commented Nov 16, 2021

Uh oh!

andrewfader commented Nov 16, 2021

Uh oh!

deivid-rodriguez commented Nov 16, 2021

Uh oh!

andrewfader commented Nov 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deivid-rodriguez commented Nov 17, 2021

Uh oh!

andrewfader commented Nov 17, 2021

Uh oh!

deivid-rodriguez commented Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewfader commented Nov 17, 2021

Uh oh!

deivid-rodriguez commented Nov 18, 2021

Uh oh!

doodzik commented Nov 22, 2021

Uh oh!

deivid-rodriguez commented Nov 22, 2021

Uh oh!

andrewfader commented Dec 13, 2021

Uh oh!

deivid-rodriguez commented Dec 14, 2021

Uh oh!

andrewfader commented Mar 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Support gemification of `pathname` #4992

Support gemification of `pathname` #4992

deivid-rodriguez commented Oct 19, 2021 •

edited

Loading

doodzik Oct 22, 2021 •

edited

Loading

deivid-rodriguez commented Oct 25, 2021 •

edited

Loading

andrewfader commented Nov 16, 2021 •

edited

Loading

deivid-rodriguez commented Nov 17, 2021 •

edited

Loading