Only use fast path when extension is CSX or none #980
Conversation
src/ScriptCs.Hosting/ModuleLoader.cs
Outdated
There was a problem hiding this comment.
Should we make the comparison with ".csx" case insensitive?
There was a problem hiding this comment.
good point
On 21 March 2015 at 21:48, Adam Ralph notifications@github.com wrote:
In src/ScriptCs.Hosting/ModuleLoader.cs
#980 (comment):@@ -69,7 +71,8 @@ public ModuleLoader(IAssemblyResolver resolver, ILog logger, Action<Assembly, Ag
{
if (modulePackagesPaths == null) return;
if (moduleNames.Length == 1 && DefaultCSharpModules.ContainsKey(moduleNames[0])) // only CSharp module needed// only CSharp module needed - use fast pathif (moduleNames.Length == 1 && DefaultCSharpModules.ContainsKey(moduleNames[0]) && (extension == DefaultCSharpExtension || string.IsNullOrWhiteSpace(extension)))Should we make the comparison with ".csx" case insensitive?
—
Reply to this email directly or view it on GitHub
https://github.com/scriptcs/scriptcs/pull/980/files#r26896677.
|
Whilst we're here, should we also instantiate internal static readonly Dictionary<string, string> DefaultCSharpModules =
new Dictionary<string, string>(StringComparer.CurrentCultureIgnoreCase)
{
{"roslyn", "ScriptCs.Engine.Roslyn.dll"},
{"mono", "ScriptCs.Engine.Mono.dll"}
}; |
src/ScriptCs.Hosting/ModuleLoader.cs
Outdated
There was a problem hiding this comment.
This blows up with a NullReferenceException if extension is null (REPL). You'll have to use string.Equals(extension, DefaultCSharpExtension, StringComparison.OrdinalIgnoreCase)
There was a problem hiding this comment.
Or you could swap the order of the conditions around the ||, but may as well just call the static on string.
|
The Travis build is still so flaky... IIRC I've seen the problem before. It's a race condition in xunit 1.0. I don't know why it only manifests in Linux though. |
|
LGTM. The module name casing can be left for now, that's a wider problem which isn't catered for in |
Only use fast path when extension is CSX or none
2 participants
Fixes #964 and unblocks 0.14 release
The fast path is only invoked if there is: