-
Notifications
You must be signed in to change notification settings - Fork 517
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
Prevent plugin unloading from killing currently running command (soft-purge in-compile) #1686
Conversation
@eproxus can you check that this fixes your thing? |
@ferd Sorry, same issue:
|
Alright, I'll need to dig some more. I'm getting pretty frustrated with the compiler's reliance on global environment and paths always being exactly how they need to be with no way to specify anything literal, this stuff is getting tricky to manage. I'll have to dig again when I have a bit more time. |
Not sure I follow, could you go into a bit more detail of what the problem is? Perhaps we can help... |
So lately, there's been an issue about compiler paths and plugins. Because the Erlang compiler uses the paths currently loaded in the virtual machine to know what to compile with (include directories, versions of modules being called, etc.). Since plugins are for rebar3 and deps are for the apps, we take care to unload all plugins before calling the compiler so that only the deps are in path. This ensures that, for example, no app accidentally depends on a plugin, or that no plugin overrides a dep's version. It's part of ensuring repeatable builds. However, we had a bug report that some plugins are required during compilation: custom resource handlers (i.e. to download from places that are not git or hg or hex). Those can specify semver values and will need to be called during a final phase of compilation, when generating the .app.src file. So what we did was a series of patches:
But now things still fail anyway. At the very least, it's no longer the same error as before (you used to have an 'undef' error, meaning the plugin was not in the path). Now from the latest dump, we do get an error about So uh, there's something in there that needs a fixing. I'm not quite sure what. There's a good chance that path duplication now takes place in another area than the last one I fixed and that causes the multi-reload, it's probably just a question of hunting it down again. |
Thank you for the lengthy explanation, I hadn't really considered the strange situation with plug-ins and their dependencies affecting compilation of projects with potentially the same dependencies. I will try to help you as much as I can to trouble shoot this issue. You should also be able to replicate it locally by using our plug-in but unfortunately it requires a few complex tasks to be completed first (building Erlang/OTP for GRiSP among other things). Let us know if you have more fixes to try out. |
The good news it appears to have nothing to do with the global plugin. The bad news is that it certainly does kill things, and appears to take place exactly at that spot: https://github.com/erlang/rebar3/pull/1668/files#diff-bb12cff1e7d12d5adbd347f4c98d9f5fR148 |
Oh oh. Found it I think. We have successfully painted ourselves in a corner with plugins and paths. Almost. Here goes: when we unload the paths, we do so while being in the command that actually runs the compile job; since the command is part of a plugin, killing it kills rebar3. Woops. I've just added a patch that could take care of it. Incidentally I don't think the global plugins' paths are required anymore so I'll take that out too. I've also added a regression suite for this. |
Prevents the killing of a plugin with itself
d6d5c9a
to
d45bacb
Compare
I am sorry to report it is still not working, but I suspect it is almost there 😄 The unload code needs to purge properly first perhaps?
|
Ah yeah it gives that warning when it can't delete because the purge was soft. I couldn't get to the second kill because I don't have a project that grisp can fully build on my end. It may need to only do soft purges in that one, but I'm not sure how great that would be for repeatability. I god damn hate that global state. |
Here is the implementation of that task: https://github.com/grisp/rebar3_grisp/blob/master/src/rebar3_grisp_deploy.erl#L49-L69 In If there is a better way to run another task as a dependency with custom configuration, I'm all ears. |
Rather than the caller having to think of what to purge or not, use erlang:check_process_code/2 to detect if the caller (rebar3) may die because of the operation. If so, do a soft purge with a conditional delete instead of a hard purge with a mandatory delete.
I've just pushed a patch that changes the approach. I was reading the stdlib doc and found about Whenever a specific module is being held, we run a soft purge with a conditional code delete, rather than a full purge. This should eliminate most problems. I've added a debug message to figure out if it ever causes other issues. |
Really making me hate having all the stuff depend on global paths in the
VM.
Issue at #1650 had reopened. Hopefully this fixes it.