[MWRAPPER-14] put all wrapper pieces in one build

[hboutemy]

Wrapper consists in 3 pieces:

• wrapper jar (maven-wrapper.jar)
• wrapper distribution (mvnw scripts)
• wrapper plugin
  that should have the same version = "the wrapper version"
  the wrapper plugin will unpack wrapper distribution with the same version as the plugin

the only drawback vs releasing wrapper jar and wrapper distribution in the Apache Maven core release is that partial mvn scripts (3 shell and 3 cmd) are copied

but in terms of versions clarity, seems much easier to understand: wrapper is something, Maven is something

notice that this is a WIP: I did not yet review maven-wrapper-plugin logic to select the distribution version

[rfscholte]

I'm not convinced this is the right thing to do. If you compare the original mvn and mvnw script when it was donated, you should have noticed that they both went their own way. It took me quite some time to get them in sync and to pick the best of both. By splitting it up it will happen again.
Maybe you should wait with this AFTER MNG-7073 so Maven contains a script template that can be reused here.

Moving the wrapper to here, does that mean that this plugin must be released every time is released too, like the takari plugin used to do? Then we've lost the benefit of the current setup, because most likely the plugin now doesn't require new releases.

[hboutemy]

I completely value the work done to sync mvn and mvnw: syncing again is just about copying the 6 partial scripts to maven-wrapper-distribution/src/assembly/maven/.

on releasing, there is only 1 simultaneous release for the 3 artifacts: wrapper, wrapper distribution and wrapper-plugin = the full wrapper, clearly separate from the Maven distribution. It is really simple and logical.
And there won't be many releases in the future, because everything is independant from Maven version.

with the latest commits, the new wrapper is working well with whatever Maven version

[rfscholte]

So how do you intend to keep up with changes like apache/maven#602 ?

[slawekjaranowski]

Another idea ... why not simply call mvn* scripts from Maven distribution ... of course after needed job for wrapper like downloading

In this case code will be in one place

[rfscholte]

@slawekjaranowski if that would work, that would be a much more acceptable solution for me.

[hboutemy]

yes, instead of copying by hand partial scripts from https://github.com/apache/maven/tree/master/apache-maven/src/assembly/shared to current project source https://github.com/apache/maven-wrapper-plugin/tree/MWRAPPER-14/maven-wrapper-distribution/src/assembly/maven , it can be automated => I'll add this soon

[slawekjaranowski]

I think that running mvn from Maven distribution will be better ...
Some change to maven scripts can be depend on Maven core code like:
apache/maven#602

so what will happen when we have newer scripts in wrapper and want to setup older Maven to use ...

[hboutemy]

oh, yes, sorry, please forget my previous comment: morning coffe took more time than I expected to warm up my brain :)

Yes, having mvnw script (stored in source control) call downloaded mvn from the Maven distribution is the only solution to be sure that exact features are available

to implement it, we'll need to have mvnwscript know where the Maven distribution has been unpacked (by maven-wrapper.jar): any idea how to do that?

[hboutemy]

I created https://issues.apache.org/jira/browse/MWRAPPER-16 to track this very good idea on mvnw launching mvn scripts: I hope we'll find a solution some day

[jvanzyl]

Another idea ... why not simply call mvn* scripts from Maven distribution ... of course after needed job for wrapper like downloading

In this case code will be in one place

That's a great idea. I originally thought of a tiny plugin (I think one might actually exist) to have the core grab the scripts from the wrapper for reuse, but your idea is far better and essentially zero maintenance.

[jvanzyl]

Historically the Maven Wrapper has been separate, and I believe works well that way. Its concern to bring Maven into existence for build, and I would argue that should be any version of Maven. The simple fact that the Takari Maven Wrapper still needs to exist is an indication that we have an issue. I understand the maintenance issue with the scripts, but I think @slawekjaranowski has an elegant solution to that problem.

