Maven Invoker Plugin
  1. Maven Invoker Plugin
  2. MINVOKER-97

Add possibility to inherit settings.xml from calling process

    Details

    • Type: New Feature New Feature
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 1.6
    • Labels:
      None
    • Number of attachments :
      2

      Description

      Invoker will use default settings xml even if differetn settings xml is specified in calling maven process. It would be very usefull to add posibility to "inherit" settings.xml to invoker:run goal (settingsFile parameter is not enough, since it seems imposible to determine settings file from in calling maven process)

      1. MINVOKER-97_rev2.patch
        18 kB
        Anders Hammar
      2. MINVOKER-97.patch
        8 kB
        Anders Hammar

        Issue Links

          Activity

          Hide
          Olivier Lamy added a comment -

          so we have to merge 2 settings file. Which one is the dominant ?

          Show
          Olivier Lamy added a comment - so we have to merge 2 settings file. Which one is the dominant ?
          Hide
          Konstantin Titorenko added a comment -

          Don't think that any kind of merge is required, what is needed is ability to use same settings.xml that was used by caller process.
          For example, if you run maven if -s key it is not really logical for invoker to ignore that setting.
          One possibility would be to add a parameter to run goal of invoker, e.g. inheritSettingsXml=true/false, then in case of true use settings.xml from caller process, current behavior otherwise.

          Show
          Konstantin Titorenko added a comment - Don't think that any kind of merge is required, what is needed is ability to use same settings.xml that was used by caller process. For example, if you run maven if -s key it is not really logical for invoker to ignore that setting. One possibility would be to add a parameter to run goal of invoker, e.g. inheritSettingsXml=true/false, then in case of true use settings.xml from caller process, current behavior otherwise.
          Hide
          Julien Nicoulaud added a comment -

          As Konstantin said, you could add an 'inheritSettings' parameter, but the 'settingsFile' would still be loaded and add/override what it needs to.

          This way we would inherit the proxies section and still use the "fast build" configuration.

          Show
          Julien Nicoulaud added a comment - As Konstantin said, you could add an 'inheritSettings' parameter, but the 'settingsFile' would still be loaded and add/override what it needs to. This way we would inherit the proxies section and still use the "fast build" configuration.
          Hide
          Anders Hammar added a comment -

          This feature is important to make this plugin work in the new Maven 3 integration of Hudson. MRELEASE-577 covers the same problem but for the m-release-p.

          Show
          Anders Hammar added a comment - This feature is important to make this plugin work in the new Maven 3 integration of Hudson. MRELEASE-577 covers the same problem but for the m-release-p.
          Hide
          Anders Hammar added a comment -

          Patch attached. Highly inspired of (aka copied from) the solution in m-release-p. IT included.

          Show
          Anders Hammar added a comment - Patch attached. Highly inspired of (aka copied from) the solution in m-release-p. IT included.
          Hide
          Olivier Lamy added a comment -

          @Anders your patch doesn't inherits (i.e. merge) but use the user settings. BTW that's good
          But I was more thinking about merging settings (the one from the plugin configuration and the user settings). Xpp3Dom has some util methods to do that.

          Show
          Olivier Lamy added a comment - @Anders your patch doesn't inherits (i.e. merge) but use the user settings. BTW that's good But I was more thinking about merging settings (the one from the plugin configuration and the user settings). Xpp3Dom has some util methods to do that.
          Hide
          Anders Hammar added a comment -

          That would differ from how it currently works for the m-release-plugin. I think they should be in sync.

          If we would merge, which one would be dominant (Back to your first question)? I think that the one defined in the plugin config should be dominant, as it would provide a means of overriding and pinning down things that should be used by the plugin's execution.

          Show
          Anders Hammar added a comment - That would differ from how it currently works for the m-release-plugin. I think they should be in sync. If we would merge, which one would be dominant (Back to your first question)? I think that the one defined in the plugin config should be dominant, as it would provide a means of overriding and pinning down things that should be used by the plugin's execution.
          Hide
          Olivier Lamy added a comment -

          agree on the one defined in plugin config as dominant.

          Show
          Olivier Lamy added a comment - agree on the one defined in plugin config as dominant.
          Hide
          Anders Hammar added a comment -

          Should the user have to enable merging by configuraton? That would preserve the current default behavior. BUT, I argue that the current behavior of ignoring the settings of the initiating process is wrong (a bug) and therefore the default behavior should be to use it. I'd even go so far that I'd argue that it shouldn't be possible to turn off. If you want a specific setting you need to specify that in the specified settings file.

          So here's my proposed solution:

          • If no settings file is specified in the plugin configuration, use the one of the invoking process.
          • If a settings file is specified in the plugin configuration, merge it with the one of the invoking process. Coniguration in the specified settings file is dominant.
          • In both cases the current behavior of local repository should be preserved.
          Show
          Anders Hammar added a comment - Should the user have to enable merging by configuraton? That would preserve the current default behavior. BUT, I argue that the current behavior of ignoring the settings of the initiating process is wrong (a bug) and therefore the default behavior should be to use it. I'd even go so far that I'd argue that it shouldn't be possible to turn off. If you want a specific setting you need to specify that in the specified settings file. So here's my proposed solution: If no settings file is specified in the plugin configuration, use the one of the invoking process. If a settings file is specified in the plugin configuration, merge it with the one of the invoking process. Coniguration in the specified settings file is dominant. In both cases the current behavior of local repository should be preserved.
          Hide
          Olivier Lamy added a comment -

          +1 on your proposal !

          Show
          Olivier Lamy added a comment - +1 on your proposal !
          Hide
          Anders Hammar added a comment -

          Could you please change the type of this ticket to bug then?

          Show
          Anders Hammar added a comment - Could you please change the type of this ticket to bug then?
          Hide
          Olivier Lamy added a comment -

          @Anders why ?
          I'm usually not too much bureaucratic but IMHO looks more a "new feature".

          Show
          Olivier Lamy added a comment - @Anders why ? I'm usually not too much bureaucratic but IMHO looks more a "new feature".
          Hide
          Anders Hammar added a comment -

          The way I'm thinking is that if it's a new feature we ought to preserve the current behavior. Thus we need a new param that the used needs to configure to enable the settings to be inherit. But if we say that the current behavior is wrong, this is a bug and we should fix it and don't have to keep the current behavior. Thus, no new param.

          Show
          Anders Hammar added a comment - The way I'm thinking is that if it's a new feature we ought to preserve the current behavior. Thus we need a new param that the used needs to configure to enable the settings to be inherit. But if we say that the current behavior is wrong, this is a bug and we should fix it and don't have to keep the current behavior. Thus, no new param.
          Hide
          Anders Hammar added a comment -

          Attached patch (rev2) implementing proposed solution. ITs included.

          Show
          Anders Hammar added a comment - Attached patch (rev2) implementing proposed solution. ITs included.
          Hide
          Olivier Lamy added a comment -

          patch committed.
          Thanks!

          Show
          Olivier Lamy added a comment - patch committed. Thanks!

            People

            • Assignee:
              Olivier Lamy
              Reporter:
              Konstantin Titorenko
            • Votes:
              4 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: