vlsi commented on code in PR #129:
URL: https://github.com/apache/xalan-java/pull/129#discussion_r1405325831
##########
serializer/src/main/java/org/apache/xml/serializer/Version.java:
##########
@@ -55,7 +55,16 @@ public final class Version
private static void readProperties() {
Properties pomProperties = new Properties();
- try (InputStream fromResource =
Version.class.getClassLoader().getResourceAsStream(POM_PROPERTIES_PATH)) {
+ ClassLoader classLoader = Version.class.getClassLoader();
+ if (classLoader == null) {
+ // Oops! Someone put Xalan is on the bootstrap class loader (BCL) -> fall
+ // back to the system class loader, because there is no Classloader
+ // instance for the BCL (native code). Due to class loader hierarchy,
+ // however, the resource will also be found when asking for it from a
+ // level below the BCL.
+ classLoader = ClassLoader.getSystemClassLoader();
+ }
+ try (InputStream fromResource =
classLoader.getResourceAsStream(POM_PROPERTIES_PATH)) {
Review Comment:
@kriegaex , if I ask “use this exact code” you will definitely blame for me
asking you to do something in “exactly the way I like”. Sure there might be
unexpected issues. Of course Class.getResource.. works, but who knows what you
tested else since you added exactly zero tests, and there was exactly zero CI
executions. That is why I asked if you considered class-level
getResourceAsStream.
If you failed to get it work, then just say so. It is frustrating when you
blame me for both "asking to follow an exact route" and "asking if you
considered a given workable alternative".
##########
serializer/src/main/java/org/apache/xml/serializer/Version.java:
##########
@@ -55,7 +55,16 @@ public final class Version
private static void readProperties() {
Properties pomProperties = new Properties();
- try (InputStream fromResource =
Version.class.getClassLoader().getResourceAsStream(POM_PROPERTIES_PATH)) {
+ ClassLoader classLoader = Version.class.getClassLoader();
+ if (classLoader == null) {
+ // Oops! Someone put Xalan is on the bootstrap class loader (BCL) -> fall
+ // back to the system class loader, because there is no Classloader
+ // instance for the BCL (native code). Due to class loader hierarchy,
+ // however, the resource will also be found when asking for it from a
+ // level below the BCL.
+ classLoader = ClassLoader.getSystemClassLoader();
+ }
+ try (InputStream fromResource =
classLoader.getResourceAsStream(POM_PROPERTIES_PATH)) {
Review Comment:
@kriegaex , if I ask “use this exact code” you will definitely blame for me
asking you to do something in “exactly the way I like”. Sure there might be
unexpected issues. Of course Class.getResource.. works, but who knows what you
tested else since you added exactly zero tests, and there was exactly zero CI
executions. That is why I asked if you considered class-level
getResourceAsStream.
If you failed to get it work, then just say so. It is frustrating when you
blame me for both "asking to follow an exact route" and "asking if you
considered a given workable alternative".
--
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: dev-unsubscr...@xalan.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@xalan.apache.org
For additional commands, e-mail: dev-h...@xalan.apache.org
