Skip to content

Trasnport: use config properties instead user properties.#735

Closed
cstamas wants to merge 1 commit intomasterfrom
fix-transport-settings
Closed

Trasnport: use config properties instead user properties.#735
cstamas wants to merge 1 commit intomasterfrom
fix-transport-settings

Conversation

@cstamas
Copy link
Copy Markdown
Member

@cstamas cstamas commented May 11, 2022

This setting should be possible to be set via MAVEN_OPTS (system wide) or in .mvn/maven.config etc, Currently it is possible only via -Dxxx AFTER mvn....

@cstamas cstamas self-assigned this May 11, 2022
session.setAuthenticationSelector( authSelector );

String transport = request.getUserProperties()
.getProperty( MAVEN_RESOLVER_TRANSPORT_KEY, MAVEN_RESOLVER_TRANSPORT_WAGON );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a merged, final version of the properties and the actual source is irrelevant?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, this is "config" properties (merged plus added things). I think I understand from where "request.systemProperties" are sourced, but from where are "request.userProperties" sourced?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look into Maven CLI, that is the source for it. Commons CLI provides it and it is passed down the line.

Copy link
Copy Markdown
Member

@slawekjaranowski slawekjaranowski May 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

@cstamas cstamas May 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO this is correct change: we do want to allow setting transport in various ways, allowing the change to be "persistent". What we have on master works ONLY if you use -Dmaven.resolver.transport=xx on CLI and nothing else. This also allows CLI override (ie. you have it set in MAVEN_OPTS but you use -Dxxx on command line)

Copy link
Copy Markdown
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a JIRA issue, this needs to go into 3.9.0 and master.

@cstamas
Copy link
Copy Markdown
Member Author

cstamas commented May 11, 2022

Do we want to address build consumer feature as well? As it suffers from very same issue

@michael-o
Copy link
Copy Markdown
Member

No, at least not for now. It is a user feature therefore a user property

@cstamas
Copy link
Copy Markdown
Member Author

cstamas commented May 13, 2022

Superseded by #739

@cstamas cstamas closed this May 13, 2022
@cstamas cstamas deleted the fix-transport-settings branch May 13, 2022 06:40
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