-
Notifications
You must be signed in to change notification settings - Fork 521
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
project_plugins are not downloaded/compiled in the order how they are defined in rebar.config #1857
Comments
As I see project_plugins is already in this wrong order when rebar_plugins:project_plugins_install/1 reads it from the State. |
If you need plugin2 to be downloaded and compiled before plugin1, you probably need plugin1 to depend on plugin2. |
I have one plugin (let's call it first_plug) which must be executed before all other plugins. We have like 20 plugins. About 10 of them are created by us, the other 10 is created by third parties. I can add first_plug as a dependency to the plugins created by us, but what about third party plugins? For example there is a rebar3_lint plugin on hexpm, should I create a pull request into that plugin? |
Well you're talking about compile order there or running order? Compile order, if you need to compile a plugin before another one I expect that this is related to something like include files or macros. If it's a runtime then the download/compile order is unrelated and dictated by the order of declaration in hooks for example. So I'm not quite sure what the actual issue is here? |
It's actually both, but in this case I'm referring to the execution order of the init functions. https://github.com/tothlac/rebar_plugin_orders/blob/master/rebar.config In rebar.config I have:
I want the init function of rebar_plugin2 to be executed first. Unfortunately when I run it I can see:
This is because rebar_state:get(State, {project_plugins, Profile}, []) in rebar_plugins:project_plugins_install/1 gives back this:
So it looks like when rebar.config is read, and the contents are copied to the State, the order is modified. |
So two bits of info. First, the values are initialized ASAP because plugins for custom resources are needed right now and then (since they can be used to download dependencies). Second, the alphabetical order is from the way dependencies are fetched as well (conflict resolution for two equal-level dependencies is just using the names for ordering as a tie-breaker to be repeatable). There is no ordering in the config file that is carrying any meaning right now. I don't think we ever anticipated the dependencies you mention for the ordering there, and I think internally we had to special-case In short, we don't have a solution for this yet. I don't know if declaration order is a thing that really makes sense since it would differ from dependency order. For example, if you have transient deps like:
From the declaration-order mechanism, anyone using multiple plugins would need to be aware of each of their dependencies and manually control their initialization mechanism, which is really tricky to get right. Can you give specifics about why you need init order respected so that it can inform any decision we make? |
In this case we can't modify the execution of init functions with dependencies between plugins. The reason why we want this: I have mentioned earlier we had our modification in rebar's code which mirrors repositories based on some configuration. I'm working currently on moving these modifications into a rebar plugin. We want to mirror not only dependencies but the plugins as well, and dependencies of the plugins.
And we want to mirror these dependencies as well. So what we want is by the time rebar3 fetches appup_plugin, it should have already called the init function of rebar3_mirroring. There must be a way to force the execution order. Of course alphabetical order would be also ok for us, then I need to modify the name of the mirroring plugin which precedes probably all other plugins. Something like : _rebar3_mirroring. That would be always executed first, would not it? What do you think would it work? |
The reason right now why we call the init function right away is that a resource plugin might be used to fetch a dep, so we can't take the time to reorder them. The reason for alphabetical order is unrelated; it's just to ensure deterministic builds (specifically making sure we don't rely on an implicit declaration order) when dealing with sub-dependencies. I agree that currently the ordering -- or lack of ability to specify plugin dependencies or ordering is problematic. I think the way to fix it right now would be through the applications tuple (as mentioned in #1858) so that if you specify 4-5 plugins, we could use these tuples to specify the order for them. It would however possibly break custom resource handlers. So that's a problem that we have to think about. I'm starting to think we should have a different mechanism for plugins and custom resources (which could maybe let us lock plugins eventually). @tsloughter may have ideas as well here. For mirroring, see #1777 -- we have plans to overhaul the hex mechanism to help with this, but obviously no timeline for now. |
What about the other question? A built-in mechanism in rebar3 would be good. We need mirroring the hex pkgs and a bit more. We mirror git repos to our atlassian/bitbucket servers, automatically create them when encountering a new repo which has not been mirrored, then during an rebar3 upgrade command fetch the new commits , automatically rebase/merge them (depending on the upgrade strategy specified in the config) against our internal modifications, and create a pull request if the automatic merge/rebase was not successful because of a conflict. This can be useful in a big as sometimes github does not work for an entire day, so the programmers can't do anything on that day. We are planning to opensource the plugin, unfortunately it is not easy. |
I think that yes, it would force it to be first. That should work, but only as long as we don't come up with a better mechanism, where then it might break since it relied on a build behaviour that could change. But that's probably acceptable to you since otherwise it won't build today. And yeah so far the mirroring we'd look at would only be with hex; for git there's no great plan there. |
I've tried out this solution to move the plugin which alphabetically precedes everything else. It worked actually. So you don't plan to change the order of the execution of the plugin init functions? |
I mean, we don't have active plans as in "features that require it" in mind. I don't think we'd be closed to the idea, but that's a distinction between "what is in the roadmap" and "what we are not going to do". I think right now the plugin priority was not in the roadmap, but I'm also not opposed to having a better mechanism for the ordering of plugins. In practice what this means is: I'm not sure either @tsloughter or myself would find the time to do this in our free time considering other things in the works (i.e. updating the hex API and dep handling). However, we'd probably be fine brainstorming ideas and guiding implementations if you (or anyone else) wishes to join the project and spend time on it. |
Sure, I would be also interested in it. In the meantime I've created a small pr which accidentally fixed all the issues I was struggling with excerpt the plugin ordering, but that's fine right now since alphabetical ordering worked fine. The build job on travis also looks fine. There was only one problem with it: #1859 |
Unfortunately it looks like even after the tests were not failing, and it solved the current problems there are still problems with this modification in even more complex cases. |
Do you currently have any ideas how to fix it? |
Hm, re-looking at the patch, the reason why you can't take the plugins out of the codepath they were is that dependency-handling plugins (custom resources) must be around during the compile phase for the .app.src file generation and version locking, which can be custom-made. However I don't think there's a big negative to having the plugin path available earlier. Would your patch work if you added the path in both areas? I guess it would still remove the plugin from the path and that could be a bit of a problem? regarding the new hex version, the branch is already usable (for testing and experimenting purposes). There is not a lot of doc, but if you pop into either the #rebar IRC channel on freenode or the #rebar3 channel on slack, Tristan can probably offer a bit of guidance on it. The expected mechanism is to specify multiple indexes that will be checked in order for each dependency. There's some fancy logic with lock files to ensure only the right versions of some libs are fetched. There will be no automatic publication from one registry to the next one as part of fetching dependencies, simply because what people might want is not just mirrors, but also "overlay" repositories (i.e. I want to use my 5 corporate libraries from our private index with public libraries from hex). Auto-publishing into private environments could be super problematic for some setups. I would probably instead create a kind of 'sync' job whose purpose was to read the remote index and simply mirror it. |
I was finally able to add some changes to the code and with these changes everything works well. It is not a big modification, I just had to call |
Say for example that you run a plugin that loads dependency B. Dependency B is on version 1.4.0 and has its own .hrl file. You own application also has a dependency on B, but it specified B at version 1.5.0, which has different .hrl files that can be loaded in your app. Since the compiler loads the .hrl files in include_lib based on currently loaded paths, the plugin will prevent the application from compiling against the proper dependency set. That's the reason why, just around the compiling phase, we unload the plugins. It ensures that the build will be repeatable based on what is in the lock file and not based on current plugins (which might be globally installed and not project-specific). The same problem exists with parse_transforms as well, with a dependency such as lager with lager_transform. |
I believe this was fixed with all the code-handling patches in 3.7.0? Please reopen if this is still a problem |
With version: rebar 3.6.1+build.4118.ref940b1c59 on Erlang/OTP 20 Erts 9.3
Having this in my rebar.config:
I want plugin2 to be downloaded and compiled first. However, it looks like they are fetched in an alphabetical order. It's actually not a problem if there is a way to force rebar3 to download plugin2 first.
The text was updated successfully, but these errors were encountered: