Make UnionPath behave as paths from ZipFileSystem do. #2
Conversation
…y behave as jar paths do
# Conflicts: # sm-test/src/test/java/net/minecraftforge/securemodules/test/TestUnionFS.java # sm-test/src/test/java/net/minecraftforge/securemodules/test/TestUnionPath.java # src/main/java/cpw/mods/jarhandling/impl/Jar.java
| @@ -98,28 +112,25 @@ public boolean isAbsolute() { | |||
|
|
|||
| @Override | |||
| public Path getRoot() { | |||
| // Found nothing in the docs that say a non-absolute path can't have a root | |||
| // although this is uncommon. However, other stuff relies on it so leave it | |||
There was a problem hiding this comment.
What is the other stuff that relies on it?
There was a problem hiding this comment.
That comment got added in the first PR to fix issues with UnionPath. The basic issue was, that at the time, a lot of projects treated relative paths as if they were absolute, relativizing them with other absolute paths and calling getRoot() on it. iirc, when I was playing around with it, I had to make some changes to modlauncher to get forge starting and then it crashed somewhere in FMLLoader. However that was almost 3 years ago and I don't know, how up-to-date that comment still is.
I would assume, it shouldn't really be much af a problem by now because Jar now uses regular JarFileSystems when there's only one path (that wasn't the case back then) and code seems to work with them as well (which it didn't when that comment was written).
|
I'll double check this next time I touch SecureModules. One of my end goals is to migrate the UnionFS out to its own projects. Its just coupled with SM fairly tightly. But yes, its my intention that UnionFS acts as a read-only zip file system. Because that is its use case, acting as a standin for a jar file during dev time. I'll have to double check the ZipFS/ZipPath implementation to see why your changes behave like they do. And yes the few cases where it causes nulls to be returned where not expected would be a breaking change. So would need some testing with modpacks to see if modders so weird things. |
This is a copy of McModLauncher/securejarhandler#26 for SecureModules.
Currently the
UnionPathimplementation differs from the standardJarPathimplementation in several ways. In the past it was said thatUnionPathshould mimicJarPathand therefore behave the same (see https://discord.com/channels/313125603924639766/922237746460893234/953166160529076274).This pull fixes that. It also runs all the tests for
UnionPathagainstJarPaths to verify the tests test the behaviour that is desired.Merging this pull request could potentially break other projects that depend on the current incorrect behaviour of
UnionPath.