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

ActionText install command goes into recursive loop #39128

Closed
abhaynikam opened this issue May 3, 2020 · 0 comments · Fixed by #39137
Closed

ActionText install command goes into recursive loop #39128

abhaynikam opened this issue May 3, 2020 · 0 comments · Fixed by #39137

Comments

@abhaynikam
Copy link
Contributor

abhaynikam commented May 3, 2020

Steps to reproduce

I was working on my PR: #37824 where I noticed that ActionText install command goes into a recursive loop.

@jonathanhefner I can see an issue with the changes added in the commit: f6f5163 and PR: #37516

When inline: true is added to the rails_command the command is executed recursively and doesn't break. It invokes the generator again and goes into a loop.

To reproduce this issue, please either remove capture: true option from here:

and run

bundle exec rails new latest-master --dev
cd latest-master
bundle exec rails action_text:install

To my curiosity, I noticed, webpack:install command also has inline: true yet it doesn't fail. I debugged it and found that webpack:install task comes from webpacker and adding inline: true option is never accepted nor affect in any way. We can remove it.

Similarly, for active_storage:update here(https://github.com/rails/rails/blob/master/railties/lib/rails/generators/rails/app/app_generator.rb#L319) is marked as remove_task so it should not be executed for Rails 6 applications. This can be also reverted.

Please do let me know if I can help in any way.

FYI, removing inline: true makes everything work as it was previously but fails the intend of addition of that option I guess.

Expected behavior

The task should be completed and should not go in a recursive loop

Actual behavior

The task goes in a recursive loop

System configuration

Rails version: master

Ruby version: 2.6.3

jonathanhefner added a commit to jonathanhefner/rails that referenced this issue May 3, 2020
Rake stores the current top-level task and arguments in global state.
Invoking another top-level task within the same process requires
overwriting this state.  Doing so indiscriminately can cause incorrect
behavior, such as infinitely repeating the original task.  In
particular, this is a problem when running one task from another via
`rails_command "...", inline: true`.

The solution is to save and restore the global state in each call to
`RakeCommand.perform` using the `Rake.with_application` method.

Fixes rails#39128.
jonathanhefner added a commit to jonathanhefner/rails that referenced this issue May 3, 2020
Rake stores the current top-level task and arguments in global state.
Invoking another top-level task within the same process requires
overwriting this state.  Doing so indiscriminately can cause incorrect
behavior, such as infinitely repeating the original task.  In
particular, this is a problem when running one task from another via
`rails_command "...", inline: true`.

The solution is to save and restore the global state in each call to
`RakeCommand.perform` using the `Rake.with_application` method.

Fixes rails#39128.
jonathanhefner added a commit to jonathanhefner/rails that referenced this issue May 4, 2020
Rake stores the current top-level task and arguments in global state.
Invoking another top-level task within the same process requires
overwriting this state.  Doing so indiscriminately can cause incorrect
behavior, such as infinitely repeating the original task.  In
particular, this is a problem when running one task from another via
`rails_command "...", inline: true`.

The solution is to save and restore the global state in each call to
`RakeCommand.perform` using the `Rake.with_application` method.

Fixes rails#39128.
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

Successfully merging a pull request may close this issue.

1 participant