Maven 2 & 3

Lack of error checks on profiles

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.0.9
  • Fix Version/s: 2.0.11, 2.1.0, 3.0-alpha-3
  • Component/s: Profiles
  • Labels:
    None
  • Complexity:
    Intermediate
  • Patch Submitted:
    Yes
  • Number of attachments :
    3

Description

DefaultProfileManager performs no error checks on the profile IDs So If I specify bogus profile IDs from plugins (like mvn -P no-such-profile), Maven doesn't complain, and it just runs as if nothing was specified. This is very error prone.

Also, I've seen some documentation that says deactivating a profile is "-P !profile". As far as I can tell from the code, this is wrong, but because of the lack of error check, such usage just gets ignored, and the user is left confused as to why the profile isn't deactivated.

Issue Links

Activity

Hide
John Casey added a comment -

given the fact that we can trigger settings profiles at the CLI, we can't make these cases fail the build I don't think...but we could provide a prominent warning.

Show
John Casey added a comment - given the fact that we can trigger settings profiles at the CLI, we can't make these cases fail the build I don't think...but we could provide a prominent warning.
Hide
Torben S. Giesselmann added a comment -

This patch will have Maven print a warning if a profile which should have been explicitely activated has in fact not been activated.

It's just a warning message, therefore I didn't write a test case. But just out of curiosity: how would I test whether a warning with a specific message was logged or not? Is there a pattern for that?

Show
Torben S. Giesselmann added a comment - This patch will have Maven print a warning if a profile which should have been explicitely activated has in fact not been activated. It's just a warning message, therefore I didn't write a test case. But just out of curiosity: how would I test whether a warning with a specific message was logged or not? Is there a pattern for that?
Hide
Brett Porter added a comment -

you can do it in an integration test by checking the log output

Show
Brett Porter added a comment - you can do it in an integration test by checking the log output
Hide
Torben S. Giesselmann added a comment -

... and here's the IT. Yay!

Show
Torben S. Giesselmann added a comment - ... and here's the IT. Yay!
Hide
Brett Porter added a comment -

applied - great patch, thanks!

Two things to watch out for:

  • if your IT runs multiple times on a project, set different log filenames for debugging purposes (I added this)
  • watch the code formatting (tabs, specifically
Show
Brett Porter added a comment - applied - great patch, thanks! Two things to watch out for:
  • if your IT runs multiple times on a project, set different log filenames for debugging purposes (I added this)
  • watch the code formatting (tabs, specifically
Hide
Benjamin Bentmann added a comment -

This might need some more love: The current approach produces a lot of warnings during a multi-module buid, namely one per each module that does not contain a profile. See the attached demo project (run "mvn validate -P test") or one of the core bootstrap jobs on Hudson that run with the profile "run-its" which is only present in one of the modules, leaving about fifty warnings about profile activation around.

While I don't have objections to a warning per module, their placement is the log output appears suboptimal. Right now, the warnings appear right at the start, even before the reactor build order, so outside of any context that might be helpful for a user to associate the warning with the module that caused it. Also, given the verbosity of Maven's log, the warnings are easily overlooked when printed first instead of say at the end of a module build.

Show
Benjamin Bentmann added a comment - This might need some more love: The current approach produces a lot of warnings during a multi-module buid, namely one per each module that does not contain a profile. See the attached demo project (run "mvn validate -P test") or one of the core bootstrap jobs on Hudson that run with the profile "run-its" which is only present in one of the modules, leaving about fifty warnings about profile activation around. While I don't have objections to a warning per module, their placement is the log output appears suboptimal. Right now, the warnings appear right at the start, even before the reactor build order, so outside of any context that might be helpful for a user to associate the warning with the module that caused it. Also, given the verbosity of Maven's log, the warnings are easily overlooked when printed first instead of say at the end of a module build.
Hide
Brett Porter added a comment -

moved to the reactor so it only checks once for all the projects being built.

The warning is still "inconspicuous", but I can't really see a better place to put it. This is a more general Maven problem

Show
Brett Porter added a comment - moved to the reactor so it only checks once for all the projects being built. The warning is still "inconspicuous", but I can't really see a better place to put it. This is a more general Maven problem
Hide
Benjamin Bentmann added a comment -

Ported to 3.x in r787045.

Show
Benjamin Bentmann added a comment - Ported to 3.x in r787045.
Hide
Ian Springer added a comment -

The current warning message is:

"Profile with id: '" + explicitProfileId + "' has not been activated."

I think this message is misleading, because the profile actually is activated - it's just not used at all in the pom being processed. I suggest changing the message to something like:

"Profile with id '" + explicitProfileId + "' is activated, but this pom does not contain any usages of the profile."

Also, I don't think it makes sense to print this warning at all in a multi-module build. In the large multi-module project I work on, we have a number of profiles that are only used in a handful of the 50 or so modules.

Show
Ian Springer added a comment - The current warning message is: "Profile with id: '" + explicitProfileId + "' has not been activated." I think this message is misleading, because the profile actually is activated - it's just not used at all in the pom being processed. I suggest changing the message to something like: "Profile with id '" + explicitProfileId + "' is activated, but this pom does not contain any usages of the profile." Also, I don't think it makes sense to print this warning at all in a multi-module build. In the large multi-module project I work on, we have a number of profiles that are only used in a handful of the 50 or so modules.
Hide
Brett Porter added a comment -

Ian, please open a new issue for suggested changes. Closed issues don't get any attention.

Show
Brett Porter added a comment - Ian, please open a new issue for suggested changes. Closed issues don't get any attention.

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: