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

address issue when running commands with #exec #189

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

blerchin
Copy link
Contributor

@blerchin blerchin commented Aug 9, 2022

This is a followup to #181

After upgrading one of our repos to 2.0.11, we unfortunately learned that running commands with Kernel#exec instead of Kernel#system introduces a failure in the assets:precompile pipeline. Due to exec hijacking the process from Ruby, subsequent steps will not run after the tailwind build completes.

This will probably need to be treated as an urgent bugfix as it may result in a failure of the asset pipeline to output images, javascript, etc.

This change effectively reverts #181, and adds the exception: true flag which will allow #system to raise when encountering failure codes.

@masterkain
Copy link

just got hit by this

@philipithomas
Copy link

Also affected by this 👍🏻

end

desc "Watch and build your Tailwind CSS on file changes"
task :watch do
exec "#{TAILWIND_COMPILE_COMMAND} -w"
system "#{TAILWIND_COMPILE_COMMAND} -w"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason exception: true is not used here? Even though the command is not expected to terminate, it can still fail to start in the first place.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason other than it hasn't been a problem we needed to solve. If you'd like to submit a PR I'd consider merging it.

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 this pull request may close these issues.

6 participants