You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "davisusanibar (via GitHub)" <gi...@apache.org> on 2023/02/23 11:31:05 UTC
[GitHub] [arrow] davisusanibar opened a new pull request, #34312: GH-34293: [Java] Error loading native libraries on Windows
davisusanibar opened a new pull request, #34312:
URL: https://github.com/apache/arrow/pull/34312
To fix error on Java native libraries related to `File.separator` on Windows OS: Use `/` to separate path instead of `\`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm merged pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #34312:
URL: https://github.com/apache/arrow/pull/34312
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1531697447
Just close this and consider to be part of https://github.com/apache/arrow/issues/34763
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1497014374
@davisusanibar Sure! What failure should I fix?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar closed pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar closed pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
URL: https://github.com/apache/arrow/pull/34312
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1624789554
Conbench analyzed the 6 benchmark runs on commit `f951f0c4`.
There were 2 benchmark results indicating a performance regression:
- Commit Run on `ursa-thinkcentre-m75q` at [2023-07-05 01:10:14Z](http://conbench.ursa.dev/compare/runs/c9193f082ba3481e9de3cf1d13539ef7...d1a291166fc040e5ad7089dde94c4aae/)
- [source=cpp-micro, suite=parquet-bloom-filter-benchmark](http://conbench.ursa.dev/compare/benchmarks/064a4976429571ea80002993211b6291...064a4c39c71974258000c1859a1c6c8b)
- [params=<Subtract, Int8Type>/size:524288/inverse_null_proportion:0, source=cpp-micro, suite=arrow-compute-scalar-arithmetic-benchmark](http://conbench.ursa.dev/compare/benchmarks/064a497187c770148000641b57dbcd20...064a4c3504ac791e80004cc7fee8e57f)
The [full Conbench report](https://github.com/apache/arrow/runs/14849014768) has more details.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rtadepalli commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "rtadepalli (via GitHub)" <gi...@apache.org>.
rtadepalli commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1496602929
Oh ok, interesting. Thank you for the explanation. I was unaware that loading a resource requires `/`, and the native filesystem path separator does not apply. It all makes sense now, and changing CMakeLists.txt to produce `x86_64\arrow_dataset_jni.dll` would not do much since we're not operating with the file system abstraction, but rather with the resource abstraction.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1487266686
@github-actions crossbow submit java-jars
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1496551195
> My apologies if I am misunderstanding something here -- I am thinking that the issue is that there is a DLL file named `x86_64\arrow_dataset_jni.dll` and loading this fails with a `FileNotFoundException` because the actual DLL is named `x86_64/arrow_dataset_jni.dll`.
>
> If this is right, would changing the name of the generated DLL file and using `File.separator` not work? I am a bit unclear on what you mean by "there are no another way that maintain this code inside java classes" -- do you please mind explaining? Do you mean to say that even if we make the CMake change, we would still need to make changes in the application because loading the JAR would fail?
1. CMake did the work without problems: Create folder `x86_64` and put the native library inside that `arrow_dataset_jni.dll`
2. Next step is Java time, Java at runtime need to localize and load that native library specified by the filename argument.
3. `File.separator` (Windows) is translated/converted to `\` (that generate problems), and File.separator (Unix's) is translated/converted to `/` (working without problems)
4. BUT Java independently of the OS mention [getResource](https://docs.oracle.com/javase/8/docs/api/java/lang/ClassLoader.html#getResource-java.lang.String-:~:text=The%20name%20of%20a%20resource%20is%20a%20%27/%27%2Dseparated%20path%20name%20that%20identifies%20the%20resource.): `The name of a resource is a '/'-separated path name that identifies the resource.`
For that reason in case is needed to identified a resource (as the DLL) is needed to use `/` instead of `File.separator` independently of the OS.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1441672515
@github-actions crossbow submit java-jars
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1441602336
:warning: GitHub issue #34293 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1441602268
* Closes: #34293
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1612971055
Yes.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1512705298
I think feature freeze has already passed, unfortunately - this could possibly have been included but I still want to at least remove the useless test (if we're going to defer proper testing for a different issue)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1441678295
Revision: 4be45fb4354c2833adbfe41f18eec7418b4ea846
Submitted crossbow builds: [ursacomputing/crossbow @ actions-45d5f87c66](https://github.com/ursacomputing/crossbow/branches/all?query=actions-45d5f87c66)
|Task|Status|
|----|------|
|java-jars|[](https://github.com/ursacomputing/crossbow/actions/runs/4252559634/jobs/7396305148)|
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on a diff in pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #34312:
URL: https://github.com/apache/arrow/pull/34312#discussion_r1150857387
##########
java/dataset/src/test/java/org/apache/arrow/dataset/file/TestFileSystemDataset.java:
##########
@@ -386,6 +388,27 @@ public void testBaseCsvRead() throws Exception {
}
}
+ @Test
+ public void testResourceToLoad() throws IOException {
+ if (System.getProperty("os.name").equalsIgnoreCase("Windows")) {
+ assertEquals("\\", File.separator);
+ final String pathWithoutFileSeparator = "/" + "avroschema" + "/" + "user.avsc";
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithoutFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithoutFileSeparator);
+ }
+ }
+ final String pathWithFileSeparator = File.separator + "avroschema" + File.separator + "user.avsc";
+ Assertions.assertThrows(FileNotFoundException.class, () -> {
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithFileSeparator);
+ }
+ }
+ }, "FileNotFound \\avroschema\\user.avsc");
+ }
+ }
+
Review Comment:
`File.separator` cause the problem on Windows OS by [ClassLoader](https://docs.oracle.com/javase/8/docs/api/java/lang/ClassLoader.html#getResource-java.lang.String-:~:text=The%20name%20of%20a%20resource%20is%20a%20%27/%27%2Dseparated%20path%20name%20that%20identifies%20the%20resource.) java 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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a diff in pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #34312:
URL: https://github.com/apache/arrow/pull/34312#discussion_r1150840663
##########
java/dataset/src/test/java/org/apache/arrow/dataset/file/TestFileSystemDataset.java:
##########
@@ -386,6 +388,27 @@ public void testBaseCsvRead() throws Exception {
}
}
+ @Test
+ public void testResourceToLoad() throws IOException {
+ if (System.getProperty("os.name").equalsIgnoreCase("Windows")) {
+ assertEquals("\\", File.separator);
+ final String pathWithoutFileSeparator = "/" + "avroschema" + "/" + "user.avsc";
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithoutFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithoutFileSeparator);
+ }
+ }
+ final String pathWithFileSeparator = File.separator + "avroschema" + File.separator + "user.avsc";
+ Assertions.assertThrows(FileNotFoundException.class, () -> {
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithFileSeparator);
+ }
+ }
+ }, "FileNotFound \\avroschema\\user.avsc");
+ }
+ }
+
Review Comment:
I don't see why we're testing the Java standard library here. My previous review was asking why this issue was not caught by any test at all on Windows: it implies that we don't test any of this JNI code on Windows, and that is the real issue I want to fix.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1498163121
> I see. Then can we work on #34763 before this?
Current Dataset 11.0.0 is not working on Windows OS, this PR is to be able to work this on next release 12.0.0. We could work on #34763 as an improvement on the next releases.
>
> I have a local Windows environment but it doesn't have Java. So it's helpful for me that we can try the problem on GitHub Actions.
>
> Do you want to work on #34763? Or do you want me to work on #34763?
I'll work on that, but not for release 12.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rtadepalli commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "rtadepalli (via GitHub)" <gi...@apache.org>.
rtadepalli commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1496528236
My apologies if I am misunderstanding something here -- I am thinking that the issue is that there is a DLL file named `x86_64\arrow_dataset_jni.dll` and loading this fails with a `FileNotFoundException` because the actual DLL is named `x86_64/arrow_dataset_jni.dll`.
If this is right, would changing the name of the generated DLL file and using `File.separator` not work? I am a bit unclear on what you mean by "there are no another way that maintain this code inside java classes" -- do you please mind explaining? Do you mean to say that even if we make the CMake change, we would still need to make changes in the application because loading the JAR would fail?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rtadepalli commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "rtadepalli (via GitHub)" <gi...@apache.org>.
rtadepalli commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1496503807
Is there an issue with the CMake approach I pointed out above? The .dll generation pipeline is in the repo, and we could make CMake use a different separator for generating DLLs. No code would need to be changed, unless this is the preferred way.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1487578283
With version: 11.0.0
```
java.lang.IllegalStateException: error loading native libraries: java.io.FileNotFoundException: x86_64\arrow_dataset_jni.dll
at org.apache.arrow.dataset.jni.JniLoader.load(JniLoader.java:94)
at org.apache.arrow.dataset.jni.JniLoader.loadRemaining(JniLoader.java:74)
at org.apache.arrow.dataset.jni.JniLoader.ensureLoaded(JniLoader.java:61)
at org.apache.arrow.dataset.jni.NativeMemoryPool.getDefault(NativeMemoryPool.java:34)
at me.Recipe.main(Recipe.java:25)
```
With new version arrow-dataset-12.0.0-SNAPSHOT: https://github.com/ursacomputing/crossbow/releases/tag/actions-b5ef0c7d3c-github-java-jars
```
- wget https://github.com/ursacomputing/crossbow/releases/download/actions-b5ef0c7d3c-github-java-jars/arrow-dataset-12.0.0-SNAPSHOT.jar
- mvn install:install-file "-Dfile=C:\Users\dsusanibar\jni\arrow-dataset-12.0.0-SNAPSHOT.jar" "-DgroupId=org.apache.arrow" "-DartifactId=arrow-dataset" "-Dversion=12.0.0-SNAPSHOT" "-Dpackaging=jar"
```
Cookbook: https://arrow.apache.org/cookbook/java/dataset.html#query-data-content-for-file
```
import org.apache.arrow.dataset.file.FileFormat;
import org.apache.arrow.dataset.file.FileSystemDatasetFactory;
import org.apache.arrow.dataset.jni.NativeMemoryPool;
import org.apache.arrow.dataset.scanner.ScanOptions;
import org.apache.arrow.dataset.scanner.Scanner;
import org.apache.arrow.dataset.source.Dataset;
import org.apache.arrow.dataset.source.DatasetFactory;
import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.RootAllocator;
import org.apache.arrow.vector.VectorSchemaRoot;
import org.apache.arrow.vector.ipc.ArrowReader;
public class Recipe {
public static void main(String[] args) {
String uri = "file:///C:\\Users\\dsusanibar\\IdeaProjects\\win-cookbooks\\src\\main\\resources\\files\\data1.parquet";
ScanOptions options = new ScanOptions(/*batchSize*/ 32768);
try (
BufferAllocator allocator = new RootAllocator();
DatasetFactory datasetFactory = new FileSystemDatasetFactory(allocator, NativeMemoryPool.getDefault(), FileFormat.PARQUET, uri);
Dataset dataset = datasetFactory.finish();
Scanner scanner = dataset.newScan(options);
ArrowReader reader = scanner.scanBatches()
) {
while (reader.loadNextBatch()) {
try (VectorSchemaRoot root = reader.getVectorSchemaRoot()) {
System.out.print(root.contentToTSVString());
}
}
} catch (Exception e) {
e.printStackTrace();
}
}
}
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1496481542
Hi @kou could you help me on this PR?
`File.separator` cause the problem on Windows OS by [ClassLoader](https://docs.oracle.com/javase/8/docs/api/java/lang/ClassLoader.html#getResource-java.lang.String-:~:text=The%20name%20of%20a%20resource%20is%20a%20%27/%27%2Dseparated%20path%20name%20that%20identifies%20the%20resource.) java docs, it was introduced on Arrow 11 release and this is not able to resolve for Windows OS.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a diff in pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #34312:
URL: https://github.com/apache/arrow/pull/34312#discussion_r1150860565
##########
java/dataset/src/test/java/org/apache/arrow/dataset/file/TestFileSystemDataset.java:
##########
@@ -386,6 +388,27 @@ public void testBaseCsvRead() throws Exception {
}
}
+ @Test
+ public void testResourceToLoad() throws IOException {
+ if (System.getProperty("os.name").equalsIgnoreCase("Windows")) {
+ assertEquals("\\", File.separator);
+ final String pathWithoutFileSeparator = "/" + "avroschema" + "/" + "user.avsc";
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithoutFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithoutFileSeparator);
+ }
+ }
+ final String pathWithFileSeparator = File.separator + "avroschema" + File.separator + "user.avsc";
+ Assertions.assertThrows(FileNotFoundException.class, () -> {
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithFileSeparator);
+ }
+ }
+ }, "FileNotFound \\avroschema\\user.avsc");
+ }
+ }
+
Review Comment:
I don't see how this test changes that. And I don't see what you mean: it seems this should have failed and been caught if any of the tests ran on Windows at all. So the main problem is to make sure we test JNI code on Windows somewhere.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on a diff in pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #34312:
URL: https://github.com/apache/arrow/pull/34312#discussion_r1150899168
##########
java/dataset/src/test/java/org/apache/arrow/dataset/file/TestFileSystemDataset.java:
##########
@@ -386,6 +388,27 @@ public void testBaseCsvRead() throws Exception {
}
}
+ @Test
+ public void testResourceToLoad() throws IOException {
+ if (System.getProperty("os.name").equalsIgnoreCase("Windows")) {
+ assertEquals("\\", File.separator);
+ final String pathWithoutFileSeparator = "/" + "avroschema" + "/" + "user.avsc";
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithoutFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithoutFileSeparator);
+ }
+ }
+ final String pathWithFileSeparator = File.separator + "avroschema" + File.separator + "user.avsc";
+ Assertions.assertThrows(FileNotFoundException.class, () -> {
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithFileSeparator);
+ }
+ }
+ }, "FileNotFound \\avroschema\\user.avsc");
+ }
+ }
+
Review Comment:
Let me use the new dataset jar to validate if load is working with this change
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ssvendsen commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "ssvendsen (via GitHub)" <gi...@apache.org>.
ssvendsen commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1512660295
Any chance this this could be included in version 12 independently of #34763? Windows support is broken in version 11 anyway.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1612401466
> I don't see why we have to close this?
>
> FWIW, I never merged this because there is unaddressed feedback.
I apologize for the long time ago.
Would it be okay to remove this test `TestFileSystemDataset.testResourceToLoad` to address the last feedback?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a diff in pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #34312:
URL: https://github.com/apache/arrow/pull/34312#discussion_r1151848306
##########
java/dataset/src/test/java/org/apache/arrow/dataset/file/TestFileSystemDataset.java:
##########
@@ -386,6 +388,27 @@ public void testBaseCsvRead() throws Exception {
}
}
+ @Test
+ public void testResourceToLoad() throws IOException {
+ if (System.getProperty("os.name").equalsIgnoreCase("Windows")) {
+ assertEquals("\\", File.separator);
+ final String pathWithoutFileSeparator = "/" + "avroschema" + "/" + "user.avsc";
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithoutFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithoutFileSeparator);
+ }
+ }
+ final String pathWithFileSeparator = File.separator + "avroschema" + File.separator + "user.avsc";
+ Assertions.assertThrows(FileNotFoundException.class, () -> {
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithFileSeparator);
+ }
+ }
+ }, "FileNotFound \\avroschema\\user.avsc");
+ }
+ }
+
Review Comment:
This test is still not useful
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on a diff in pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #34312:
URL: https://github.com/apache/arrow/pull/34312#discussion_r1150850276
##########
java/dataset/src/test/java/org/apache/arrow/dataset/file/TestFileSystemDataset.java:
##########
@@ -386,6 +388,27 @@ public void testBaseCsvRead() throws Exception {
}
}
+ @Test
+ public void testResourceToLoad() throws IOException {
+ if (System.getProperty("os.name").equalsIgnoreCase("Windows")) {
+ assertEquals("\\", File.separator);
+ final String pathWithoutFileSeparator = "/" + "avroschema" + "/" + "user.avsc";
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithoutFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithoutFileSeparator);
+ }
+ }
+ final String pathWithFileSeparator = File.separator + "avroschema" + File.separator + "user.avsc";
+ Assertions.assertThrows(FileNotFoundException.class, () -> {
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithFileSeparator);
+ }
+ }
+ }, "FileNotFound \\avroschema\\user.avsc");
+ }
+ }
+
Review Comment:
The current JUnit tests are able to read resources without any issues.
Basically, this error was not detected because of the lack of checking of `getResourceAsStream` in the main code to load native libraries.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on a diff in pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on code in PR #34312:
URL: https://github.com/apache/arrow/pull/34312#discussion_r1150875075
##########
java/dataset/src/test/java/org/apache/arrow/dataset/file/TestFileSystemDataset.java:
##########
@@ -386,6 +388,27 @@ public void testBaseCsvRead() throws Exception {
}
}
+ @Test
+ public void testResourceToLoad() throws IOException {
+ if (System.getProperty("os.name").equalsIgnoreCase("Windows")) {
+ assertEquals("\\", File.separator);
+ final String pathWithoutFileSeparator = "/" + "avroschema" + "/" + "user.avsc";
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithoutFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithoutFileSeparator);
+ }
+ }
+ final String pathWithFileSeparator = File.separator + "avroschema" + File.separator + "user.avsc";
+ Assertions.assertThrows(FileNotFoundException.class, () -> {
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithFileSeparator);
+ }
+ }
+ }, "FileNotFound \\avroschema\\user.avsc");
+ }
+ }
+
Review Comment:
Ohhh ok ok, I got your point. Then, there are two parts:
1. Run Java dataset (+ all JNI modules) test code also on Windows OS
2. Fix current JNI load resources able to run on Windows OS
This fix is for part (2)
Could be ok to create another ticket for the option (1)?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1487270636
Revision: ea46814a8bf234c23aef0c7cac6f4b6f9f6e2ecd
Submitted crossbow builds: [ursacomputing/crossbow @ actions-b5ef0c7d3c](https://github.com/ursacomputing/crossbow/branches/all?query=actions-b5ef0c7d3c)
|Task|Status|
|----|------|
|java-jars|[](https://github.com/ursacomputing/crossbow/actions/runs/4545536042/jobs/8012992734)|
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1487583650
Just filled issue for CI Java JNI modules https://github.com/apache/arrow/issues/34763
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a diff in pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #34312:
URL: https://github.com/apache/arrow/pull/34312#discussion_r1150881383
##########
java/dataset/src/test/java/org/apache/arrow/dataset/file/TestFileSystemDataset.java:
##########
@@ -386,6 +388,27 @@ public void testBaseCsvRead() throws Exception {
}
}
+ @Test
+ public void testResourceToLoad() throws IOException {
+ if (System.getProperty("os.name").equalsIgnoreCase("Windows")) {
+ assertEquals("\\", File.separator);
+ final String pathWithoutFileSeparator = "/" + "avroschema" + "/" + "user.avsc";
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithoutFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithoutFileSeparator);
+ }
+ }
+ final String pathWithFileSeparator = File.separator + "avroschema" + File.separator + "user.avsc";
+ Assertions.assertThrows(FileNotFoundException.class, () -> {
+ try (final InputStream stream = TestFileSystemDataset.class.getResourceAsStream(pathWithFileSeparator)) {
+ if (stream == null) {
+ throw new FileNotFoundException(pathWithFileSeparator);
+ }
+ }
+ }, "FileNotFound \\avroschema\\user.avsc");
+ }
+ }
+
Review Comment:
Sure, though without that, it's hard to validate that we've fixed things here
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1496506714
> Is there an issue with the CMake approach I pointed out above? The .dll generation pipeline is in the repo, and we could make CMake use a different separator for generating DLLs. No code would need to be changed, unless this is the preferred way.
Yes, this could solve some scenarios. In this case this is needed at runtime to inspect native libraries inside the jar, there are no another way that maintain this code inside java classes. Do you agree?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] davisusanibar commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "davisusanibar (via GitHub)" <gi...@apache.org>.
davisusanibar commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1497124594
> @davisusanibar Sure! What test failures should I fix?
It's more related to build/test Java JNI modules also on Windows OS environment. Currently it's tested on Unix environment and all is working as expected, errors will appear on Windows OS. I have created a ticket: https://github.com/apache/arrow/issues/34763
This initial PR only contains the changes needed to load native libraries also on Windows OS environment (tested at: https://github.com/apache/arrow/pull/34312#issuecomment-1487578283)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1498145293
I see. Then can we work on #34763 before this?
I have a local Windows environment but it doesn't have Java. So it's helpful for me that we can try the problem on GitHub Actions.
Do you want to work on #34763?
Or do you want me to work on #34763?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1498330472
I see but I can't help you without #34763...
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] rtadepalli commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "rtadepalli (via GitHub)" <gi...@apache.org>.
rtadepalli commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1446906812
I don't think we should remove `File.separator`. Instead, could we try using the [`file(TO_CMAKE_PATH)`](https://cmake.org/cmake/help/latest/command/file.html) class in `CMakeLists.txt` to generate the DLL file name appropriately using `\` as a separator if the platform is Windows?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #34312: GH-34293: [Java] Error loading native libraries on Windows
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #34312:
URL: https://github.com/apache/arrow/pull/34312#issuecomment-1532216017
I don't see why we have to close this?
FWIW, I never merged this because there is unaddressed feedback.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org