I would very much like to archive the Takari Maven Wrapper repository, and add some features to the wrapper at Apache. With the wrapper separated we may even be able to help people still using Maven 2.x (I've seen lots of 2.x projects) as the wrapper makes CI so much easier. But again, I think the wrapper has a separate concern, and we may want to add new features to the wrapper and it's been very nice with the Takari Maven Wrapper to just release it when necessary without concern for any particular version of Maven.

But I do think having the wrapper and wrapper plugin together makes sense. I didn't think about it very hard with the Takari Wrapper but it definitely would have been easier if all the wrapper capability was together. I think what's in the PR here with the addendum of @slawekjaranowski's solution is ideal.

[rfscholte]

The only potential problem I see is that a script calls another script that calls java. Should be easy to validate.

[slawekjaranowski]

From wrapper perspective should not be important what mvn command is and what happen inside mvn.

We should assume that mvn command from Maven distribution simply work.

[hboutemy]

ok, so no objection to merge this PR?
please approve it

[rfscholte]

it still contains the copy of the original scripts, right? Then I'm -1.

[hboutemy]

wrong logic: not merging MWRAPPER-14 (and not releasing a usable wrapper) does not help on evolving with MWRAPPER-16

please think again: scripts released as part of Maven core do not help to have consistency between wrapper scripts copied to project Git and scripts from downloaded Maven distribution = the right objective we share. It just helps on their maintenance in ASF Git (between maven.git and maven-wrapper-plugin.git), at the cost of a confusing release cycle and limitation on possible target Maven version (future Maven 4 only)

but at least, I can create MWRAPPER-16 branch to be merged to MWRAPPER-14: it's just sad to not be able to work in a constructive way

[jvanzyl]

@rfscholte are you fine with a first step of dynamically importing the scripts and implementing the more sophisticated https://issues.apache.org/jira/browse/MWRAPPER-16 later? I’d really like to shut down the Takari Wrapper repo sooner rather than later, if possible.

[rfscholte]

This commit touches 75 files, and after MWRAPPER-16 a lot of them are removed.
Keep in mind that I moved it to maven-core due to the huge overlap of scripts, but that argument disappears with MWRAPPER-16.
I'd suggest to start from scratch with the scripts in MWRAPPER-14, and just skip MWRAPPER-14.

[rfscholte]

If this value needs to be in sync with the Maven release, then this class (and its artifact) doesn't belong here.

[hboutemy]

yes, the more I'm merging back everything in one build, the more I'm in fact reverting many changes that were done while you tried to put some pieces in Maven core

you're probably right, instead of continuing the current branch, we probably better start again from original maven-wrapper history and merge maven-wrapper-plugin in it: that may be easier to get to a new stable situation
is it what you had in mind?

[rfscholte]

Right, so there are a couple of things that have been solved by moving it from takari to maven-core:

• no additional releases required.
• identical scripts are in sync.
  We shouldn't loose these improvements.

[hboutemy]

last try:

• code = https://github.com/hboutemy/maven-wrapper = the initial wrapper donation with 4 commits to switch to Apache, replace provisio to assembly, add maven-wrapper-plugin and document
• site: https://maven.apache.org/wrapper-archives/wrapper-LATEST/

This approach of wrapper does not need any releases when new Maven versions are released, and can work with any version from the past and any Maven SNAPSHOT (like the original donation)

Getting identical features of installed mvnw in a project Git vs downloaded Maven distribution mvn will be part of MWRAPPER-16 = stop trying to re-implement mvn logic in mvnw to launch a JVM and Maven core, but call mvn = the only solution to get consistency

all, please review: in the end, I expect to create a maven-wrapper.git repository containing my initial work (that contains initial donation), and we'll track future features an release normally in MWRAPPER Jira

[slawekjaranowski]

@hboutemy great job.

I've tested by:

• build project - unit and IT test are executed, plugin and artifacts are installed
• use in another project with some of released maven version ... looks ok

In this point of time I'm not looking deeper in code, simple build and use.

I think that push this code to public repo and release this version as is - will be the best approach.
It opens a way to testing by other users as well.

Next improvement can be done in small, easy to review changes.

[jvanzyl]

I run the following in the cloned repository itself:

mvn clean install
mvn org.apache.maven.plugins:maven-wrapper-plugin:3.0.3-SNAPSHOT:wrapper
./mvnw clean install

Which yields:

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
curl: (22) The requested URL returned error: 404 
Error: Could not find or load main class org.apache.maven.wrapper.MavenWrapperMain

All I have in the .mvn directory is the following:

bash-3.2$ tree .mvn
.mvn
└── wrapper
    └── maven-wrapper.properties

@slawekjaranowski how did you test it?

[slawekjaranowski]

in maven-wrapper

mvn clean install -P run-its

and in other project

mvn org.apache.maven.plugins:maven-wrapper-plugin:3.0.3-SNAPSHOT:wrapper -Dtype=bin -Dmaven=3.6.2

important is type=bin, so I have:

tree .mvn/
.mvn/
└── wrapper
    ├── maven-wrapper.jar
    └── maven-wrapper.properties

[jvanzyl]

@hboutemy I think that's poor default behavior, the type=bin should be the default. That's not how the current released wrapper works. No one will read the docs, that's not what will be expected and the first experience from new users will be poor.

[slawekjaranowski]

Of course, wrapperUrl in properties is wrong, but release (even staging) should resolve it

wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.0.3-SNAPSHOT/maven-wrapper-3.0.3-SNAPSHOT.zip

[jvanzyl]

I'm not sure why the slight change was made, but the latest published instructions put the JAR in there by default. Just a warning that it's asking for potential support issues. Maybe someone has one network setup and commits the wrapper change not noticing the JAR isn't there. Changes network setups and their network or corporate setup doesn't allow access.

I think it's nicer not to have any JARs checked in but changing the way it works before it's released may not make for a great first release.

[hboutemy]

the issue you have is only with SNAPSHOTs of maven-wrapper, that are obviously not published to central
But with releases, there won't be any problem for normal users using default

during development with SNAPSHOT maven-wrapper, see last section of https://maven.apache.org/wrapper-archives/wrapper-LATEST/

[hboutemy]

but for sure, there is the question: what should be the default? current scriptor bin as it was in Takari?
I kept the script default value that Robert chose previously, changing is easy, it's really just a choice to do

[hboutemy]

thinking at default type more in depth, I think that I now understand the logic
= making script the default is not sufficient, because if .mvn/wrapper/maven-wrapper.jar or *.jar is not in .gitignore, the jar will inevitably come into future Git commit
then the logic is to have bin by default when installing maven-wrapper, and the .gitignore configuration will trigger if the jar is stored or not (and if not stored, the consequence in network access)

thinking like this, I now feel that yes, bin should be default...

[rfscholte]

The choice for script was because in general we don't want binaries in the SCM. There's no real damage if the file is committed anyway.

[rfscholte]

There are still things in the code I want to comment on, but that's hard to describe here.

[rfscholte]

Can https://github.com/hboutemy/maven-wrapper be cloned to maven-studies? That makes it easier for everybody to do their changes.

[hboutemy]

ok, code pushed in MWRPPER-14 branch in maven-studies https://github.com/apache/maven-studies/tree/MWRAPPER-14

[hboutemy]

The choice for script was because in general we don't want binaries in the SCM. There's no real damage if the file is committed anyway.

making script the default is not sufficient, because if .mvn/wrapper/maven-wrapper.jar or *.jar is not in .gitignore, the jar will inevitably come into future Git commit, not by the guy who did mvn wrapper:wrapper, by the user who will do ./mvnw and commit everything without remarking that wrapper.jar was downloaded

then forcing mvn wrapper:wrapper not to download maven-wrapper.jar just adds an additional friction (= mvnw needs to be able to download the jar, without the help of Maven settings)

[rfscholte]

In the end the-person-that-called-wrapper:wrapper should decide. It is a matter of a "sources-only repository" (either download via script or Java) versus "don't download", not the "cloner/forker".
To me it is weird we make an exception for the wrapper.jar, whereas we all prefer sources only for everything else.

IIUC everything is configurable to download, including repo-manager, proxy, etc. It should be possible to guide in case it doesn't work.

Also, regarding the potential unintended commit: the plugin could help here generating scm-specific ignore files.