Skip to content
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

Fix issue with resource files not loaded on Ubuntu Linux #5898

Closed
wants to merge 1 commit into from
Closed

Fix issue with resource files not loaded on Ubuntu Linux #5898

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 5, 2021

On Ubuntu Linux the attempt to enumerate DAO block files in a JAR resource directory fails.

See #5894 (comment)

All the block data is then downloaded from the seednodes.

This PR does the enumeration using nio.File.FileSystem so that block files in resources can be used.

@ghost ghost mentioned this pull request Dec 5, 2021
FileSystem fileSystem = FileSystems.newFileSystem(dirUrl.toURI(), Collections.emptyMap());
List<Path> filePaths = Files.walk(fileSystem.getPath(resourceDir), 1).collect(Collectors.toList());
if (filePaths.size() <= 1) {
log.info("No files in directory. {}", dirUrl.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toString is not necessary.

Suggested change
log.info("No files in directory. {}", dirUrl.toString());
log.info("No files in directory. {}", dirUrl);

@ripcurlx
Copy link
Contributor

ripcurlx commented Dec 6, 2021

Doesn't work now on macOS unfortunately

java.lang.IllegalArgumentException: Path component should be '/'
	at java.base/sun.nio.fs.UnixFileSystemProvider.checkUri(UnixFileSystemProvider.java:81)
	at java.base/sun.nio.fs.UnixFileSystemProvider.newFileSystem(UnixFileSystemProvider.java:90)
	at java.base/java.nio.file.FileSystems.newFileSystem(FileSystems.java:338)
	at java.base/java.nio.file.FileSystems.newFileSystem(FileSystems.java:287)
	at bisq.core.dao.state.storage.BsqBlocksStorageService.copyFromResources(BsqBlocksStorageService.java:131)
	at bisq.core.dao.state.storage.DaoStateStorageService.lambda$readFromResources$4(DaoStateStorageService.java:119)
	at java.base/java.lang.Thread.run(Thread.java:832)

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ripcurlx ripcurlx added this to the v1.8.0 milestone Dec 6, 2021
@stejbac
Copy link
Contributor

stejbac commented Dec 6, 2021

I was debugging the same issue on Windows a couple of days ago, caused by the expression new File(dirUrl.toUri()). I ended up adapting the following method from StackOverflow, which seems to work:

https://stackoverflow.com/questions/3923129/get-a-list-of-resources-from-classpath-directory/48190582#48190582

It looks like you have to use two different code paths to list the resource directory, depending on the environment.

(I created a draft patch for the fix, but never got round to creating a PR out of it. The foreach loop to do the copying needs modifying to work on Windows as well, just like in this PR.)

@ghost
Copy link
Author

ghost commented Dec 6, 2021

@stejbac Looks like we were both debugging the same issue. As this one does not work for MacOs, can you make your PR?

@ghost ghost closed this Dec 6, 2021
@stejbac
Copy link
Contributor

stejbac commented Dec 6, 2021

@jmacxx OK, sure.

@chimp1984
Copy link
Contributor

@stejbac Ah great that you tackled that as well. Could we extract the handling in a FileUtil class to make it re-usable. A pity that simple filehandling from resources is such a mess in java...
We have verious other loadFromResource calls. Maybe worth to re-check if there are issues as well.

@ghost ghost mentioned this pull request Dec 20, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants