technomancy / leiningen

Moved to Codeberg; this is a convenience mirror
https://codeberg.org/leiningen/leiningen
Other
7.29k stars 1.61k forks source link

Discrepancy in pom.xml dependencies and uberjar included dependencies #2790

Open mrrodriguez opened 2 years ago

mrrodriguez commented 2 years ago

Describe the bug

I've tested this behavior across several 2.9.x versions of lein, but looking at the code involved, it doesn't look like anything has changed that would affect the behavior in quite some time.

When using with-profile to add a profile during the uberjar task, the pom.xml that is generated and placed inside the uberjar jar itself can not accurately reflect the contents of the uberjar in terms of dependencies.

The reason for this is the discrepancy between how with-profile adds the given profiles to (-> project meta :included-profiles). This is then specially included in tasks like uberjar and jar. If a profile here has :dependencies, they will be used during the building of the contents of the uberjar/jar.

However, several other parts of the uberjar/jar build explicitly remove non-leaky profiles, ie profiles not marked with the :leaky metadata. The most interesting one for the sake of this issue is the pom that is made inside the uberjar/jar. Notably the pom/make-pom fn called there removes non-leaky profiles and does not attempt to use the (-> project meta :included-profiles) which, in this case, would bring in the with-profile passed profiles.

The result is an uberjar that can build with :dependencies inside it that are not part of the pom.xml. The workaround/fix is to mark any profiles used with with-profile to build an uberjar, with :leaky metadata so the pom creation (and any other affected meta files in the jar) will not strip those profiles out.

To Reproduce Steps to reproduce the behavior:

  1. Define a simple lein project.clj - called app here:
    (defproject app "0.1.0-SNAPSHOT"
    :dependencies [[org.clojure/clojure "1.11.0"]]
    :profiles {:extras {:dependencies [[ring/ring-core "1.9.3"]]}})
  2. Run lein with-profile extras uberjar
  3. Observe the ring/ring-core class files compiled into the uberjar, but the META-INF/maven/app/app/pom.xml inside the jar does not include the ring/ring-core in the <dependencies>.
  4. Change the project.clj in step (1) with: the addition of the :leaky metadata on the :extras profile:
    (defproject app "0.1.0-SNAPSHOT"
    :dependencies [[org.clojure/clojure "1.11.0"]]
    :profiles {:extras ^:leaky {:dependencies [[ring/ring-core "1.9.3"]]}})
  5. Repeat step (2).
  6. Observe teh ring/ring-core class files are included in the uberjar now and the pom from step (3) includes it as a <dependencies> entry.

Expected behavior Ideally the behavior of with-profile with uberjar would create consistency with regards to how :leaky profiles interact with the explicit with-profile profiles. I believe that the with-profile explicitly set profiles would generally not expect to be unmerged during the build of the given task and its include sub-tasks that are used to build the final artifact.

technomancy commented 2 years ago

Ideally the behavior of with-profile with uberjar would create consistency with regards to how :leaky profiles interact with the explicit with-profile profiles. I believe that the with-profile explicitly set profiles would generally not expect to be unmerged during the build of the given task and its include sub-tasks that are used to build the final artifact.

Sorry, I don't really follow this logic. The documentation states:

If you mark your profile with ^:leaky metadata, then the profile will not be stripped out when the pom and jar files are created.

The point of the leaky metadata is that you can get the behavior you describe, if you want it. This seems consistent with the observed behavior. If the default behavior worked the way you expect, there wouldn't be any need for the leaky feature.

mrrodriguez commented 2 years ago

Ideally the behavior of with-profile with uberjar would create consistency with regards to how :leaky profiles interact with the explicit with-profile profiles. I believe that the with-profile explicitly set profiles would generally not expect to be unmerged during the build of the given task and its include sub-tasks that are used to build the final artifact.

Sorry, I don't really follow this logic. The documentation states:

If you mark your profile with ^:leaky metadata, then the profile will not be stripped out when the pom and jar files are created.

The point of the leaky metadata is that you can get the behavior you describe, if you want it. This seems consistent with the observed behavior. If the default behavior worked the way you expect, there wouldn't be any need for the leaky feature.

@technomancy the main point I was describing is the with-profile mechanism in some cases “breaks” the policy around unmerging leaky profiles. In my example I showed it shows that with-profile profiles are “sometimes” active during stages of the uberjar process. So those profiles do effect the artifact built. But for some parts of the artifact building - like the pom - with-profile doesn’t subvert the leaky semantics so you get the inconsistency. If leaky profiles were required for building the artifact then I wouldn’t expect that to only be true for parts of it.

technomancy commented 2 years ago

Ah, I see what you mean now.

Normally with profiles at this point I assume any changes to the profile logic will cause an equal or greater number of problems as they solve. (after this has shown itself to be the case time and time again) However, this problem seems to be fairly focused and limited in scope to just the pom inside the uberjar. So I think this might be an exception.

It sounds like you have taken the time to dive into the implementation and have a decent understanding of what's going on. Would you be able to provide a fix for this?

mrrodriguez commented 2 years ago

Ah, I see what you mean now.

Normally with profiles at this point I assume any changes to the profile logic will cause an equal or greater number of problems as they solve. (after this has shown itself to be the case time and time again) However, this problem seems to be fairly focused and limited in scope to just the pom inside the uberjar. So I think this might be an exception.

It sounds like you have taken the time to dive into the implementation and have a decent understanding of what's going on. Would you be able to provide a fix for this?

I can look into this a bit. I do understand the implementation well enough I think. I also agree that profile-logic changes do seem to be an easy source of more trouble.

Is the way you are thinking that makes sense here to have the pom built with the same profiles as the uberjar/jar, which is with the leaky profiles - but also those profiles explicitly set in :included-profiles metadata that, in my case above, come from the with-profile higher-order task? The alternative I think would be to remove non-leaky profiles from the jar building step of the uberjar as well, ie. not just include all :include-profiles without checking.

technomancy commented 2 years ago

Is the way you are thinking that makes sense here to have the pom built with the same profiles as the uberjar/jar, which is with the leaky profiles - but also those profiles explicitly set in :included-profiles metadata that, in my case above, come from the with-profile higher-order task?

That's right; the pom inside the uberjar should cover all the dependencies used to create the uberjar, which includes those explicitly added in by with-profile. Thanks!

technomancy commented 2 years ago

I would like to release 2.9.9 some time in the next week; do you think you could get the fix by then, or should it wait for a future version?

mrrodriguez commented 2 years ago

I would like to release 2.9.9 some time in the next week; do you think you could get the fix by then, or should it wait for a future version?

I think it'll have to be a future version. I will try to get to look at this within the next 2 weeks.

technomancy commented 2 years ago

Well, one week for the release might be a little optimistic on my side. If it's coming soon then holding off another week isn't a big deal. =)

mrrodriguez commented 2 years ago

Just an update here. I've been delayed getting to this. I'd say I will be hopeful to make some progress towards it by early next week. If you need to release earlier than that, feel free to not block on it. However, I will try to make the progress either way.

technomancy commented 2 years ago

Any progress on this? Just got a fix for the other big blocker bug for 2.9.9.

mrrodriguez commented 2 years ago

Any progress on this? Just got a fix for the other big blocker bug for 2.9.9.

Hi, Sorry I've dropped the ball on this one. I am working on hitting work deadlines, so I'll be delayed getting to it for another month or so. I do intend to get back to it if no one has looked at it by then though.

Sorry.

technomancy commented 2 years ago

No worries; you're the one most affected by this. It'll just mean it will be longer before it makes it out into a release.