On 13 August 2018 at 14:57, Remko Popma <remko.po...@gmail.com> wrote: > If it was up to me I would use JUL in this particular case to keep the > library free of dependencies. JUL is reasonable since performance is not an > issue. > > Users that use a logging framework can redirect the JUL logging into their > log file with the JUL adapter of their logging library. > > But that’s just me.
+1, seems good to me as well. > Remko > > (Shameless plug) Every java main() method deserves http://picocli.info > >> On Aug 13, 2018, at 21:05, Bruno P. Kinoshita >> <brunodepau...@yahoo.com.br.INVALID> wrote: >> >> I was thinking more about using that as an argument for using a logging >> framework, but I think you are right. Probably slf4j/commons-logging + some >> default binding, then allowing users to change the implementation. >> >> Perhaps slf4j + log4j? >> >> >> Bruno >> >> ________________________________ >> From: Remko Popma <remko.po...@gmail.com> >> To: Commons Developers List <dev@commons.apache.org> >> Sent: Monday, 13 August 2018 11:55 PM >> Subject: Re: [imaging] IMAGING-154 remove Debug class >> >> >> >> For new projects I would really avoid using Commons Logging. Ceki had a >> point when he created SLF4J. >> >> >> The projects you mentioned that have a dependency on Commons Logging were >> started before SLF4J (and perhaps JUL) were created, I believe. >> >> >> >> (Shameless plug) Every java main() method deserves http://picocli.info >> >> >>>> On Aug 13, 2018, at 18:22, Bruno P. Kinoshita >>>> <brunodepau...@yahoo.com.br.INVALID> wrote: >> >>> Right now that's where I am standing too. If adding a dependency to log4j >>> is a no-no, then I'd probably check if jul would be OK, or otherwise maybe >>> import just Log4J's LowLevelLogUtil into the project would work? >> >> >> >>> Commons DBCP, Commons Configuration, Commons Beanutils, Commons JEXL, and >>> Commons Validator. All of these have a compile dependency to Commons >>> Logging. So it wouldn't be creating a new precedent. >> >> >> >>> Commons Pool and Commons Compress have some optional dependencies, but none >>> for logging... maybe an optional dependency, with disabling the logging by >>> default **could** work? >> >> >> >>> Cheers >> >>> Bruno >> >> >> >>> ________________________________ >> >>> From: Remko Popma <remko.po...@gmail.com> >> >>> To: Commons Developers List <dev@commons.apache.org> >> >>> Sent: Monday, 13 August 2018 9:50 AM >> >>> Subject: Re: [imaging] IMAGING-154 remove Debug class >> >> >> >> >>> If you want to avoid a dependency I would not create a custom logging >>> abstraction but just use JUL. Most logging libraries have JUL adapters so >>> clients can do the bridging on their side. >> >> >>> (Shameless plug) Every java main() method deserves http://picocli.info >> >> >>>> On Aug 13, 2018, at 0:17, Matt Sicker <boa...@gmail.com> wrote: >> >> >>>> What I've seen done when trying to avoid a logging API dependency is to >> >>>> create a minimal logging API purely for framework use. This can have a >> >>>> default System.err style implementation, but the idea is to make it easy >> >>>> (and performant hopefully) to bridge into the end user's choice of >> >>>> framework and configuration without actually requiring a real logging >> >>>> framework (at least until you want to use it in production). While it seems >> >>>> overkill, the problem is that neither JUL nor System.err are sufficient for >> >>>> logging. Even a simple API like Android's logging API can be good enough to >> >>>> abstract it in a small library. >> >> >>>> For a look at the very simplest route, you can see how Log4j2 handles >> >>>> logging before any logging classes have been initialized. It's basically a >> >>>> configurable wrapper around System.err: >> >>>> https://github.com/apache/logging-log4j2/blob/master/log4j-api/src/main/java/org/apache/logging/log4j/util/LowLevelLogUtil.java >> >> >> >>>>> On Sun, 12 Aug 2018 at 09:12, Gary Gregory <garydgreg...@gmail.com> wrote: >> >> >>>>> You could also log to a pluggable print stream which could be sys err by >> >>>>> default. Kind of like what JDBC allows. My bias is to Log4j 2 as well :-) >> >> >>>>> Gary >> >> >>>>>> On Sun, Aug 12, 2018, 08:00 Remko Popma <remko.po...@gmail.com> wrote: >> >> >>>>>> There’s a couple of considerations about doing logging in a library, but >> >>>>>> I’ll just mention a few: >> >> >>>>>> * dependencies >> >>>>>> * performance >> >>>>>> * ease of use >> >> >>>>>> * Dependencies* >> >>>>>> Will the library be less attractive to users if it requires an external >> >>>>>> dependency? Then don’t introduce one (so: use system err or JUL). On the >> >>>>>> other hand, if the vast majority of usages is in a context with many >> >>>>> other >> >>>>>> external libraries (like in a web container) you have more freedom. >> >> >>>>>> *Performance* >> >>>>>> Please take a look at the log4j 2 performance page ( >> >>>>>> https://logging.apache.org/log4j/2.x/performance.html#tradeoffs). >> >>>>> Console >> >>>>>> logging is 50x (yes fifty times) slower than file logging. >> >>>>>> That’s a strong argument against system err logging. I’m not a fan of >> >>>>> JUL, >> >>>>>> but if you need to avoid dependencies you’re better off using JUL, that’s >> >>>>>> only 5x slower than log4j. Also depends on how much logging you expect to >> >>>>>> do in the worst case. >> >> >>>>>> *Ease of use* >> >>>>>> I’m biased and would say that Log4j 2 has the nicest and richest API. >> >>>>>> Console logging (System.err.printf) probably has the poorest API. Other >> >>>>>> libraries sit in the middle. >> >> >>>>>> *Final note* >> >>>>>> I would never log to System out, always use system err instead. This >> >>>>>> allows programs using your library to pipe output to other programs >> >>>>> without >> >>>>>> their output getting mixed with your library’s diagnostic output. >> >> >>>>>> Hope this helps, >> >> >>>>>> Remko >> >> >>>>>> (Shameless plug) Every java main() method deserves http://picocli.info >> >> >>>>>>> On Aug 12, 2018, at 21:21, Gilles <gil...@harfang.homelinux.org> >> >>>>> wrote: >> >> >>>>>>> Hello Bruno. >> >> >>>>>>>> On Sun, 12 Aug 2018 08:56:37 +0000 (UTC), Bruno P. Kinoshita wrote: >> >>>>>>>> Hi all, >> >> >> >>>>>>>> I commented on IMAGING-154, but copying the last comment here as it >> >>>>>>>> contains the approach I would like to follow to fix the last change in >> >>>>>>>> the project blocking >> >>>>>>>> a 1.0 release: >> >> >> >>>>>>>> --- >> >> >>>>>>>> So went ahead to re-design the Debug class, in a way users could >> >>>>>>>> still enable/disable debugging, and also use a PrintStream so that >> >>>>>>>> other thing rather than System.out could be used. >> >> >> >> >>>>>>>> Then, realized removing System.out was the natural next step. But >> >>>>>>>> alas, the library uses System.out for debugging, but sometimes it uses >> >>>>>>>> it for writing to System.out in a "verbose mode". What is more >> >>>>>>>> complicated, is that sometimes classes methods like `toString()` are >> >>>>>>>> calling debug methods that receive a PrintStream already. >> >> >> >> >>>>>>>> So I spent some more time quickly comparing what other libraries I've >> >>>>>>>> seen being used / or used for image processing: >> >> >> >>>>> https://kinoshita.eti.br/2018/08/12/use-of-logging-in-java-image-processing-libraries/ >> >>>>>> . >> >>>>>>>> Turns out only very low level libraries, such as the JNI bridge for >> >>>>>>>> OpenCV, im4java, and Java's ImageIO can do with just throwing >> >>>>>>>> Exception's. >> >> >> >> >>>>>>>> All other libraries have one way or another of logging. Be it with >> >>>>>>>> JUL, SLF4J, custom loggers, or with the ol' System.out/err. >> >> >> >> >>>>>>>> My preferred compromise for this ticket was to keep Debug, making >> >>>>>>>> System.out possible but optional, and mark the class internal only. >> >>>>>>>> Now my preferred solution is to keep the Debug internal, but add a >> >>>>>>>> logger to it. And then also add logging to replace where System.out is >> >>>>>>>> used for the "verbose" mode. >> >>>>>>>> --- >> >> >> >> >>>>>>>> Any thoughts? Objections? If none, I will try to work on this issue >> >>>>>>>> next weekend, making the Debug class internal only, and replacing >> >>>>>>>> System.out by a logging utility. After that, we should be good to >> >>>>>>>> start preparing the vote for 1.0. >> >> >> >> >>>>>>>> * I know it's hard to get a consensus on having logging in Commons >> >>>>>>>> components, as we have normally low level libraries, where using >> >>>>>>>> logging is not always practical. >> >> >>>>>>> There are Log4j2 experts reading here. It would be interesting >> >>>>>>> to hear them about what is practical or not. There are several >> >>>>>>> aspects to "practical": simplicity, flexibility, compatibility, >> >>>>>>> performance, ... >> >>>>>>> How does Log4j2 fare in these areas? >> >>>>>>> Is there a known (through experience) limit in where it should >> >>>>>>> be used? >> >> >>>>>>>> But I would now argue that Java own >> >>>>>>>> ImageIO is low level. But ImageJ2, Processing, OpenJPEG, and Commons >> >>>>>>>> Imaging are located at a higher level, some times even using it for >> >>>>>>>> basic image handling/parsing/reading. >> >> >>>>>>> As with many discussions on this list, conflicting arguments occur >> >>>>>>> because people lack common (!) definitions. >> >>>>>>> So one goes: "You cannot do <something> in a low-level component" >> >>>>>>> but does not define "low-level"... >> >> >>>>>>>> * Feel free to cast a counter-argument for it, but please think >> >>>>>>>> whether you'd still be -0, +0 for this change. We have delayed 1.0 for >> >>>>>>>> a while, so if you have a strong opinion on not adding a logger, >> >>>>>>>> please provide an alternative for IMAGING-154. >> >> >>>>>>>> Otherwise we may fail >> >>>>>>>> to prepare a 1.0 release yet again, and then somebody else may have to >> >>>>>>>> work on it in a few months/years... >> >> >>>>>>> We are there because the project is too rigid about itself as >> >>>>>>> a whole (cf. for example the [RNG] thread about BC). >> >>>>>>> IMHO, it's not the always least common denominator that is the >> >>>>>>> best decision... >> >>>>>>> As you noticed, components most easily stall in their development >> >>>>>>> for lack of proper review, or risk acceptance (i.e. assume that >> >>>>>>> those who are closer to the code (at a given time) probably know >> >>>>>>> best... :-/ >> >> >>>>>>> My opinion is that we can take the risk to introduce logging, and >> >>>>>>> if people complain somehow, take it back later. >> >> >>>>>>>> one possible compromise for this, >> >>>>>>>> might be i) make Debug internal, >> >> >>>>>>> +1 >> >> >>>>>>> [Hmm... Does "internal" mean that minor release can break BC >> >>>>>>> on such a class?] >> >> >>>>>>>> ii) remove all System.out calls, >> >> >>>>>>> +1 or >> >>>>>>> -1 >> >> >>>>>>> [Depends on what "low-level" means here. "stdout"/"stderr" is >> >>>>>>> indeed used in low-level utilities but is the intent the same >> >>>>>>> here?] >> >> >>>>>>>> which means removing the verbose flags, the checks, and calls to >> >>>>>>>> System.out in there. >> >> >>>>>>> It would be a loss of potentially useful information (e.g. for >> >>>>>>> debugging). >> >> >>>>>>> Regards, >> >>>>>>> Gilles >> >> >> >>>>>>>> Thanks! >> >>>>>>>> Bruno >> >> >>>>>>>> ________________________________ >> >>>>>>>> From: Bruno P. Kinoshita <brunodepau...@yahoo.com.br.INVALID> >> >>>>>>>> To: Commons Developers List <dev@commons.apache.org> >> >>>>>>>> Sent: Tuesday, 6 February 2018 11:30 PM >> >>>>>>>> Subject: Re: [imaging] IMAGING-154 remove Debug class >> >> >> >> >>>>>>>> Hi sebb, >> >> >>>>>>>>> Another aspect of debugging is ensuring that methods are small and >> >> >>>>>>>>> easily tested independently. >> >>>>>>>>> However this is difficult to do, and care must be taken to ensure >> >>>>> that >> >>>>>>>>> the public API is not unnecessarily extended.. >> >> >>>>>>>> A very good point. >> >> >>>>>>>> The parsers in commons-imaging expose some #dump... methods >> >>>>>>>> ( >> >> >>>>> https://github.com/apache/commons-imaging/blob/7e7f96857df999175bb614732e13272a82f7962a/src/main/java/org/apache/commons/imaging/ImageParser.java#L794 >> >>>>>> ). >> >> >>>>>>>> While I can see that parsers may need to dump the data they are >> >>>>>>>> holding in some structured way for inspecting, reporting, serializing, >> >>>>>>>> etc, it looks like some other classes were affected by it too. For >> >>>>>>>> example... >> >> >> >>>>>>>> A JPEG Segment has a #dump() method >> >> >> >> >> >>>>> https://github.com/apache/commons-imaging/blob/7e7f96857df999175bb614732e13272a82f7962a/src/main/java/org/apache/commons/imaging/formats/jpeg/segments/Segment.java#L34 >> >> >> >>>>>>>> which gets defined in each subclass of Segment. It can be confusing >> >>>>>>>> to have a method such as #dump() in a Segment, from the point of view >> >>>>>>>> of someone writing a photo editor for example. The user could use that >> >>>>>>>> to pass his/her own logger's PrintWriter, which would make removing or >> >>>>>>>> changing logging in the future in commons-imaging. >> >> >> >>>>>>>> If we keep the Debug class, and make it internal, there would still >> >>>>>>>> be these methods to take care. And there are some methods where users >> >>>>>>>> can provide a PrintWriter, while others instead use System.out >> >>>>>>>> (e.g. >> >> >>>>> https://github.com/apache/commons-imaging/blob/7e7f96857df999175bb614732e13272a82f7962a/src/main/java/org/apache/commons/imaging/FormatCompliance.java#L70 >> >>>>>> ). >> >> >>>>>>>> Cheers >> >>>>>>>> Bruno >> >> >>>>>>>> ________________________________ >> >>>>>>>> From: sebb <seb...@gmail.com> >> >>>>>>>> To: Commons Developers List <dev@commons.apache.org>; Bruno P. >> >>>>>>>> Kinoshita <brunodepau...@yahoo.com.br> >> >>>>>>>> Sent: Tuesday, 6 February 2018 11:06 PM >> >>>>>>>> Subject: Re: [imaging] IMAGING-154 remove Debug class >> >> >> >> >>>>>>>> On 6 February 2018 at 09:52, Bruno P. Kinoshita >> >>>>>>>> <brunodepau...@yahoo.com.br.invalid> wrote: >> >>>>>>>>> Hi Jorg, >> >> >>>>>>>>> I'd be fine with that solution too. I think this one would cause the >> >>>>>> smaller change to the code as is. >> >> >>>>>>>>> I believe my preference is still to remove the Debug class. But >> >>>>>> between logging and making Debug internal only, I'd choose making it >> >>>>>> internal. >> >> >>>>>>>> +1 >> >> >>>>>>>> I think making it internal means it can still be dropped later. >> >> >>>>>>>>> Looking forward to hearing what others think about these options. >> >> >> >>>>>>>> Another aspect of debugging is ensuring that methods are small and >> >>>>>>>> easily tested independently. >> >>>>>>>> However this is difficult to do, and care must be taken to ensure that >> >>>>>>>> the public API is not unnecessarily extended.. >> >> >>>>>>>>> Thanks >> >>>>>>>>> Bruno >> >> >> >>>>>>>>> ________________________________ >> >>>>>>>>> From: Jörg Schaible <joerg.schai...@bpm-inspire.com> >> >>>>>>>>> To: dev@commons.apache.org >> >>>>>>>>> Sent: Tuesday, 6 February 2018 9:24 PM >> >>>>>>>>> Subject: Re: [imaging] IMAGING-154 remove Debug class >> >> >> >> >>>>>>>>> Hi Bruno, >> >> >> >>>>>>>>> if it might also be helpful to our users, why not keep and provide >> >>>>> it. >> >>>>>> As >> >> >>>>>>>>> I understand it, the Debug class is a tool helping in development to >> >> >>>>>>>>> analyze some behavior. >> >> >> >>>>>>>>> Nothing stops us from declaring this class internal (we might even >> >>>>> put >> >>>>>> it >> >> >>>>>>>>> into a package "internal" or "debug") that might be changed without >> >> >>>>>>>>> further comment. Nobody may rely on it in production code, but during >> >> >>>>>>>>> development it might be helpful. With such an approach we might not >> >>>>>> have >> >> >>>>>>>>> a need to find a better interface to provide this functionality. >> >> >> >>>>>>>>> Just my 2¢, >> >> >>>>>>>>> Jörg >> >> >> >> >>>>>>>>> Am Mon, 05 Feb 2018 12:20:58 +0000 schrieb Bruno P. Kinoshita: >> >> >> >>>>>>>>>> Hello, >> >> >> >> >>>>>>>>>> If memory serves me well, some time ago we had a discussion around >> >> >>>>>>>>>> sanselan & commons-imaging 1.0. One of the issues with >> >>>>> commons-imaging >> >> >>>>>>>>>> 1.0 was the Debug class. >> >> >> >> >>>>>>>>>> https://issues.apache.org/jira/browse/IMAGING-154 >> >> >> >> >>>>>>>>>> I finished the pull request, but Gilles raised an important point, >> >>>>>> about >> >> >>>>>>>>>> discussing other alternatives first. >> >> >> >> >>>>>>>>>> Initially I am against logging in low level libraries, especially >> >> >>>>>>>>>> commons components. But some time ago I had to debug TIFF issues in >> >> >>>>>>>>>> commons-imaging, and having the dump methods was a tremendous help. >> >> >> >> >> >> >>>>>>>>>> The issue is that some imaging algorithms/processing have a lot of >> >> >>>>>>>>>> variables that can be altered. And keeping an eye on all of them in >> >>>>>> the >> >> >>>>>>>>>> debugger can be quite hard - though not impossible. >> >> >> >> >>>>>>>>>> So all in all, now I am more confident to proceed without the Debug >> >> >>>>>>>>>> class. But some users could have a hard time investigating possible >> >> >>>>>>>>>> issues in the library without seeing what's going on within the >> >>>>>> library. >> >> >> >> >>>>>>>>>> IMO, that could be solved with the logging/dump features... or >> >>>>>> through a >> >> >>>>>>>>>> better design, especially around exception handling/throwing. The >> >>>>>> latter >> >> >>>>>>>>>> is my preferred approach. Instead of logging, I prefer - whenever >> >> >>>>>>>>>> possible - that low level libraries throw exceptions and let me >> >>>>> handle >> >> >>>>>>>>>> the logging. >> >> >> >> >> >> >>>>>>>>>> So, any thoughts? :) I'm +1 to remove the Debug class, and +0 to a >> >> >>>>>>>>>> logging added to commons-imaging. >> >> >> >> >>>>>>>>>> Bruno >> >> >> >>>>>>> --------------------------------------------------------------------- >> >>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> >>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >> >> >> >> >> >> >>>> -- >> >>>> Matt Sicker <boa...@gmail.com> >> >> >> >>> --------------------------------------------------------------------- >> >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> >>> For additional commands, e-mail: dev-h...@commons.apache.org >> >> >>> --------------------------------------------------------------------- >> >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> >>> For additional commands, e-mail: dev-h...@commons.apache.org >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> >> For additional commands, e-mail: dev-h...@commons.apache.org >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
