[GitHub] [nifi] tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library loading fixed/improved

2019-11-22 Thread GitBox
tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library 
loading fixed/improved
URL: https://github.com/apache/nifi/pull/3894#discussion_r349570247
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NativeLibFinder.java
 ##
 @@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.nar;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+
+public interface NativeLibFinder {
 
 Review comment:
   I think all 3 solutions (member, abstract superclass, trait interface) have 
their pros and cons.
   In my opinion the trait interface is the best usually, but it's hard to tell.
   In this case probably the abstract superclass wins out.
   
   **Member (composition)**
   Pros
   - Can be reused
   - Further abstraction and extension not limited
   - Proper isolation
   - State code can be written once
   
   Cons
   - More glue code (plus one field, instantiation, data transfer)
   - Need to make sure states are properly set up in order. (Must not reference 
the member before it's set up properly. Can be mitigated if 
immutable/stateless.)
   
   **Abstract superclass (Regular inheritance)**
   Pros
   - Least amount of glue code
   - Proper isolation
   - State setup is usually not an issue
   - State code can be written once
   - State can be accessed anywhere
   
   Cons
   - Reusability it very restricted (probably not an issue in this case)
   - Restricts further abstraction and extension
   
   **Trait interface (Behavioural inheritance)**
   Pros
   - Minimal/Least amount of glue code
   - Can be reused
   - Further abstraction and extension not limited
   - No issues with state
   
   Cons
   - Isolation is very poor (everything is public)
   - Can't have state (although can be mitigated by defining and referencing 
accessors)
   
   All in all, abstract superclasses have their well-known issues. Trait 
interfaces are basically composition 2.0 except Java doesn't really implement 
this concept very well. The thing is, no matter the use-case, these issues with 
the trait interfaces will be there. So we basically either accept them with 
their flaws as a viable approach or never use them at all. I think the cons are 
not that bad.
   
   However in this case adding another classloader superclass fits in the 
existing picture well so I'll go with 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [nifi] tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library loading fixed/improved

2019-11-22 Thread GitBox
tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library 
loading fixed/improved
URL: https://github.com/apache/nifi/pull/3894#discussion_r349586981
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NarClassLoader.java
 ##
 @@ -116,10 +122,13 @@
  * Maven NAR plugin will fail to build the NAR.
  * 
 
 Review comment:
   I'll update the docs.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [nifi] tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library loading fixed/improved

2019-11-22 Thread GitBox
tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library 
loading fixed/improved
URL: https://github.com/apache/nifi/pull/3894#discussion_r349557277
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NativeLibFinder.java
 ##
 @@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.nar;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+
