-
Notifications
You must be signed in to change notification settings - Fork 16
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
fix: rewrite Subcommand.run using Kernel.spawn #109
base: master
Are you sure you want to change the base?
Conversation
The current implementation was doing fork/exec by hand because it had been created under ruby 1.8, which did not have spawn. It makes zero sense nowadays, where there exists a multiplatform implementation in the language itself.
e9cd697
to
4197e56
Compare
This error started appearing in a new version of rubocop. It is auto-correctable, so auto-correct it
4197e56
to
b713882
Compare
Looks good and a very welcome change indeed. I'm curious though, what motivated this? |
I'm having freezes during updates in CI in the ARM docker images (running qemu-arm-static). This is new, and it seems the system gets stuck in some of the pipe handling. I thought it might help (I'm kind of desperate - but I start to believe the problem is more emulation-related). Did not solve the issue, and I honestly still don't know what happens. Thought it would be a pity to throw a nice cleanup commit like that, so there it is. |
@doudou we also see some freezes in CI with qemu->docker->arm, but this is (when I remember correctly) during compilation of ROS packages using catkin. We use autoproj to organize the workspace though (but not for the build). I think that hints to an issue with the emulation rather than autobuild, as it is not used by autoproj when not buidling, right? |
It actually is during import. But I more and more think it is an issue with QEmu indeed. |
I meant for us it is mostly compilation with catkin. |
The current implementation was doing fork/exec by hand because it
had been created under ruby 1.8, which did not have spawn. It makes
zero sense nowadays, where there exists a multiplatform implementation
in the language itself.