You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/08/24 06:55:24 UTC
[GitHub] [maven] rmannibucau opened a new pull request, #793: MNG-7532 Log shouldn't have been deprecated so ensure it is not
rmannibucau opened a new pull request, #793:
URL: https://github.com/apache/maven/pull/793
Follow up of dev discussions and to avoid to reduplicate the API in the new plugin API, restoring `Log` official status.
Following this checklist to help us incorporate your
contribution quickly and easily:
- [X] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [X] Each commit in the pull request should have a meaningful subject line and body.
- [X] Format the pull request title like `[MNG-XXX] SUMMARY`, where you replace `MNG-XXX`
and `SUMMARY` with the appropriate JIRA issue. Best practice is to use the JIRA issue
title in the pull request title and in the first line of the commit message.
- [X] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [X] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [ ] You have run the [Core IT][core-its] successfully.
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1237402830
@slachiewicz there are some discussions about the related pages since there was never a consensus nor a consistent update of that on our website.
Short term goal of that pr was to avoid to reintroduce the same api in another package for mvn 4 plugin api since we already have a first citizen api we mis-deprecated.
Guess we can split the work and keep the doc topic for another pr/the already open thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] ceki commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
ceki commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225793381
The client facing API of SLF4J is stable and has been stable since version 1.0. You can compile an application using the API in SLF4J 1.0 (released in 2005) and it will run just fine with SLF4J version 2.0 on the classpath.
As for Maven creating its own logging API, there is a [FAQ entry](https://www.slf4j.org/faq.html#optional_dependency) treating this question.
Anyway, thank you all for your awesome work on Maven.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1613609202
> We can not expect that all code in plugins only use Maven and JDK API, thirt party library used in plugins code can uses any logging framework.
Sure and we comply to that, only thing is that end user can only expect maven logging framework to be integrated with maven ecosystem (think CI and maven customizations) so if the plugin provider wants something well integrated it has to use that.
frontend maven plugin forwarded node logs to maven logging system for that to give an example.
> It should not be important what logging framework is used in Maven core.
Probably a good wish but factually it is.
> Technically we can forward logging message from one framework to another and it can be transparent for plugins code.
Good wish too but there are several cases which are not trivial. Personally I see it as limitations when deployed inside maven and most plugins have work arounds for that so all good, but we are not neutral enough (as the JVM is) to support anything.
It is more or less the same than we are not 100% isolated between plugins - classloader is partially only, env is almost not isolated etc...
Not sure about the spring reference but it is the same, they work in some env but not always and this is ok.
The only thing maven guarantees is that the plugin API will go through maven logging system so if integration is customized it will get the logs, anything else is not guaranteed - except JUL by construction indeed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225612530
> Do you expect SLF4J 2.x not to live 10 years?
I expect it to not be usable within 5 years at least due to the current usage of 1.x and SLF4J is a good example of why we shouldnt rely on a 3rd party for our own API (once again, point is used by *users*), it broke several times and we can't help.
Only facade we can leverage would be Java Util Logging if you really want to kill our existing logging abstraction and use something existing IMHO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225848083
> The client facing API of SLF4J is stable and has been stable since version 1.0.
Not really 1.5->1.6 were not compat for ex (LocationAwareLogger#log, MessageFormatter, etc...). It hurt several apps already.
So even I agree it is generally stable, it is not something we can assume for our *api*.
It also increase the work needed to stay compatible since we have our own binding.
It is really not "is slf4j bad" topic - in particular since it is fine to use it in @apache/maven core modules - but simply a design issue (we could workaround technical details if we would like to use it on a pure technical side), we should own our API to ensure mojo and extensions are reliable in time and not highly unstable (like gradle had been and was forced to introduce the wrapper to workaround it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225374475
@michael-o I don't have all the threads handy but we have several dev@ threads explaining it has several technical pitfalls (the way the API loads the impl - v1 or v2) and that we don't want to bind ourselves to an implementation we can't sustain in time. This PR just avoids to have to redefine yet another package and API for plugin (work Guillaume is doing) since we already have it and can enhance it with `Supplier` or alike for formatter like features - more efficient as of today.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1842590373
> Very few use slf4j since it was done in a very recent maven release and most of plugin still comply to older api too.
Not buying this: MANY of our plugins are under maintained, are even coded against Maven2 principles (as I noted in Slack). So, the fact they use something old, is nothing new :smile:
First mention of use of slf4j was around 2008 in dev list (regarding Wagon implementations, shading and all), so unsure what "recent" thing we talk about. Maybe deprecation was "recent" yes, like many other Maven internal issues there were stuck for over 10+ years... but that is NOT a reason to not make them through.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225894024
> > The client facing API of SLF4J is stable and has been stable since version 1.0.
>
> Not really 1.5->1.6 were not compat for ex (LocationAwareLogger#log, MessageFormatter, etc...). It hurt several apps already. So even I agree it is generally stable, it is not something we can assume for our _api_. It also increase the work needed to stay compatible since we have our own binding.
>
> It is really not "is slf4j bad" topic - in particular since it is fine to use it in @apache/maven core modules - but simply a design issue (we could workaround technical details if we would like to use it on a pure technical side), we should own our API to ensure mojo and extensions are reliable in time and not highly unstable (like gradle had been and was forced to introduce the wrapper to workaround it).
You are mostly talking about impl, not API not. For the plugin dev.SLF4J simply works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1842552477
Ok so guess we should:
1. merge this PR to not encourage people to use slf4j since we agreed it was wrong a long time ago now
2. encourage to move to maven 4 when possible (until then keep the old bridge solution plugins were using)
3. probably document (enhance if needed) the classloading tuning to enable plugins to inherit or not from slf4j (think we discussed that)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1845227359
> @gnodet well code was shared but you can review https://github.com/rmannibucau/mojo-logging-demo which is only about 2 cases (but there are more like using alpha version of slf4j or the opposite). Overall the outcome of discussions is that even if it would be simpler to leak all our internals (it is not only logging there), it is not really possible because we adopted an isolated architecture (realms) cause plugins have to do their stuff and we should just support it so we should isolated everything but the mvn4 api (status quo for mvn3).
>
> side note: if this is no more assumed by most of community - I don't think it is the case but just highlighting what it means - we would have to drop the classrealm which wouldn't make much sense anymore and just request any plugin writer to fork for their work which rely on any other dependency than the ones we provide - don't think it is good but it is ultimately what we mean if we leak more and more.
SLF4j api has been exported for more than 13 years, I haven't traced back to the very beginning though...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225589776
> @michael-o right but it is not clear what we do of plexus - we are between being based on it and dropping it. Personally I tend to think we should rely on the abstraction we want to expose long term in a very stable way so either JUL (which can be used as an abstraction) or an org.apache.maven.somethingLogging but if we import plexus @asf it sounds like another valid option.
Do you expect SLF4J 2.x not to live 10 years?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] ceki commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
ceki commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225892740
@rmannibucau Since you mention `LocationAwareLogger`, the person who has been been publicly and loudly complaining about `LocationAwareLogger` for the last 12 years is the same person who asked for that breaking change in the first place. I am sure that is a bit hard to believe but please have a look at [SLF4J-118](https://jira.qos.ch/browse/SLF4J-118). The ticket is rather short and quite informative in its own right.
As for stability, perhaps Maven has a higher standard regarding API compatibility than SLF4J, as Maven really shines in that regard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1631926675
@gnodet let's ignore a second the classloading errors and take a simple functional case: you want logger X to be logged particularly and the app knows the provider it uses so can configure it directly - this is a common case as we discussed on slack. I'll use JUL binding for the example but it is not rare it is logback or log4j2 too.
Do we want to say to users "no, will never work until you implement another classloading abstraction to isolate yourself from plugin ecosystem"? Not on my side.
Here is a simple mojo showing it:
```pom.xml
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>1.7.36</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-jdk14</artifactId>
<version>1.7.36</version>
</dependency>
```
```java
@Mojo(name = "demo", defaultPhase = NONE)
public class MyMojo extends AbstractMojo {
@Override
public void execute() {
run();
}
private void run() {
final var loggerName = getClass().getName();
final var logger = Logger.getLogger(loggerName);
final var delegate = Logger.getLogger("");
logger.addHandler(new Handler() {
@Override
public void publish(final LogRecord record) { // just a hack to get console logger but not close it
delegate.getHandlers()[0].publish(record);
}
@Override
public void flush() {
// no-op
}
@Override
public void close() throws SecurityException {
// no-op
}
});
LoggerFactory.getLogger(loggerName)
.info("I am here");
}
}
```
Indeed you have to assume the logging configuration is not part of the execute method but a transitive one - a bit like spring logging system.
Running this mojo we get:
```
[INFO] --- demo:0.0.1-SNAPSHOT:demo (run) @ consumer ---
[INFO] I am here
```
Which is not really the expectation:
```
juil. 12, 2023 8:24:12 AM com.github.rmannibucau.maven.reproducer.Main main
INFO: I am in xml
juil. 12, 2023 8:24:12 AM com.github.rmannibucau.maven.reproducer.Main main
INFO: I am in xml
```
So we can see that the format is wrong - which can be an issue for downside parsing, the plugin will not enable to test grokking for ex, but the setup is not even respected in maven (the duplicated log is an intentional coding bug but does not surface with maven). Worse, maven ignores `logger.setUseParentHandlers(false);` code - which is intended to fix the duplicated line.
So maven does not enable to do any log related work there.
So I really think we should give the control of that to mojo:
* slf4j can be imported or not in the limit of the strict API - not the SPI
* JUL should likely be isolated like in tomcat per mojo - default = inherit but a toggle would enable to let the plugin use its own config/custom logging manager if the system prop is set there (by delegation we can do it)
The last trick we'll see - since we spoke of slf4j 2 - is that it does not use its classloading custom lookup anymore for the logger factory but a plain ServiceLoader JPMS based so ClassRealm is not ready for that at all. Guess we'll need to work on that at some point so I don't think we'll need a workaround in our SLF4J impl to delegate to plugin impl if any but means we will still have some issues with slf4j.
Hope it helps
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1842231553
Up, seems we are still un a broken state where we say to use slf4j for plugins and I didn't find the mvn4 API (pretty sure you did something @gnodet but didnt see it in mvn4 branch).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1842577827
@cstamas
> bet 99.999 of our users use logger just for that
99.99% uses Log facade (most of plugins just wire the logs to this interface since it is the only one in maven api today).
Just review github and our own mojo for ex.
Very few use slf4j since it was done in a very recent maven release and most of plugin still comply to older api too.
> Take away simplicity and availability of SLF4J API only due someone somewhere fiddles with logging implementations
Think you miss the fact SLF4J is an *implementation* for us and a detail we don't want to support for years cause as explained in the related threads slf4J is *not* stable and already bite several asf project so we must only promote *as api* something we control, this is a pure design and experience perspective.
If I'm following your reasoning we should just expose maven/lib to mojo and I agree it is trivial to agree this is bad but all libs are available and I don't think we use anything hard to consume so I don't think this reasoning can last very long.
> merge this PR to not encourage people to use slf4j since we agreed it was wrong a long time ago now
Ok, let me rephrase: "since using slf4j was agreed to be wrong, let's not deprecate the only API we have and control and we'll bridge to mvn4 api".
The "there was never consensus" was on "moving to slf4j as api", this was never discussed nor agreed so revert is the minimum we can do.
> For me this PR is bicycle shedding at it's best.
As explained this deprecation is a bug and was *not* intended so we should just fix it, after we can rediscuss what we do if you feel it was right but as of today this merge shouldn't have happent.
Agreement was on maven core code which can use slf4j as primary API, never ever on mojo - agreement was actually the opposite.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225385847
> @michael-o I don't have all the threads handy but we have several dev@ threads explaining it has several technical pitfalls (the way the API loads the impl - v1 or v2) and that we don't want to bind ourselves to an implementation we can't sustain in time. This PR just avoids to have to redefine yet another package and API for plugin (work Guillaume is doing) since we already have it and can enhance it with `Supplier` or alike for formatter like features - more efficient as of today.
I see, what I would like to avoid is to add features to this wrapper which will duplicate SLF4J at the end. We have this: Plexus Logging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1614186284
@slachiewicz if we switch we should just go to JUL IMHO (see the list for details) but not sure it would be for v4, hopefully v5. Anyway, does not change anything to the fact we don't want to show our impl to end users since one feature we provide is the hability to integrator to handle it as they want. All log libs have pitfalls and we can't make everything working (typically there are some cases with some bridge which will fail), so let's stay simple and 1. drop slf4j from consumers view in code, 2. if we need to change current impl move to JUL which enables to be dep free, abstract any impl (several asf projects do like cxf) and work in all integrator env without hacks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1613621466
> > We can not expect that all code in plugins only use Maven and JDK API, thirt party library used in plugins code can uses any logging framework.
>
> Sure and we comply to that, only thing is that end user can only expect maven logging framework to be integrated with maven ecosystem (think CI and maven customizations) so if the plugin provider wants something well integrated it has to use that. frontend maven plugin forwarded node logs to maven logging system for that to give an example.
>
> > It should not be important what logging framework is used in Maven core.
>
> Probably a good wish but factually it is.
>
> > Technically we can forward logging message from one framework to another and it can be transparent for plugins code.
>
> Good wish too but there are several cases which are not trivial. Personally I see it as limitations when deployed inside maven and most plugins have work arounds for that so all good, but we are not neutral enough (as the JVM is) to support anything. It is more or less the same than we are not 100% isolated between plugins - classloader is partially only, env is almost not isolated etc...
>
> Not sure about the spring reference but it is the same, they work in some env but not always and this is ok.
>
> The only thing maven guarantees is that the plugin API will go through maven logging system so if integration is customized it will get the logs, anything else is not guaranteed - except JUL by construction indeed.
If we:
* provide a simple abstraction for mojo (the current Log)
* export major APIs (commons-logging and slf4j
* redirect those, in addition to JUL
This should cover most basic cases.
If course, libraries that do setup specifically a logging framework, it will most certainly not be integrated, but we can't do much about that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1842559579
This thread is full of misconceptions. Slf4j API worked and works. I bet 99.999 of our users use logger just for that, for logging. What Romain is talking about is setting up your own logger implementation and so on.
First, BEFORE we do anything about this, can we see some numbers (like Mojos "just logging" vs "setting up their own loggers as Romain explains")? As I feel we have the situation of "it would be nice to do it", but the actual benefit would be beneficial only for 1-2 mojo coders?
Take away simplicity and availability of SLF4J API _only due someone somewhere fiddles with logging implementations_? Would be like forcing left-handed scissors on everyone, because there are left handed people out there (I am one of them).
As always, and since eons, if you want something super-duper-uber-smart stuff, go for it, just isolate it in your own classloader, no problem here.
For me this PR is bicycle shedding at it's best.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1844895027
IIRC, the example that was given by @rmannibucau was when a library actually requires a given implementation of slf4j. This happens sometimes when a library is configuring the underlying implementation programmatically.
@rmannibucau you should really come up with an IT so that we can investigate actual problems rather than never-ending discussions on principles.
Proposal for v4 plugins. In those case, I'm not sure what happens if we expose even the slf4j implementation.
https://github.com/apache/maven/pull/1336/files#diff-9600f93ce04acabe017b026dc1ebbb445e8cb363bb2677a764a783ab85cf804aR105-R116
I've also hit some problems with plexus logger which is now hidden, so it needs to be configured manually:
https://github.com/apache/maven-compiler-plugin/pull/147/files#diff-d4bac42d8f4c68d397ddbaa05c1cbbed7984ef6dc0bb9ea60739df78997e99eeR597-R598
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1613438697
> it's incomplete - we would also update maven site and say that slf4j introduced for logging in 3.1 is now obsolete?
It seems to me that https://maven.apache.org/maven-logging.html is mostly about the logging system in general, not really the api to use for mojos.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] slachiewicz commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "slachiewicz (via GitHub)" <gi...@apache.org>.
slachiewicz commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1615050966
For Maven 5 sounds good, hope we someone will deliver new better API and bridges to existing frameworks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1630988003
So the problem with exporting the slf4j unmodified is that things may break if a plugin depends on a library which itself imposes constraints on the slf4j binding used (by configuring it programmatically for example).
By exporting the maven logging api, we ensure plugins won't be affected, whatever libraries they have in their classloader.
It would be nice to have a project that reproduce the issues, as it's not yet clear what kind of failure would happen. @rmannibucau ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1842561917
> Ok so guess we should:
>
> 1. merge this PR to not encourage people to use slf4j since we agreed it was wrong a long time ago now
I find this silly and self contradicting: "we agreed to not use it" while it is you who said "there was never consensus". So which one? You cannot have both....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1845285830
@gnodet well yes and no, original export was partial and broken but makes 6-7 years it is bothering, but anyway does not change we don't want it to surface as a maven provided API from the previous discussions so deprecated was a missed 3 years ago IMHO. Until we get maven 4 "1.0.0" we should just deprecate Log and then switch to the provider Log of mvn 4 API, looks quite straight forward to me as path, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1842540338
> > Up, seems we are still un a broken state where we say to use slf4j for plugins and I didn't find the mvn4 API (pretty sure you did something @gnodet but didnt see it in mvn4 branch).
>
> https://github.com/apache/maven/blob/master/api/maven-api-core/src/main/java/org/apache/maven/api/plugin/Log.java
>
> I'm actually working on fixing some parts in the v4 plugins, including the class loader which should only include the Maven 4 api, so basically `org.apache.maven.api.*` and `jakarta.inject` packages.
However, not exposing slf4j would mean we should somehow provide a bridge from slf4j to `org.apache.maven.api.plugin.Log` so that plugins can use other libraries but still log to the maven infrastructure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225399117
@michael-o right but it is not clear what we do of plexus - we are between being based on it and dropping it. Personally I tend to think we should rely on the abstraction we want to expose long term in a very stable way so either JUL (which can be used as an abstraction) or an org.apache.maven.somethingLogging but if we import plexus @asf it sounds like another valid option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225590071
I'd be interested to head @ceki's opinion on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225954621
> You are mostly talking about impl, not API not. For the plugin dev.SLF4J simply works.
No @michael-o this is 100% the API sadly. We went through that several times on several apache projects as mentionned and as mentionned it is not only due to slf4j quality but also our own design so let's just do our homework and align on the dev@ discussion our code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1614177949
> Then we can also think how to support slf4j 1 and 2. More and more libs switch to v2
Afaik, the client api is compatible, if maven exports slf4j 2.x and provides an implementation, client should be fine.
See https://www.slf4j.org/faq.html#compatibility
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1844941097
@gnodet well code was shared but you can review https://github.com/rmannibucau/mojo-logging-demo which is only about 2 cases (but there are more like using alpha version of slf4j or the opposite). Overall the outcome of discussions is that even if it would be simpler to leak all our internals (it is not only logging there), it is not really possible because we adopted an isolated architecture (realms) cause plugins have to do their stuff and we should just support it so we should isolated everything but the mvn4 api (status quo for mvn3).
side note: if this is no more assumed by most of community - I don't think it is the case but just highlighting what it means - we would have to drop the classrealm which wouldn't make much sense anymore and just request any plugin writer to fork for their work which rely on any other dependency than the ones we provide - don't think it is good but it is ultimately what we mean if we leak more and more.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1844940065
This is a Draculan discussion. It sucks out everything.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1225364928
I still don't understand why we need a wrapper around a facade...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] slachiewicz commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "slachiewicz (via GitHub)" <gi...@apache.org>.
slachiewicz commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1613763443
Then we can also think how to support slf4j 1 and 2. More and more libs switch to v2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] slachiewicz commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by GitBox <gi...@apache.org>.
slachiewicz commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1237364037
it's incomplete - we would also update maven site and say that slf4j introduced for logging in 3.1 is now obsolete?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rmannibucau commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1613449431
yes, slf4j was never validated for mojo, there was an excess of enthusiasm in the javadoc but it was never validated nor ack-ed by the community so no big deal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] slawekjaranowski commented on pull request #793: [MNG-7532] Log shouldn't have been deprecated so ensure it is not
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1613588496
For Maven plugins we can provide own API or abstraction ... but users can use third party library for implementing plugin code.
We can not expect that all code in plugins only use Maven and JDK API, thirt party library used in plugins code can uses any logging framework.
It should not be important what logging framework is used in Maven core.
Technically we can forward logging message from one framework to another and it can be transparent for plugins code.
So I'm for not providing next abstraction but allow users use some of commonly available frameworks, like spring-boot does: https://docs.spring.io/spring-boot/docs/current/reference/html/features.html#features.logging
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1842530392
> Up, seems we are still un a broken state where we say to use slf4j for plugins and I didn't find the mvn4 API (pretty sure you did something @gnodet but didnt see it in mvn4 branch).
https://github.com/apache/maven/blob/master/api/maven-api-core/src/main/java/org/apache/maven/api/plugin/Log.java
I'm actually working on fixing some parts in the v4 plugins, including the class loader which should only include the Maven 4 api, so basically `org.apache.maven.api.*` and `jakarta.inject` packages.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-7532] Log shouldn't have been deprecated so ensure it is not [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #793:
URL: https://github.com/apache/maven/pull/793#issuecomment-1842680729
@cstamas please just check out "slf4j plugin api" in your mailbox and check the related 5-6 threads, there is *NO* consensus in the community about using slf4j in the *api* so this is not something we can promote. Agree the community is split - also why we must not promote slf4j yet for plugins.
So let's just drop the deprecation for now, and if you want we can reopen the debate but I kind of think maven 4 will close it by design, we all agreed maven 4 will abstract anything an user can rely on so there is no real point debating if slf4j is good or not post maven 4.
I also understand you put slf4j everywhere but on my side of the fence, slf4j is almost nowhere, so status is simple:
* yes slf4j works by a bug in extensions.xml for simple plugin
* slf4j does not generally works as a state
* we agreed to not aim at continuing slf4j (or any maven lib) support (by default - until the parent fist flag is configurable) in mojo
So not sure what is really to discuss to be honest, look like you fight for an already finished battle :thinking:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org