+public interface NativeLibFinder {
+Logger LOGGER = LoggerFactory.getLogger(NativeLibFinder.class);
+
+String OS = System.getProperty("os.name").toLowerCase();
+
+List getNativeLibDirs();
+
+Map getNativeLibNameToPath();
+
+String getTmpLibFilePrefix();
+
+default Set getUsrLibDirs() {
+Set usrLibDirs = 
Arrays.stream(getJavaLibraryPath().split(File.pathSeparator))
+.map(String::trim)
+.filter(pathAsString -> !pathAsString.isEmpty())
+.map(File::new)
+.map(toDir())
+.filter(Objects::nonNull)
+.collect(Collectors.toSet());
+
+return usrLibDirs;
+}
+
+default String getJavaLibraryPath() {
+return System.getProperty("java.library.path", "");
+}
+
+default Function toDir() {
+return file -> {
+if (file.isFile()) {
+return file.getParentFile();
+} else if (file.isDirectory()) {
+return file;
+} else {
+return null;
+}
+};
+}
+
+default String findLibrary(String libname) {
+Path libLocation = getNativeLibNameToPath().compute(
+libname,
+(__, currentLocation) ->
+Optional.ofNullable(currentLocation)
+.filter(Objects::nonNull)
+.filter(location -> location.toFile().exists())
+.orElseGet(
+() -> getNativeLibDirs().stream()
+.map(nativeLibDir -> 
findLibrary(libname, nativeLibDir))
+.filter(Objects::nonNull)
+.findFirst()
+.map(libraryOriginalPathString 
-> createTempCopy(libname, libraryOriginalPathString))
+.orElse(null)
+)
+);
+
+String libLocationString = Optional.ofNullable(libLocation)
+.map(Path::toFile)
+.map(File::getAbsolutePath)
+.orElse(null);
+
+return libLocationString;
+}
+
+default Path createTempCopy(String libname, String 
libraryOriginalPathString) {
+Path tempFile;
+
+try {
+tempFile = Files.createTempFile(getTmpLibFilePrefix() + "_", "_" + 
libname);
+Files.copy(Paths.get(libraryOriginalPathString), tempFile, 
REPLACE_EXISTING);
+} catch (Exception e) {
+LOGGER.error("Couldn't create temporary copy of the library '" + 
libname + "' found at '" + libraryOriginalPathString + "'", e);
+
+tempFile = null;
+}
+
+return tempFile;
+}
+
+default String findLibrary(String libname, File nativeLibDir) {
+final File dllFile = new File(nativeLibDir, libname + ".dll");
+final File dylibFile = new File(nativeLibDir, libname + ".dylib");
+final File libdylibFile = new File(nativeLibDir, "lib" + libname + 
".dylib");
+final File libsoFile = new File(nativeLibDir, "lib" + libname + ".so");
+final File soFile = new 

[GitHub] [nifi] tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library loading fixed/improved

2019-11-22 Thread GitBox
tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library 
loading fixed/improved
URL: https://github.com/apache/nifi/pull/3894#discussion_r349586673
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/test/java/org/apache/nifi/nar/NativeLibFinderTest.java
 ##
 @@ -0,0 +1,566 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.nar;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+import static org.mockito.MockitoAnnotations.initMocks;
+
+public class NativeLibFinderTest {
+public static final String NATIVE_LIB_NAME = "native_lib";
+
+@Mock
+private NativeLibFinder testSubjectHelper;
+
+private String javaLibraryPath;
+
+private Path tempDirectory;
+private List nativeLibDirs;
+private Map nativeLibNameToPath;
+
+private boolean isOsWindows;
+private boolean isOsMaxOsx;
+private boolean isOsLinux;
+
+@Before
+public void setUp() throws Exception {
+initMocks(this);
+tempDirectory = 
Files.createTempDirectory(this.getClass().getSimpleName());
+}
+
+@After
+public void tearDown() throws Exception {
+tempDirectory.toFile().deleteOnExit();
+
+Files.walk(tempDirectory)
+.sorted(Comparator.reverseOrder())
+.map(Path::toFile)
+.forEach(File::delete);
+
+}
+
+@Test
+public void testFindLibraryShouldReturnNullOnWindowsWhenNoDLLAvailable() 
throws Exception {
+// GIVEN
+isOsWindows = true;
+
+createTempFile("so");
+createTempFile("lib", "so");
+createTempFile("dylib");
+createTempFile("lib", "dylib");
+
+String expected = null;
+
+// WHEN
+// THEN
+testFindLibrary(expected);
+}
+
+@Test
+public void testFindLibraryShouldReturnDLLOnWindows() throws Exception {
+// GIVEN
+isOsWindows = true;
+
+Path expectedNativeLib = createTempFile("dll");
+createTempFile("so");
+createTempFile("lib", "so");
+createTempFile("dylib");
+createTempFile("lib", "dylib");
+
+String expected = expectedNativeLib.toFile().getAbsolutePath();
+
+// WHEN
+// THEN
+testFindLibrary(expected);
+}
+
+@Test
+public void testFindLibraryShouldReturnNullOnMacWhenNoDylibOrSoAvailable() 
throws Exception {
+// GIVEN
+isOsMaxOsx = true;
+
+createTempFile("dll");
+
+String expected = null;
+
+// WHEN
+// THEN
+testFindLibrary(expected);
+}
+
+@Test
+public void testFindLibraryShouldReturnDylibOnMac() throws Exception {
+// GIVEN
+isOsMaxOsx = true;
+
+createTempFile("dll");
+createTempFile("so");
+createTempFile("lib", "so");
+Path expectedNativeLib = createTempFile("dylib");
+createTempFile("lib", "dylib");
+
+String expected = expectedNativeLib.toFile().getAbsolutePath();
+
+// WHEN
+// THEN
+testFindLibrary(expected);
+}
+
+@Test
+public void testFindLibraryShouldReturnLibDylibOnMac() throws Exception {
+// GIVEN
+isOsMaxOsx = true;
+
+createTempFile("dll");
+createTempFile("so");
+createTempFile("lib", "so");
+Path expectedNativeLib = createTempFile("lib", "dylib");
+
+String expected = expectedNativeLib.toFile().getAbsolutePath();
+
+// WHEN
+// THEN
+testFindLibrary(expected);
+   

[GitHub] [nifi] tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library loading fixed/improved

2019-11-22 Thread GitBox
tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library 
loading fixed/improved
URL: https://github.com/apache/nifi/pull/3894#discussion_r349537862
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NativeLibFinder.java
 ##
 @@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.nar;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+
+public interface NativeLibFinder {
+Logger LOGGER = LoggerFactory.getLogger(NativeLibFinder.class);
+
+String OS = System.getProperty("os.name").toLowerCase();
+
+List getNativeLibDirs();
+
+Map getNativeLibNameToPath();
+
+String getTmpLibFilePrefix();
+
+default Set getUsrLibDirs() {
+Set usrLibDirs = 
Arrays.stream(getJavaLibraryPath().split(File.pathSeparator))
+.map(String::trim)
+.filter(pathAsString -> !pathAsString.isEmpty())
+.map(File::new)
+.map(toDir())
+.filter(Objects::nonNull)
+.collect(Collectors.toSet());
+
+return usrLibDirs;
+}
+
+default String getJavaLibraryPath() {
+return System.getProperty("java.library.path", "");
+}
+
+default Function toDir() {
+return file -> {
+if (file.isFile()) {
+return file.getParentFile();
+} else if (file.isDirectory()) {
+return file;
+} else {
+return null;
+}
+};
+}
+
+default String findLibrary(String libname) {
+Path libLocation = getNativeLibNameToPath().compute(
 
 Review comment:
   In fact I have started with the imperative approach and switched to this 
style for the very reason that I think it's more understandable and more 
robust. I guess I got used to this style by now. And although I admit this can 
be abused (and I'm probably guilty of that sometimes), in my opinion, even 
considering code quality - readability and robustness-, the functional approach 
has objective advantage over the imperative one:
   
   Functional style forces the divide and conquer strategy on the programmer 
(which is very helpful when dealing with more complex problems). It's about 
breaking down a process into sequential phases where each phase is separated 
from the rest (not unlike how NiFi works).
   One can understand the logic step by step which helps readability and since 
the steps are completely isolated from each other and the general flow is more 
declarative the overall code is less prone to bugs.
   
   Also it's easier to write a cleaner code imo as the declarative nature 
imposes boundaries that guides the flow. As I said I started with an imperative 
approach which had 3 private methods and about twice as much code as this. 
_Now_ I _can_ write it almost the same way as the current functional style, but 
only because _now_ I see the near-optimal solution.
   
   Nevertheless I'll  commit a change with a more traditional code style and 
leave the current one as an unused method for all to be able to compare.
   
   After taking a look at both we can have a final decision which to use.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [nifi] tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library loading fixed/improved

2019-11-22 Thread GitBox
tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library 
loading fixed/improved
URL: https://github.com/apache/nifi/pull/3894#discussion_r349575722
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NativeLibFinder.java
 ##
 @@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.nar;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+
+public interface NativeLibFinder {
+Logger LOGGER = LoggerFactory.getLogger(NativeLibFinder.class);
+
+String OS = System.getProperty("os.name").toLowerCase();
+
+List getNativeLibDirs();
+
+Map getNativeLibNameToPath();
+
+String getTmpLibFilePrefix();
+
+default Set getUsrLibDirs() {
 
 Review comment:
   I'm not sure how can it stay testable without increasing the complexity an 
uncomfortable amount but I'll try to come with something.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [nifi] tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library loading fixed/improved

2019-11-22 Thread GitBox
tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library 
loading fixed/improved
URL: https://github.com/apache/nifi/pull/3894#discussion_r349538192
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NativeLibFinder.java
 ##
 @@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.nar;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
+
+public interface NativeLibFinder {
+Logger LOGGER = LoggerFactory.getLogger(NativeLibFinder.class);
+
+String OS = System.getProperty("os.name").toLowerCase();
+
+List getNativeLibDirs();
+
+Map getNativeLibNameToPath();
+
+String getTmpLibFilePrefix();
+
+default Set getUsrLibDirs() {
+Set usrLibDirs = 
Arrays.stream(getJavaLibraryPath().split(File.pathSeparator))
+.map(String::trim)
+.filter(pathAsString -> !pathAsString.isEmpty())
+.map(File::new)
+.map(toDir())
+.filter(Objects::nonNull)
+.collect(Collectors.toSet());
+
+return usrLibDirs;
+}
+
+default String getJavaLibraryPath() {
+return System.getProperty("java.library.path", "");
+}
+
+default Function toDir() {
 
 Review comment:
   Indeed, good catch.


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [nifi] tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library loading fixed/improved

2019-11-22 Thread GitBox
tpalfy commented on a change in pull request #3894: NIFI-6884 - Native library 
loading fixed/improved
URL: https://github.com/apache/nifi/pull/3894#discussion_r349527062
 
 

 ##
 File path: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-nar-utils/src/main/java/org/apache/nifi/nar/NarClassLoader.java
 ##
 @@ -205,26 +216,41 @@ private void updateClasspath(File root) throws 
IOException {
 }
 
 @Override
-protected String findLibrary(final String libname) {
+public String findLibrary(String libname) {
 
 Review comment:
   The problem is that the abstract `ClassLoader` has an implementation 
returning null that takes precedent over the interface's default implementation.
   
   But I guess this ties back to the bigger question of wether is it even a 
good idea to use an interface as a trait instead of a member or an abstract 
superclass. (I give my opinion on that on that specific comment.)
   
   (Actually `ClassLoader` defines `findLibrary` as _protected_, which left 
alone wouldn't even compile because it would mean it would try to override the 
visibility of the _public_ definition of the interface. This actually points 
out a weakness of the trait interface approach. Had the ClassLoader defined the 
`findLibrary` as public, it would have been easy to simply leave out this call 
to the interface - as it would have compiled but wouldn't have worked properly 
as it would have returned null.)
   


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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services