Skip to content

Conversation

@rodrigovilina
Copy link
Contributor

Summary

Filter PATH environment variable so script doesn't loop on itself.

Fixes: #38666
Related: #39619

When updating a rails application to the master branch, and running rails app:update, if you have bin in your PATH you would get thrown into an infinite loop.

This excludes bin from the PATH variable, so when bin/yarn is called, it does not loop on itself when calling exec "yarn".

Related Issue: #38666
Also briefly mentioned here: #39282 (comment)

Other Information

@rails-bot rails-bot bot added the railties label Nov 19, 2020
@rodrigovilina rodrigovilina changed the title Update yarn.tt Fix rails new hanging on master Nov 19, 2020
@rodrigovilina rodrigovilina changed the title Fix rails new hanging on master Fix rails new and rails app:update hanging on master bug. Nov 19, 2020
@rafaelfranca
Copy link
Member

Can we do #39619 (comment)?

@rodrigovilina
Copy link
Contributor Author

Actually, I tried and thought it didn't work, but on second try I got it; commiting changes.

@rodrigovilina
Copy link
Contributor Author

@rafaelfranca Done 👌 .

@rodrigovilina
Copy link
Contributor Author

rodrigovilina commented Nov 20, 2020

Actually, I kept testing and I don't think it's working. If the first params to exec is both the path and the command (as in
"exec PATH='#{filtered_path}' yarn"), it thinks that that's the whole command and can't find it.

Isn't it an acceptable solution to assign the full path back once the call has been finished? I reckon that having the path that we are excluding (./bin) is neither required nor suggested to run rails, hence, it is not going to happen that that path is required afterwards.

@rafaelfranca
Copy link
Member

This is more what we had in mind:

require 'pathname'

APP_ROOT = File.expand_path('..', __dir__)
Dir.chdir(APP_ROOT) do
  executable_path = ENV["PATH"].split(File::PATH_SEPARATOR).find do |path|
    normalized_path = File.expand_path(path)

    normalized_path != __dir__ && File.executable?(Pathname.new(normalized_path).join('yarn'))
  end

  exec File.expand_path(Pathname.new(executable_path).join('yarn')), *ARGV
rescue Errno::ENOENT
  $stderr.puts "Yarn executable was not detected in the system."
  $stderr.puts "Download Yarn at https://yarnpkg.com/en/docs/install"
  exit 1
end

Can you apply those changes?

@rafaelfranca rafaelfranca added this to the 6.1.0 milestone Nov 24, 2020
@rodrigovilina
Copy link
Contributor Author

I'll test locally and apply 👌

Prevent script from looping onto itself
@rodrigovilina
Copy link
Contributor Author

@rafaelfranca done 👌.

@rafaelfranca rafaelfranca merged commit b13897f into rails:master Nov 27, 2020
rafaelfranca added a commit that referenced this pull request Nov 27, 2020
Fix `rails new` and `rails app:update` hanging on master bug.
@rodrigovilina rodrigovilina deleted the patch-1 branch November 27, 2020 19:26
schneems added a commit to codetriage/CodeTriage that referenced this pull request Nov 30, 2020
@schneems
Copy link
Member

Thank you for your work here!

tricknotes added a commit to tricknotes/rails that referenced this pull request Dec 3, 2020
When we use `bin/yarn` without original yarn,
the following error will be occurred:
```
bin/yarn:12:in `initialize': no implicit conversion of nil into String (TypeError)
```

This means `executable_path` is `nil`.

To handle missing yarn correctly, checking `executable_path` seems good.
This is a result of my local without yarn.
```
Yarn executable was not detected in the system.
Download Yarn at https://yarnpkg.com/en/docs/install
rake aborted!
```

This commit follows up rails#40646.
jonathanhefner added a commit to jonathanhefner/rails that referenced this pull request Dec 27, 2020
Since rails#40646, `bin/yarn` manually searches `PATH` for the `yarn`
executable.  In Windows environments, executables have an `.exe` file
extension, so we must search for `yarn.exe` as well.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All rails commands hang on edge Rails

3 participants