Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Fix #6823: backslash in require() on Windows #6824

Merged
merged 1 commit into from Dec 12, 2016
Merged

Fix #6823: backslash in require() on Windows #6824

merged 1 commit into from Dec 12, 2016

Conversation

lucaswerkmeister
Copy link
Contributor

@lucono can you see if this fixes the issue?

@CeylonBuildBot
Copy link

Can one of the admins verify this patch? (see README)

(Improvement to fix suggested by @lucono.)
@lucono
Copy link

lucono commented Dec 11, 2016

@lucaswerkmeister, that fix did not compile because Java String.replaceAll expects a regular expression for the first argument, which fails because the \ character is an escape character, and therefore not a valid regex on its own. It fails with the following exception:

java.util.regex.PatternSyntaxException: Unexpected internal error near index 1
        at java.util.regex.Pattern.error(Pattern.java:1955)
        at java.util.regex.Pattern.compile(Pattern.java:1702)
        at java.util.regex.Pattern.<init>(Pattern.java:1351)
        at java.util.regex.Pattern.compile(Pattern.java:1028)
        at java.lang.String.replaceAll(String.java:2223)
        at com.redhat.ceylon.compiler.js.loader.JsModuleSourceMapper.resolveModule(JsModuleSourceMapper.java:171)
        at com.redhat.ceylon.compiler.typechecker.analyzer.ModuleValidator.resolveModuleIfRequired(ModuleValidator.java:312)
        at com.redhat.ceylon.compiler.typechecker.analyzer.ModuleValidator.verifyModuleDependencyTree(ModuleValidator.java:227)
        at com.redhat.ceylon.compiler.typechecker.analyzer.ModuleValidator.access$500(ModuleValidator.java:39)
        at com.redhat.ceylon.compiler.typechecker.analyzer.ModuleValidator$2.run(ModuleValidator.java:119)
        at com.redhat.ceylon.model.typechecker.context.TypeCache.doWithExplicitCaching(TypeCache.java:50)
        at com.redhat.ceylon.model.typechecker.context.TypeCache.doWithoutCaching(TypeCache.java:75)
        at com.redhat.ceylon.compiler.typechecker.analyzer.ModuleValidator.verifyModuleDependencyTree(ModuleValidator.java:97)
        at com.redhat.ceylon.compiler.typechecker.TypeChecker.executePhases(TypeChecker.java:170)
        at com.redhat.ceylon.compiler.typechecker.TypeChecker.process(TypeChecker.java:140)
        at com.redhat.ceylon.compiler.js.CeylonCompileJsTool$2.run(CeylonCompileJsTool.java:400)
        at com.redhat.ceylon.model.typechecker.context.TypeCache.doWithExplicitCaching(TypeCache.java:50)
        at com.redhat.ceylon.model.typechecker.context.TypeCache.doWithoutCaching(TypeCache.java:75)
        at com.redhat.ceylon.compiler.js.CeylonCompileJsTool.run(CeylonCompileJsTool.java:397)
        at com.redhat.ceylon.common.tools.CeylonTool.run(CeylonTool.java:547)
        at com.redhat.ceylon.common.tools.CeylonTool.execute(CeylonTool.java:423)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at com.redhat.ceylon.launcher.Launcher.runInJava7Checked(Launcher.java:108)
        at com.redhat.ceylon.launcher.Launcher.run(Launcher.java:38)
        at com.redhat.ceylon.launcher.Launcher.run(Launcher.java:31)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:497)
        at com.redhat.ceylon.launcher.Bootstrap.runVersion(Bootstrap.java:162)
        at com.redhat.ceylon.launcher.Bootstrap.runInternal(Bootstrap.java:117)
        at com.redhat.ceylon.launcher.Bootstrap.run(Bootstrap.java:93)
        at com.redhat.ceylon.launcher.Bootstrap.main(Bootstrap.java:85)

Submitted a different PR which uses String.replace(...) and tested okay on Windows.

@lucaswerkmeister
Copy link
Contributor Author

@lucono I’ve already integrated the changes of that PR into this one, can you test again? (Should be the same change now.)

@lucono
Copy link

lucono commented Dec 11, 2016

@lucaswerkmeister Updated and re-tested, and looks good.

@lucaswerkmeister
Copy link
Contributor Author

Great, thanks!

Copy link
Contributor

@chochos chochos left a comment

Choose a reason for hiding this comment

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

Looks good, merging.

@chochos chochos merged commit 0603c37 into master Dec 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants