spoon icon indicating copy to clipboard operation
spoon copied to clipboard

No need of using `M2_HOME` environment variable as fallback to find path to maven executable in Windows

Open algomaster99 opened this issue 3 years ago • 6 comments

We no longer need to get the path to maven executable via M2_HOME because its usage has been deprecated with newer Windows releases.

EDIT: Support for M2_HOME was deprecated in Maven 3.5.0. See 5th point in the overview about the changes.

algomaster99 avatar Nov 15 '21 08:11 algomaster99

Could you try to find the actual release notes for when this M2_HOME was deprecated and/or removed? From the Maven release notes that is.

The links here just refer to it as "being gone", or something like that.

slarse avatar Nov 15 '21 19:11 slarse

@slarse Yes, I can find that. I once looked up release notes of Windows environment which just hit me that it was the wrong place to look at. I will try finding it on the Apache maven website.

algomaster99 avatar Nov 15 '21 19:11 algomaster99

@slarse , found it! Look at the updated issue description.

algomaster99 avatar Nov 15 '21 19:11 algomaster99

@algomaster99 Good find.

I don't think it's a good idea to remove the M2_HOME fallback yet. Maven 3.5 is relatively recent (released in 2017). Up until relatively recently, we were running Maven 3.3 on Jenkins, and releases as old as Maven 3.1 are still supported. As there are still-supported releases where using M2_HOME is an entirely valid practice, it doesn't seem right to remove it outright.

There are however a couple of action points.

At the very least, we shouldn't mention M2_HOME when we can't find the Maven binary, only that the binary/program couldn't be found. The error message should advice the user to ensure that the binary is on path (system path for Windows, I believe). I suspect that the current error message (""M2_HOME must be initialized to use this MavenLauncher constructor.") was written before guessMavenHome() was implemented, and only M2_HOME was used.

We could go a step further and log a warning if we have to use the M2_HOME fallback, and advise the user to put the Maven binary on the path.

slarse avatar Nov 15 '21 21:11 slarse

Summarising the above comment,

  • [x] Refactor error messages so that it doesn't mislead us that M2_HOME is not on PATH. Done in: https://github.com/INRIA/spoon/pull/4298
  • [ ] Log a warning if the code uses M2_HOME.

algomaster99 avatar Nov 26 '21 15:11 algomaster99

If we want to be super nice about it, we should only log a warning message if using M2_HOME with Maven version 3.5.0 or later. Because before that, M2_HOME was fair game.

slarse avatar Nov 27 '21 09:11 slarse