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

leiningen command-line update-in has lower priority than ~/profiles.clj #1969

Closed
laurentpetit opened this issue Aug 28, 2015 · 10 comments
Closed

Comments

@laurentpetit
Copy link
Contributor

CCW, when launching is project server REPL, is forcing a project's JVM to start in debug mode by adding this to the leiningen command line: lein .... update-in :jvm-opts concat ["-Xdebug" "-Xrunjdwp:transport=dt_socket,server=y,suspend=n"] -- repl.

Many people seem to have {:user {:jvm-opts ^:replace []}} in their profiles.clj. When it is so, then the update-in ... command-line stuff is silently ignored as can be seen by inspecting the project's process command line created by Leiningen.

So, a whole category of people are currently unable to launch their project with a REPL in Debugging mode.

Is it normal that what is declared on the command line does not take precedence over what is declared in ~/.lein/profiles.clj(or even ./project.clj - not tested`) ? I would say it's not normal.

What do you think? Bug? Or should CCW be doing things differently to force the insertion of the remote JVM debug options to the project process through invoking the leiningen command?

@technomancy
Copy link
Owner

technomancy commented Aug 28, 2015 via email

@laurentpetit
Copy link
Contributor Author

@technomancy thanks for the quick qualif!
Do you thing this is a low hanging fruit I could try to provide a PR for, or is it a delicate part of the code that should better be handled by leiningen internal experts? If low hanging fruit, can you provide some starter point?

@technomancy
Copy link
Owner

How easy this is depends on whether it's a bug in the update-in task or deeper in the merge logic. My hope is that it's a bug in the task which should be fixed by a fairly straightforward patch there; if it's deeper inside leiningen-core then it'll take some more digging to uncover.

@hypirion
Copy link
Collaborator

hypirion commented Sep 8, 2015

As a workaround and FYI, {:user {:jvm-opts []}} will replace the default :jvm-opts with [] and should work fine with the CLI call you present. I guess many people use {:user {:jvm-opts ^:replace []}} to ensure that default optimisations are set, but the ^:replace part is not needed anymore.

@hypirion
Copy link
Collaborator

hypirion commented Sep 8, 2015

Also, I've looked into this due to #1981, and it seems that the problem at hand here is quite convoluted because I'm not entirely sure whether this is expected functionality or not.

Many tasks merge in extra profiles when they need or where it's expected – perhaps the best known example is uberjar, but repl also does this. In a sense, you can consider those tasks of expanding into something equivalent of lein with-profile +some,profiles actual-task.

So if we have the following profiles.clj fil (or as a profile in project.clj)

{:a {:jvm-opts ["foo" "bar"]}}

Then what would you expect the :jvm-opts to be if you performed lein update-in :jvm-opts concat '["baz"]' -- lein with-profile +a mytask?

It's entirely possible to make update-in changes to the project map unaffected by merge logic (i.e. they always win in a merge), but this may have unintended and confusing consequences down the line. I think that would cause more harm than good in some cases.

For example, if the changes were immutable and you want to see how a project works with the latest clojure version, then lein update-in :dependencies assoc 0 '[org.clojure/clojure "1.8.0-alpha4"]' -- uberjar would drag in all your dev-dependencies into your uberjar as well.

@hypirion hypirion added this to the 3.0.0 milestone Nov 20, 2015
@hypirion
Copy link
Collaborator

I've thought about this and can see a sensible way out of this. However, that particular solution won't be available before a preview of 3.0.0 lands. As such, I've placed this issue in the 3.0.0 milestone.

If there's an immediate need for update-in to be prioritised over everything else, I can add in functionality for that in 2.x, but for now I'd like to punt it.

@benedekfazekas
Copy link

clojure-emacs/cider#1824 may be related to this one? It seems that whatever added with update-in does not "see" transient dependencies. Perhaps there is a weird order of adding deps issue here?!

If I switch off the cider autoinjecting dependencies and define the necessary dependencies in the .lein/profiles.clj file cider repl starts up fine.

Wonder if throwing #2074 into the mix is appropriate here? What I mean is it seems that it would be nice to have a command line way of adding deps/plugins overriding anything but seeing the whole classpath (incl transient deps as well)

@hypirion
Copy link
Collaborator

@benedekfazekas: Do you mean transitive dependencies? I'd be surprised if that would be an issue related to update-in. Seems more likely that this is related to the org.clojure/clojure dependency – I'll have a look on the issue later when I get time.

@benedekfazekas
Copy link

yes, sorry, i meant transitive deps of course. thanks @hypirion

@technomancy
Copy link
Owner

At this point I honestly think this is unlikely to ever happen; the disruption caused by this change is greater than the problem it would fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants