New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update eXist to use Log4j 2 #527
Conversation
Damn, @adamretter is on fire!! |
It would be good to add the Log4j 1.2 bridge jar to give people time to update their existing modules. |
I agree... |
For backward compatibility sake :/ |
@wolfgangmm @dizzzz People can add the log4j 1.2 bridge to their modules, so I don't think we should add it to the core, otherwise it is likely it will be used by mistake. |
Hmm no that would be a source of library conflicts. |
@dizzzz why? |
log4j is probably used by every module, so you would end up with 4 to 5 copies in a normal install. If we can make migration easy at low cost, we should do so. |
@wolfgangmm but that problem already exists right: If an external Module A depends on the same library as external Module B, then you have two copies of the same module. Adding all libraries to eXist is not a good way to solve this issue. Also I am trying to work toward the next version of eXist, hence this work happening in |
@adamretter how did you make the change? Using some scripting? |
@dizzzz No it was done by hand today. |
I have thinking about this a few times.... Log4j2 is really an improvement |
We have full control over what goes into core, so there's no risk of people using old log4j for core code, because we'll see it. I don't understand where the problem is with adding one jar which is easily available. On the other hand, quite a few people already complained to me about unexpected and unannounced changes to the code base. It's ok to force people to update their stuff one or two times during a development cycle, but if it happens more often, they get really annoyed (that's my hands on impression right now). Incompatible changes should be done in a concentrated effort and should ideally be announced, so less experienced devs know what to do. |
@wolfgangmm I agree but it seems to me that that forced update happens when users move from 2.2 to 2.3 (or 3.0) which will only happen once. If this is really a concern, perhaps you or @dizzzz could send a pull-request to add the bridge to |
so the current |
@@ -27,7 +27,9 @@ | |||
package org.exist.extensions.exquery.restxq.impl; | |||
|
|||
import java.util.List; | |||
import org.apache.log4j.Logger; | |||
import org.apache.logging.log4j.LogManager; | |||
import org.apache.logging.log4j.LogManager; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double import?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the double import of LogManager is in many files....
For the rest... it looks like a straight forward update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I will fix those now...
@dizzzz I had assumed that the next release would be a 3.0 due to the massive amount of changes, and addition of support for XQuery 3.1 Arrays. |
@dizzzz I just fixed the double-imports and rebased. |
v2.2 vs v3.0 ; assumptions are very dangerous :-) there was not an agreement made on this yet. |
I am OK with this PR...... @wolfgangmm @ljo @shabanovd ? |
me too |
darn, there is the first hit:
causing exist-db not to start but... I can't remove monex either...... |
so we need to have this bridge any, asap; unfortunately I do not have any github access, and the log4j2 download links (2.1) seemto be dead, all? |
Sorry, how did you get that error? I have been running this code a lot here On 19 March 2015 at 10:31, Dannes Wessels notifications@github.com wrote:
Adam Retter skype: adam.retter |
Installed monex as written before |
@adamretter please cold you add the bridge jars well? if ppl have monex installed,the database will not start any more. That reflects very bad on us..... |
-----BEGIN PGP SIGNED MESSAGE----- Den 2015-03-19 11:19, Dannes Wessels skrev:
I agree, but I want the bridge, since another round updating all xar -----BEGIN PGP SIGNATURE----- iD8DBQFVCrb9hcIn5aVXOPIRAq5BAJ9Q96CZZiUGLazXoPtrpBT1REz8xwCeJIz1 |
@dizzzz I won't add the bridge as I strongly believe it is the wrong thing to do. Everything about building modular software over the years screams at me that there is a dependency management issue here which is solely the responsibility of the 3rd party module. Of course, if you or anyone else wishes to add the bridge to eXist you are more than welcome... although I am not sure where you would put it in the eXist codebase so that it makes sense, i.e. it is certainly not a |
@dizzzz Where should such a jar which is only there for external 3rd party modules be placed? |
|
@dizzzz Could you not make the pull-request yourself this evening? |
-----BEGIN PGP SIGNED MESSAGE----- Den 2015-03-19 13:32, Adam Retter skrev:
I don't see this clear cut separation in the case of logging, we expose -----BEGIN PGP SIGNATURE----- iD8DBQFVCtNqhcIn5aVXOPIRAiX1AJ424p8BL9BrsUns2FBKj2IewaM30wCfQ2m2 |
@ljo you are missing the point. We expose an API, if a module depends on a library it is it's business to ship with or obtain that library... otherwise in the extreme we will end up including the Internet. |
-----BEGIN PGP SIGNED MESSAGE----- Den 2015-03-19 14:58, Adam Retter skrev:
@adamretter, in simple letters no, feature libraries, yes, but -----BEGIN PGP SIGNATURE----- iD8DBQFVCuvQhcIn5aVXOPIRAqPXAJ9wx4t1Gt9Eo4WU0MQ9G/J838baBQCgjnLx |
@dizzzz My response was not meant to be harsh, but it did include some sarcasm. I have been working on eXist self-funded almost full-time since September 2014. You will have noticed the increased output from me and that 99% of my pull-requests are focused on fundamental improvements to eXist (cleanup, refactoring, modernisation) as opposed to adding features. I have become very downhearted about the vast quantity of legacy code in eXist and the culture of building temporary patch on top of temporary patch. I am trying to get away from that, I no longer want to put tape over the cracks, instead I am investing my own time and money into really trying to make eXist better. I am also limiting myself to doing this in small steps so that these changes can be digested and accepted by the eXist developers and community; Some days I really want to just rip into the code base! I am against adding this jar, because the reasoning behind adding it represents the true bigger problem, i.e. it is a temporary patch which ignores the real problem (i.e. dependency management in external modules). As we all know from our years working on eXist, temporary fixes have a nasty habit of sticking around for years even when deprecated. There is much code in eXist that it feels impossible to remove because someone somewhere might be using it (although in reality no one may actually be using it, or the cost trade-off isn't worth the effort). Much of this old code has to be maintained by us at great cost and really holds back the advancement of the project. I am not going to invest my time and money into adding yet another temporary patch to eXist (in this case a non-dependent jar) which will stay around and not be removable in future. It is just more sticky-tape to maintain, and I don't want to make my life harder and I won't take eXist backwards. eXist is a community project, so if someone wants to add this jar and maintain its lifecycle then they are welcome to do so and I won't stand in the way. I will though casually point out to them, that their time would be better invested by fixing the real underlying issue so that all may benefit. I hope that makes sense. Nothing in this thread is intended personally toward anyone. I just want to do better and I want eXist to be better. I am certain that everyone wants to head in that direction also, but we each have different personal approaches to this. As you have already pointed out my perfectionism, my own approach which was taught to me at a very young age, goes like so: "If something is worth doing, do it properly!" |
@ljo Sure I exaggerated, but my point is that it is a slippery slope! For example, I bet if you wrote a plugin for Apache HTTPD and complained to the httpd project that they didn't include the particular logging library you were using in their code-base they would tell you it is not their problem to solve. If you really want this bridge jar then send a pull-request and I am sure that someone in the project will merge it. |
@dizzzz Monex is an external module. It is not part of the eXist project on GitHub, right? Otherwise I would have fixed it when I fixed all the other code. Also you are assuming everyone uses Monex. Whilst I like Monex and I can see the use in it, I don't use it and I know others who also don't use it. eXist the XML Native Database works just fine, it compiles, builds and runs from source code, and passes its test-suite. I am sure the Monex project can make a new release of the module when the next version of eXist comes out right? Also I don't think you can be too disappointed, I have been entirely clear from before the pull-request was merged that I would not add this bridge jar. I have not changed my position. I have also made it clear that if anyone wants to add it by pull-request, then they are more than welcome. |
so.... my proposal is:
this way we will buy time for developers to migrate (and not let them face with these issues). We just cannot expect developers to upgrade XARs without pre-announcements, it just costs time to prepare these changes. It is unacceptible that an install breaks spontaneously, under all costs! |
The monex extension:........ is developed by our team..... released by our team; use by important customers.... |
-----BEGIN PGP SIGNED MESSAGE----- Den 2015-03-19 18:25, Adam Retter skrev:
@adamretter you are unfortunately distorting our arguments again. This Let me put a middle ground suggestion for you all since the leanisation -----BEGIN PGP SIGNATURE----- iD8DBQFVC9dyhcIn5aVXOPIRAoqTAJ9o6ir+G3n/kddWlJ9XBDilchMdHgCfQhHx |
For the sake of my health, I have switched off all github notifications yesterday. This thread gave me the impression my contributions to this project are no longer welcome and my work is not valuable. I had a bad night because of it. Updating monex or other expath packages is simply not possible at this point, due to flaws in eXist core. I was working on a fix, but my motivation is gone entirely. |
Adam (and all), i think this thread has been gone out a bit out of control. While i personally really appreciate your engagement very much and also Managing an open source project is tricky business and is a bit like Step-by-step improvements are often a much more demanding task than to Regarding Monex - this is surely not an arbitrary external module. It So, please move on but be kind to the people that aren't as fast as you Joern Am 20.03.15 um 09:16 schrieb ljo:
|
@wolfgang That was not the impression I was trying to convey. Your contributions are of course appreciated. My point is that eXist has an API for plugging in external modules. Those modules are external. What I mean when I say they are not a part of core eXist, is that I mean they are not in https://github.com/exist-db/exist. In fact they are not even in https://github.com/exist-db at all. We cannot go searching out every possible third-party module to test it against any change that is made in eXist, that is entirely unreasonable and futile. If the modules are not in the eXist repo, you should consider that I don't know about them... which is fine because they use the external module API for god-sakes anyway. Dannes and Leif and I obviously have different opinions on this issue. I have patiently listened to everyones arguments and tried to promote good software engineering principles. However I have not seen a good argument which would change my mind. We still disagree and that is fine. I have throughout said that others are welcome to disagree and send a pull-request. Anyone is still free to do that. Personally, I am not going to do work which I think is wrong, and that opinion is valid. I don't expect others to do things either that they are not comfortable with. Regardless, I have found this thread tedious and time consuming. So rather than respond to the most recent comments this morning, which seem to just say the same things over again. I used the time constructively instead to update the two modules that I know @dizzzz and @wolfgangmm care most about i.e. Monex and Mongrel. I would suggest, if these are considered core modules, they should not be on personal GitHub repos and should in the least be moved to the eXist-db organisation to reflect that. https://github.com/wolfgangmm/monex/pull/15 https://github.com/dizzzz/Mongrel/pull/36 Fixing the logging issue in Monex took me 12 minuites and in Mongrel just 4 minutes. The fix is backwards compatible with older versions of eXist too, so I don't expect any complaints there! For good measure I also learnt some Ivy and added dependency management to Monex. @dizzzz This may interest you for Mongrel as this is simpler than your Ivy approach in so far as the bootstrapping is built in and you don't need to first run https://github.com/wolfgangmm/monex/pull/16 In total fixing the issues here took me just under 1 hour, that includes learning your project builds and code-bases. That is considerably less time than was wasted in the dealing with this thread of conversation. Enjoy! |
Log4j version 2 brings many advantages:
java.util.logging
) to log4j. This is very useful for projects like EXPath Repo which otherwise spews its logging to the console. It now logs to$EXIST_HOME/webapp/WEB-INF/logs/expath-repo
.