search

eclipse-archived / ceylon

The Ceylon compiler, language module, and command line tools
http://ceylon-lang.org
Apache License 2.0
399 stars 62 forks source link

ModuleImpl.resourceByPath() uses incorrect path on Windows #7052

Closed CPColin closed 7 years ago

CPColin commented 7 years ago

The code in com.redhat.ceylon.compiler.java.runtime.metamodel.decl.ModuleImpl.resourceByPath() calls JVMModuleUtil.quoteJavaKeywordsInFilename(), which loads the passed path as a java.io.File object and calls getName() on that object. In Windows, the resulting path uses backslashes instead of forward slashes. When this new path is passed to RuntimeModelLoader.getContents() a few lines down, that method returns null, because CachedTOCJar.containsFile() checks a set that exclusively uses forward slashes.

This leads to ceylon.locale::locale() being unable to load any locales, as discussed here:

https://groups.google.com/d/msg/ceylon-users/mhg1sGaX3js/p-1pKT5_AgAJ

This behavior likely affects all module resource loading in Windows, when the module-resources directory is not present on disk. If that directory is available, as when a developer is currently developing a module, the second strategy in resourceByPath() will probably successfully find the resource and the developer will be none the wiser, until somebody imports the published module.

Note that the other place that uses quoteJavaKeywordsInFilename() evades the problem by replacing all backslashes with forward slashes.

gavinking commented 7 years ago

@CPColin Thanks, should be fixed now, please try it out.

CPColin commented 7 years ago

The commit looks good, but I've never built the ceylon project before, so it might be a bit before I can verify the functionality. I think this can be verified by changing the assumeTrue in test.ceylon.locale::testAmPm to assertTrue; the locale being unable to load was why I made it an assumeTrue, if I remember correctly.

CPColin commented 7 years ago

I'll get my Windows environment upgraded to Eclipse Oxygen and Ceylon 1.3.3 and test this soon!

CPColin commented 7 years ago

Confirmed! Checked out the version-1.3.3 branch and had to revert the ceylon.test module from 1.3.3.1 to 1.3.3 to get it to compile, but I can confirm that the testAmPm test completes properly on Windows now. Thanks!

gavinking commented 7 years ago

Great!