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

[MRESOLVER-588] Add env-variables as sys-props in Maven-4 SessionBuilderSupplier #539

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HannesWell
Copy link
Contributor

@HannesWell HannesWell commented Jul 17, 2024

Fixes https://issues.apache.org/jira/browse/MRESOLVER-588 for the Maven-3 SessionBuilderSupplier

@HannesWell
Copy link
Contributor Author

A similar change (plus the line session.setSystemProperties(System.getProperties());) could be done in the SessionBuilderSupplier for Maven4. But I assume this is better done in Maven-4 directly, isn't it?

@HannesWell HannesWell changed the title [MRESOLVER-588] Add env-variables as sys-props in SessionBuilderSupplier [MRESOLVER-588] Add env-variables as sys-props in Maven-4 SessionBuilderSupplier Jul 17, 2024
@cstamas
Copy link
Member

cstamas commented Aug 2, 2024

Personally, I'd skip doing this altogether and here is why:

  • as i see in Maven4 supplier (plus Maven side of bits) this is not happening at all
  • the goal of supplier is to provide "baseline" only, and nothing in Resolver uses ENV variables
  • the idea is that user who needs env or whatever, just customize supplier by overriding required method

@HannesWell
Copy link
Contributor Author

* the goal of supplier is to provide "baseline" only, and nothing in Resolver uses ENV variables

I understand that. But the problem can be and that's why I encountered this at all, is that some artifacts cannot be resolved some system properties are missing.
For example without any system-properties, org.apache.commons:commons-text:1.12.0 fails to resolve with a ModelBuildingException saying something like [ERROR] Failed to determine Java version for profile. In this case the java.version property is missing and e.g. the animal-sniffer profile from the commons-parent cannot determine if the activation element <jdk>(,9)</jdk> is met or not. And since properties and env-properties can also be used to activate profiles in general and profiles can contain additional dependencies, not having them can influence the resolution result.
A general property activation will probably not fail the resolution if a property is missing (I haven't tested it), but the result would still be different than in a default maven installation. So the resolver could implicitly use env variablies.
But I cannot tell if the latter already is already out of the scope of the baseline?

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

Successfully merging this pull request may close these issues.

3 participants