pypowsybl icon indicating copy to clipboard operation
pypowsybl copied to clipboard

[WIP] Maven wrapper

Open geofjamg opened this issue 1 year ago • 12 comments

Please check if the PR fulfills these requirements

  • [x] The commit message follows our guidelines
  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem?

No

What kind of change does this PR introduce?

Feature

What is the new behavior (if this is a feature change)? We use Maven wrapper to build the Java part, so that we do not require to install a specific Maven version.

Does this PR introduce a breaking change or deprecate an API?

  • [ ] The Breaking Change or Deprecated label has been added
  • [ ] The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

Other information:

geofjamg avatar Jul 20 '23 21:07 geofjamg

have you tested the full ci ?

EtienneLt avatar Jul 21 '23 12:07 EtienneLt

is there anything to do with proxy configuration ?

EtienneLt avatar Jul 21 '23 12:07 EtienneLt

have you tested the full ci ?

No, but I'm confident

geofjamg avatar Jul 24 '23 09:07 geofjamg

is there anything to do with proxy configuration ?

@olperr1 I guess you add some doc on core side to how configure the proxy for the maven wrapper?

geofjamg avatar Jul 24 '23 09:07 geofjamg

is there anything to do with proxy configuration ?

@olperr1 I guess you add some doc on core side to how configure the proxy for the maven wrapper?

@geofjamg: I added a section about it in the README.md of powsybl-core... But we found out with @jonenst that it doesn't work totally. He just found the whole solution:

There are 2 steps:

  1. the maven wrapper distribution download
  2. the maven distribution download

For the first step, you should define the proxy in the current terminal via the http_proxy envvar. For the second step, you should use one of the following methods:

./mvnw -DproxyHost=XXX -DproxyPort=XXX -Dhttp.proxyUser=XXX -Dhttp.proxyPassword=XXX -Djdk.http.auth.tunneling.disabledSchemes= clean

or

export MVNW_USERNAME=XXX
export MVNW_PASSWORD=XXX
./mvnw -DproxyHost=XXX -DproxyPort=XXX -Djdk.http.auth.tunneling.disabledSchemes= clean

Note that in both cases, the -Djdk.http.auth.tunneling.disabledSchemes= option should be left empty.

I will add a PR in core to amend the README.md.

olperr1 avatar Jul 24 '23 13:07 olperr1

Do we need to add the install.sh like in core ?

EtienneLt avatar Jul 24 '23 13:07 EtienneLt

@EtienneLt could you try to build with the proxy ?

geofjamg avatar Jul 24 '23 13:07 geofjamg

Do we need to add the install.sh like in core ?

No

geofjamg avatar Jul 24 '23 13:07 geofjamg

./mvnw -DproxyHost=XXX -DproxyPort=XXX -Dhttp.proxyUser=XXX -Dhttp.proxyPassword=XXX -Djdk.http.auth.tunneling.disabledSchemes= clean

This one is problematic for us, because call of mvnw is done automatically from cmake file. It means that we need to automatically define -DproxyHost=XXX -DproxyPort=XXX -Dhttp.proxyUser=XXX -Dhttp.proxyPassword=XXX from env variables int he CmakeList.txt

Or not use the Maven wrapper...

geofjamg avatar Jul 24 '23 13:07 geofjamg

well I make the release if we keep this pr it will be in next release is it alright ?

EtienneLt avatar Jul 24 '23 13:07 EtienneLt

./mvnw -DproxyHost=XXX -DproxyPort=XXX -Dhttp.proxyUser=XXX -Dhttp.proxyPassword=XXX -Djdk.http.auth.tunneling.disabledSchemes= clean

This one is problematic for us, because call of mvnw is done automatically from cmake file. It means that we need to automatically define -DproxyHost=XXX -DproxyPort=XXX -Dhttp.proxyUser=XXX -Dhttp.proxyPassword=XXX from env variables int he CmakeList.txt

Or not use the Maven wrapper...

There is no need to execute this command every time. It can be called manually once to download the wrapper and Maven distributions. Then mvnw won't try to download anything until the maven or the maven wrapper version changes. (Note that Maven will download artifacts. So you may have to define a proper maven settings.xml file... which can contain a proxy definition.)

olperr1 avatar Jul 24 '23 13:07 olperr1

well I make the release if we keep this pr it will be in next release is it alright ?

yes, let's take time to think about the best solution for this PR

geofjamg avatar Jul 24 '23 14:07 geofjamg