Uploaded image for project: 'Maven'
  1. Maven
  2. MNG-5836

logging config is overridden by $M2_HOME/lib/ext/*.jar

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.5.0-alpha-1, 3.5.0
    • Component/s: None
    • Labels:
      None

      Description

      If one of the jars in $M2_HOME/lib/ext/*.jar happens to have simplelogger.properties, that configuration file masks logging configuration under $M2_HOME/conf/logging

        Issue Links

          Activity

          Hide
          michael-o Michael Osipov added a comment -

          That is, clearly for me, a problem in the dependency not in Maven. Only the client has to configure logging, not the dependency.

          Show
          michael-o Michael Osipov added a comment - That is, clearly for me, a problem in the dependency not in Maven. Only the client has to configure logging, not the dependency.
          Hide
          igorf Igor Fedorenko added a comment -

          Hmm. Yes, this is kinda my point, $M2_HOME/conf/logging should be the logging configuration used by maven regardless of what is included in custom libraries. In practical terms this means we need to change order of m2.conf entries, which I'll do when I get back to regular office.

          Show
          igorf Igor Fedorenko added a comment - Hmm. Yes, this is kinda my point, $M2_HOME/conf/logging should be the logging configuration used by maven regardless of what is included in custom libraries. In practical terms this means we need to change order of m2.conf entries, which I'll do when I get back to regular office.
          Hide
          michael-o Michael Osipov added a comment -

          I would rather see that faulty library fixed because it is simply wrong. Maybe a warning would be good. One should fix the cause and not the symptom.

          Show
          michael-o Michael Osipov added a comment - I would rather see that faulty library fixed because it is simply wrong. Maybe a warning would be good. One should fix the cause and not the symptom.
          Hide
          igorf Igor Fedorenko added a comment -

          Just to be clear, do you suggest that current order of m2.conf entries is correct and desirable?

          [plexus.core]
          optionally ${maven.home}/lib/ext/*.jar
          load       ${maven.home}/lib/*.jar
          load       ${maven.home}/conf/logging
          

          Can you elaborate why?

          I can't think of a case when having conf/logging as the last entry in the list is preferable. This is the place where users can change logging configuration, so I expect it to have highest priority regardless of of anything else present on classpath. For example, if conf/logging is the first entry, the logging library could provide default configuration, which the user would still be able override through explicit configuration.

          Show
          igorf Igor Fedorenko added a comment - Just to be clear, do you suggest that current order of m2.conf entries is correct and desirable? [plexus.core] optionally ${maven.home}/lib/ext/*.jar load ${maven.home}/lib/*.jar load ${maven.home}/conf/logging Can you elaborate why? I can't think of a case when having conf/logging as the last entry in the list is preferable. This is the place where users can change logging configuration, so I expect it to have highest priority regardless of of anything else present on classpath. For example, if conf/logging is the first entry, the logging library could provide default configuration, which the user would still be able override through explicit configuration.
          Hide
          michael-o Michael Osipov added a comment - - edited

          I think it is correct. Because logging should be bootstrapped very before the first caller tries to access it. Consider that some lib in $maven.home/lib/*.jar would like to log but the log config is not yet available. That would probably fail.

          Regardless of this, I would stick to the same answer I have given on stack overflow almost four years ago:

          You never provide a log implementation. The client application has to do so. Otherwhise this would be a violation of separation of concerns. Don't do any assumptions about an unknown client.

          Logging and its configuration is solely the task of the client and not a dependency. Everything else is problem.

          For example, if conf/logging is the first entry, the logging library could provide default configuration, which the user would still be able override through explicit configuration.

          We have provisioned the conf/logging directory for that, didn't we? We never expect someone to put a logback.xml into ext.

          If one of your ext JARs has simplelogger.properties, file an issue and have that fixed. I'd rather see Maven issue a warning, if that is possible, indicating the problem.

          Show
          michael-o Michael Osipov added a comment - - edited I think it is correct. Because logging should be bootstrapped very before the first caller tries to access it. Consider that some lib in $maven.home/lib/*.jar would like to log but the log config is not yet available. That would probably fail. Regardless of this, I would stick to the same answer I have given on stack overflow almost four years ago: You never provide a log implementation. The client application has to do so. Otherwhise this would be a violation of separation of concerns. Don't do any assumptions about an unknown client. Logging and its configuration is solely the task of the client and not a dependency. Everything else is problem. For example, if conf/logging is the first entry, the logging library could provide default configuration, which the user would still be able override through explicit configuration. We have provisioned the conf/logging directory for that, didn't we? We never expect someone to put a logback.xml into ext . If one of your ext JARs has simplelogger.properties , file an issue and have that fixed. I'd rather see Maven issue a warning, if that is possible, indicating the problem.
          Hide
          igorf Igor Fedorenko added a comment -

          You didn't provide an example that shows why moving conf/logging to the top of the list is a bad idea. I do think having default logging configuration inside lib/ext/maven-ext-logback.jar would make things easier to configure for the user, but have it your way, I don't feel strongly enough about this.

          Show
          igorf Igor Fedorenko added a comment - You didn't provide an example that shows why moving conf/logging to the top of the list is a bad idea. I do think having default logging configuration inside lib/ext/maven-ext-logback.jar would make things easier to configure for the user, but have it your way, I don't feel strongly enough about this.
          Hide
          hboutemy Hervé Boutemy added a comment - - edited

          Reopening the issue: I think Igor is right
          our conf is sub-optimal: it makes us at risk (of such bad dependency) without giving us any benefit

          given a logging conf should be modifiable, putting it in a jar does not really make sense (which is another way of saying "only the client has to do logging configuration")

          if nobody object, I'll do the IMHO good fix/improvement found by Igor

          Show
          hboutemy Hervé Boutemy added a comment - - edited Reopening the issue: I think Igor is right our conf is sub-optimal: it makes us at risk (of such bad dependency) without giving us any benefit given a logging conf should be modifiable, putting it in a jar does not really make sense (which is another way of saying "only the client has to do logging configuration") if nobody object, I'll do the IMHO good fix/improvement found by Igor
          Hide
          michael-o Michael Osipov added a comment -

          Hervé, even if you try to fix it. I would expect you to provide a warning in the case if a JAR contains a configuration file. Otherwise the crappy won't get noticed at all. That was my objective with.

          Show
          michael-o Michael Osipov added a comment - Hervé, even if you try to fix it. I would expect you to provide a warning in the case if a JAR contains a configuration file. Otherwise the crappy won't get noticed at all. That was my objective with.
          Hide
          hboutemy Hervé Boutemy added a comment -

          I'd like it, but I fear that's not easy to do

          And honestly, we can't make this a blocker: improving Maven to be fool-proof is a good objective, making Maven the detection tool for fool-situation doesn't seem such a good objective

          But yes, if that's easy (since we can detect if a ressource is available twice), we can add a warning if you want: the bug is not in Maven, then I suppose we'll have to find a good warning message to explain the issue

          Show
          hboutemy Hervé Boutemy added a comment - I'd like it, but I fear that's not easy to do And honestly, we can't make this a blocker: improving Maven to be fool-proof is a good objective, making Maven the detection tool for fool-situation doesn't seem such a good objective But yes, if that's easy (since we can detect if a ressource is available twice), we can add a warning if you want: the bug is not in Maven, then I suppose we'll have to find a good warning message to explain the issue
          Hide
          michael-o Michael Osipov added a comment -

          Agreed!

          Show
          michael-o Michael Osipov added a comment - Agreed!
          Hide
          hboutemy Hervé Boutemy added a comment -

          Igor, do you have a sample artifact in central that contains such logging conf, please?

          Show
          hboutemy Hervé Boutemy added a comment - Igor, do you have a sample artifact in central that contains such logging conf, please?
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in maven-3.x #1254 (See https://builds.apache.org/job/maven-3.x/1254/)
          MNG-5836 put $maven.home/conf/logging first in classpath to avoid (hboutemy: rev 3533599e42a4a563abca33a69ed0f34c19815479)

          • apache-maven/src/bin/m2.conf
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in maven-3.x #1254 (See https://builds.apache.org/job/maven-3.x/1254/ ) MNG-5836 put $maven.home/conf/logging first in classpath to avoid (hboutemy: rev 3533599e42a4a563abca33a69ed0f34c19815479) apache-maven/src/bin/m2.conf
          Hide
          igorf Igor Fedorenko added a comment -

          Better late than never, right?

          I originally wanted to provide default concurrent-safe logging configuration as part of https://github.com/takari/concurrent-build-logger . There is a number of example configuration files under src/main/distro/conf/logging/logback.xml if you want to try it.

          Show
          igorf Igor Fedorenko added a comment - Better late than never, right? I originally wanted to provide default concurrent-safe logging configuration as part of https://github.com/takari/concurrent-build-logger . There is a number of example configuration files under src/main/distro/conf/logging/logback.xml if you want to try it.
          Hide
          stephenc Stephen Connolly added a comment -

          Maven 3.4.0 has been dropped. See this thread for more details.

          This issue will need to be re-scheduled for a Maven release in the (hopefully near) future.

          Show
          stephenc Stephen Connolly added a comment - Maven 3.4.0 has been dropped. See this thread for more details. This issue will need to be re-scheduled for a Maven release in the (hopefully near) future.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build maven-3.x #1499 (See https://builds.apache.org/job/maven-3.x/1499/)
          MNG-5836 put $maven.home/conf/logging first in classpath to avoid (hboutemy: http://git-wip-us.apache.org/repos/asf/?p=maven.git&a=commit&h=c516ef79aecdeef5297e44bb5e836e67ffa5336f)

          • (edit) apache-maven/src/bin/m2.conf
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build maven-3.x #1499 (See https://builds.apache.org/job/maven-3.x/1499/ ) MNG-5836 put $maven.home/conf/logging first in classpath to avoid (hboutemy: http://git-wip-us.apache.org/repos/asf/?p=maven.git&a=commit&h=c516ef79aecdeef5297e44bb5e836e67ffa5336f ) (edit) apache-maven/src/bin/m2.conf

            People

            • Assignee:
              hboutemy Hervé Boutemy
              Reporter:
              igorf Igor Fedorenko
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development