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

Lein 2.8.2 warns about deprecated hook lein-environ.plugin/hooks #88

Open
a613 opened this issue Dec 13, 2018 · 12 comments
Open

Lein 2.8.2 warns about deprecated hook lein-environ.plugin/hooks #88

a613 opened this issue Dec 13, 2018 · 12 comments

Comments

@a613
Copy link

a613 commented Dec 13, 2018

After upgrading from lein 2.8.1 to 2.8.2:

$ lein --version
Warning: implicit hook found: lein-environ.plugin/hooks
Hooks are deprecated and will be removed in a future version.
Leiningen 2.8.2 on Java 1.8.0_151 Java HotSpot(TM) 64-Bit Server VM
@extremus
Copy link

I had the same warning with Leiningen 2.8.3 on Java 10.0.2. I've changed the :plugin section to [ lein-environ "1.1.0" :hooks false ] and it's now ok in my project.

@a613
Copy link
Author

a613 commented Dec 17, 2018

@extremus thanks, that works fine in my case too.

@terjedahl
Copy link

Yes, adding :hooks false does get rid of the warning, but doesn't it also then disable writing :env values in project.clj and profiles.clj from being written to .lein-env; thereby disabling the most useful feature of the plugin?
Can the plugin be re-written in such a way that it no longer relies on hooks?

@darwin
Copy link

darwin commented Dec 24, 2018

@terjedahl is correct. Disabling hooks will stop writing .lein-env. It seems to work just because the old file stays in place until you start from scratch (e.g. clone the repo somewhere else).

The whole idea of writing to a file which is then consumed by the library is quite brittle. For example I had a lot of headaches when using this with lein-shell plugin. Imagine running a shell script from leiningen which then happens to call some other lein command (you end up with nested lein processes). This way only inner-most process wins writing the file and others get wrong environments depending on who was last writer to the file when they happen to read it.

You have to understand implementation details of this plugin to see what went wrong. I would recommend opening a PR in leiningen itself and implement independent support for ENV variables there. Dynamic .lein-env is not a good idea.

@weavejester
Copy link
Owner

If anyone can come up with a better implementation for passing the environment data from the Leiningen project to the application process, I'd welcome a PR.

@terjedahl
Copy link

One workaround could be to replace the hook-mechanism with a middleware mechanism.
I understand that this would be conceptually wrong, as middleware shouldn't have side-effects. We wouldn't be actually be altering the project-structure either. And also, as implicit middleware is going away, the user would still have to declare it explicitly as :middleware.

Another solution might be to simply have it hook in via the :prep-tasks mechanism. This could possibly even alleviate @darwin 's issue as it could be added selectively in profiles.

@wesleyhall
Copy link

This is admittedly an idle thought, but figwheel seems to be able to pull information out of the project.clj without the need for the hooks mechanism. I wonder if a similar approach can be adopted here. Unless anybody tells me i'm crazy, i'll keep the warning in my project as a reminder to take a look as soon as I can.

@terjedahl
Copy link

I was thinking along similar lines as you @wesleyhall: As environ.core/env is a result of calling environ.core/read-env, which in turn amongst other reads from file '.lein-env', why not skip the intermediate step of generating the '.lein-env' file and instead read directly from the 'project.clj'. The challenge might be in detecting/applying profiles, though ... :-/

@wesleyhall
Copy link

@terjedahl

I did take a very quick look into this last night and Bruce @ Figwheel actually has this little project:

https://github.com/bhauman/simple-lein-profile-merge

He doesn't seem particularly enthusiastic about it being ready for wider adoption, but it is out there.

@kengruven
Copy link

I just spent a couple days learning way more than I ever wanted to about lein and environ, because I thought I was helping myself by silencing the "Warning: implicit hook found" message. Oops.

In the short term, how do you feel about a note in the README that warns idiots like me not to use :hooks false in their project.clj, even if they see red warnings in their logs?

@radhikalism
Copy link

There seems to be two issues under discussion here. One is about avoiding the Leiningen implicit hooks warning (but keeping the brittle .lein-env file strategy). The other is about finding a different way to communicate project maps to the project runtime.

On the first question of implicit hooks alone:

  • Is there any reason lein-environ cannot use :prep-tasks to explicitly generate its .lein-env file as a task, rather than hooks?
  • Or, is there any reason lein-environ cannot use an explicit hook instead of an implicit hook, at least, to avoid this warning?

@rome-user
Copy link

is there any reason lein-environ cannot use an explicit hook instead of an implicit hook, at least, to avoid this warning?

Hooks in general are scheduled to be removed in a future release of Leiningen. Anything that keeps using hooks is NG. This leaves us with the :prep-tasks question. I assume the problem with prep task is the inability of plugins to add their own prep task. The user may have to configure it manually.

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

No branches or pull requests

9 participants