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.

Sign up for GitHub

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

Jump to bottom

Added support for referencing NuGet packages in C# scripts #813

Merged
merged 22 commits into from
Jun 15, 2017
Merged

Added support for referencing NuGet packages in C# scripts #813

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
02c7124
Added support for referencing NuGet packages in C# scripts
seesharper Apr 4, 2017
ad90c6f
Merge branch 'dev' into dev
seesharper Apr 4, 2017
f1d6dd7
Fixed coding style issues
seesharper Apr 4, 2017
6252c0d
Added ScriptProjectProvider
seesharper Apr 21, 2017
c3bbfcc
Merge branch 'dev' of github.com:OmniSharp/omnisharp-roslyn into dev
seesharper Apr 21, 2017
0155e96
Fixed reference to DotNet.Script.NuGetMetadataResolver
seesharper Apr 24, 2017
63db355
Simplified project system
seesharper Apr 24, 2017
d58e538
Merge remote-tracking branch 'upstream/dev' into dev
seesharper May 8, 2017
a8683af
Fixed NuGet.Config (local references)
seesharper May 8, 2017
0e902d1
Fallback to default behavior if Nuget references failes
seesharper May 8, 2017
4607fbd
Merge branch 'dev' into dev
seesharper May 16, 2017
05756ec
Merge branch 'dev' into dev
seesharper May 16, 2017
5499e1e
Merge remote-tracking branch 'upstream/dev' into dev
seesharper Jun 2, 2017
1511099
Merge branch 'dev' of github.com:seesharper/omnisharp-roslyn into dev
seesharper Jun 2, 2017
d8eef4e
Added feature toggle default 'enableScriptNugetReferences=false'
seesharper Jun 3, 2017
a7f7daf
Fixed review change requests
seesharper Jun 3, 2017
7e52b3c
Update to Dotnet.Script.NuGetMetadataResolver 2.0.3
seesharper Jun 7, 2017
8316f1c
Allow IConfiguration to be optionally passed to ScriptHelper
seesharper Jun 7, 2017
a603bf5
Fixed usings and white removed white space
seesharper Jun 8, 2017
c9e9de7
Merge remote-tracking branch 'upstream/dev' into dev
seesharper Jun 8, 2017
e5a341d
Merge remote-tracking branch 'upstream/dev' into dev
seesharper Jun 12, 2017
ac8974a
Trigger
seesharper Jun 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/OmniSharp.Script/CachingScriptMetadataResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,31 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Scripting;

namespace OmniSharp.Script
{
public class CachingScriptMetadataResolver : MetadataReferenceResolver
{
private readonly MetadataReferenceResolver _defaultReferenceResolver;
private static Dictionary<string, ImmutableArray<PortableExecutableReference>> DirectReferenceCache = new Dictionary<string, ImmutableArray<PortableExecutableReference>>();
private static Dictionary<string, PortableExecutableReference> MissingReferenceCache = new Dictionary<string, PortableExecutableReference>();
private static MetadataReferenceResolver _defaultRuntimeResolver = ScriptMetadataResolver.Default;

public CachingScriptMetadataResolver(MetadataReferenceResolver defaultReferenceResolver)
{
_defaultReferenceResolver = defaultReferenceResolver;
}

public override bool Equals(object other)
{
return _defaultRuntimeResolver.Equals(other);
return _defaultReferenceResolver.Equals(other);
}

public override int GetHashCode()
{
return _defaultRuntimeResolver.GetHashCode();
return _defaultReferenceResolver.GetHashCode();
}

public override bool ResolveMissingAssemblies => _defaultRuntimeResolver.ResolveMissingAssemblies;
public override bool ResolveMissingAssemblies => _defaultReferenceResolver.ResolveMissingAssemblies;

public override PortableExecutableReference ResolveMissingAssembly(MetadataReference definition, AssemblyIdentity referenceIdentity)
{
Expand All @@ -30,7 +34,7 @@ public override PortableExecutableReference ResolveMissingAssembly(MetadataRefer
return MissingReferenceCache[referenceIdentity.Name];
}

var result = _defaultRuntimeResolver.ResolveMissingAssembly(definition, referenceIdentity);
var result = _defaultReferenceResolver.ResolveMissingAssembly(definition, referenceIdentity);
if (result != null)
{
MissingReferenceCache[referenceIdentity.Name] = result;
Expand All @@ -47,7 +51,7 @@ public override ImmutableArray<PortableExecutableReference> ResolveReference(str
return DirectReferenceCache[key];
}

var result = _defaultRuntimeResolver.ResolveReference(reference, baseFilePath, properties);
var result = _defaultReferenceResolver.ResolveReference(reference, baseFilePath, properties);
Copy link
Member

Choose a reason for hiding this comment

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

it's not visible what happens here for nuget packages, can you explain a little? do they get installed? what if there is no network? what if there is no nuget in the user's path? what's the lock file?

Copy link
Member

Choose a reason for hiding this comment

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

Any other feedback to @filipw's question above?

Copy link
Contributor

Choose a reason for hiding this comment

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

@seesharper: did this ever get answered?

if (result.Length > 0)
{
DirectReferenceCache[key] = result;
Expand Down
1 change: 1 addition & 0 deletions src/OmniSharp.Script/OmniSharp.Script.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Dotnet.Script.NuGetMetadataResolver" Version="1.0.8" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Scripting" Version="2.0.0" />
<PackageReference Include="Microsoft.DotNet.ProjectModel" Version="1.0.0-rc3-1-003177" />
<PackageReference Include="System.ValueTuple" Version="4.3.0" />
Expand Down
102 changes: 72 additions & 30 deletions src/OmniSharp.Script/ScriptProjectSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using Dotnet.Script.NuGetMetadataResolver;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Scripting;
using Microsoft.CodeAnalysis.Scripting.Hosting;
using Microsoft.DotNet.InternalAbstractions;
using Microsoft.DotNet.ProjectModel;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyModel;
Expand All @@ -17,7 +19,7 @@
using OmniSharp.Services;

namespace OmniSharp.Script
{
{
[Export(typeof(IProjectSystem)), Shared]
public class ScriptProjectSystem : IProjectSystem
{
Expand All @@ -39,38 +41,43 @@ public class ScriptProjectSystem : IProjectSystem

private const string CsxExtension = ".csx";
private static readonly CSharpParseOptions ParseOptions = new CSharpParseOptions(LanguageVersion.Default, DocumentationMode.Parse, SourceCodeKind.Script);

private static readonly Lazy<CSharpCompilationOptions> CompilationOptions = new Lazy<CSharpCompilationOptions>(() =>
private readonly Lazy<CSharpCompilationOptions> _compilationOptions;
private CSharpCompilationOptions CreateCompilation()
{
var resolver = NuGetMetadataReferenceResolver.Create(ScriptMetadataResolver.Default,
NugetFrameworkProvider.GetFrameworkNameFromAssembly(), _loggerFactory, _env.TargetDirectory);
Copy link
Member

Choose a reason for hiding this comment

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

Is this where the nuget framework to be used is determined? if so, I don't think it's correct to assume the version of nuget framework from the version of OmniSharp. A desktop OmniSharp (in fact, in VS Code only that version is used) can be used to provide language services for a .NET Core script and vice versa, a .NET Core OmniSharp can be used to provide language services for a destkop framework script

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can we within OmniSharp determine if the we a running the script in the context of a .Net Core app when there is no project context? The whole idea here is to support "self-contained" scripts that has no additional context other than the script itself.

Copy link
Member

Choose a reason for hiding this comment

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

so far - because CSI is desktop FX only and there was no formal specification for how .NET Core scripting should behave, OmniSharp would assume desktop FX mode for scripts in case no project.json was there, which would be the typical majority use case.

if project.json was found, then the first runtime FX defined there would be used.
in other words, the only way to force .NET Core scripting was through an external file, and I think for now this is how it should stay because it is non-standard scripting anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand that this is a problem and that is why .Net Core metadata references are only added when OmniSharp itself is running as a .Net Core app. It's not ideal, but at least it would provide a way for people wanting to try out project-less .Net Core scripting by pointing to the .Net Core OmniSharp binary from VsCode. Also commented on an alternative approach here .

#813 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

C# for VS Code does not ship a .NET Core version of OmniSharp. It runs OmniSharp with an embedded Mono runtime. So, this approach will not work unless the user downloads or builds a .NET Core-based OmniSharp and sets VS Code to use that. And, if they do that, they may have other problems since .NET Core OmniSharp can't handle csproj projects properly.

Note, this will also be a problem long term as the .NET Core versions of OmniSharp will be going away in favor of a Mono-based approach. See #666 for explanation.

Copy link
Contributor Author

@seesharper seesharper Apr 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I sort of understood #666 wouldn't help me much :)
Jokes aside, is there anything we can do to hint OmniSharp about the scripting runtime?

What if we did this

#r "runtime:NetCoreApp1.1"

In the case of a non-existing project.json, we could create one at runtime based on the runtime reference and feed that into the existing logic that resolves metadata references based on project context,

In other words we create a project context based on script runtime references.

The really nice thing about this approach is that we could also add the NuGet references found in the script to the dynamically created project.json and basically skip the NuGetMetadataReferenceResolver all together.

@filipw We could do the exact same approach in Dotnet-Script with NuGet references. Only difference here is that we actually know the scripting runtime context.


var compilationOptions = new CSharpCompilationOptions(
OutputKind.DynamicallyLinkedLibrary,
usings: DefaultNamespaces,
allowUnsafe: true,
metadataReferenceResolver: new CachingScriptMetadataResolver(),
sourceReferenceResolver: ScriptSourceResolver.Default,
assemblyIdentityComparer: DesktopAssemblyIdentityComparer.Default).
OutputKind.DynamicallyLinkedLibrary,
usings: DefaultNamespaces,
allowUnsafe: true,
metadataReferenceResolver: new CachingScriptMetadataResolver(resolver),
sourceReferenceResolver: ScriptSourceResolver.Default,
assemblyIdentityComparer: DesktopAssemblyIdentityComparer.Default).
Copy link
Contributor

Choose a reason for hiding this comment

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

If you like, maybe move the . to the next line like you've done elsewhere. That always looks better to me.

WithSpecificDiagnosticOptions(new Dictionary<string, ReportDiagnostic>
{
// ensure that specific warnings about assembly references are always suppressed
// https://github.com/dotnet/roslyn/issues/5501
{ "CS1701", ReportDiagnostic.Suppress },
{ "CS1702", ReportDiagnostic.Suppress },
{ "CS1705", ReportDiagnostic.Suppress }
});

var topLevelBinderFlagsProperty = typeof(CSharpCompilationOptions).GetProperty("TopLevelBinderFlags", BindingFlags.Instance | BindingFlags.NonPublic);
var binderFlagsType = typeof(CSharpCompilationOptions).GetTypeInfo().Assembly.GetType("Microsoft.CodeAnalysis.CSharp.BinderFlags");

var ignoreCorLibraryDuplicatedTypesMember = binderFlagsType?.GetField("IgnoreCorLibraryDuplicatedTypes", BindingFlags.Static | BindingFlags.Public);
{"CS1701", ReportDiagnostic.Suppress},
{"CS1702", ReportDiagnostic.Suppress},
{"CS1705", ReportDiagnostic.Suppress}
});

var topLevelBinderFlagsProperty = typeof(CSharpCompilationOptions).GetProperty("TopLevelBinderFlags",
BindingFlags.Instance | BindingFlags.NonPublic);
var binderFlagsType =
typeof(CSharpCompilationOptions).GetTypeInfo().Assembly.GetType("Microsoft.CodeAnalysis.CSharp.BinderFlags");

var ignoreCorLibraryDuplicatedTypesMember = binderFlagsType?.GetField("IgnoreCorLibraryDuplicatedTypes",
BindingFlags.Static | BindingFlags.Public);
var ignoreCorLibraryDuplicatedTypesValue = ignoreCorLibraryDuplicatedTypesMember?.GetValue(null);
if (ignoreCorLibraryDuplicatedTypesValue != null)
{
topLevelBinderFlagsProperty?.SetValue(compilationOptions, ignoreCorLibraryDuplicatedTypesValue);
}

return compilationOptions;
});

}
private readonly IMetadataFileReferenceCache _metadataFileReferenceCache;

// used for tracking purposes only
Expand All @@ -79,16 +86,31 @@ public class ScriptProjectSystem : IProjectSystem
private readonly Dictionary<string, ProjectInfo> _projects;
private readonly OmniSharpWorkspace _workspace;
private readonly IOmniSharpEnvironment _env;
private readonly ILoggerFactory _loggerFactory;
private readonly ILogger _logger;
private readonly IAssemblyLoader _assemblyLoader;
private static readonly Lazy<string> _targetFrameWork = new Lazy<string>(ResolveTargetFramework);
Copy link
Member

Choose a reason for hiding this comment

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

Rename to _targetFramework


private static string ResolveTargetFramework()
{
return Assembly.GetEntryAssembly().GetCustomAttributes()
Copy link
Member

Choose a reason for hiding this comment

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

what problem did you run into that you added this code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem. I was just making sure that I did not change the way metadata references are being added when running on the full desktop framework.

.OfType<System.Runtime.Versioning.TargetFrameworkAttribute>()
.Select(x => x.FrameworkName)
.FirstOrDefault();
}


[ImportingConstructor]
public ScriptProjectSystem(OmniSharpWorkspace workspace, IOmniSharpEnvironment env, ILoggerFactory loggerFactory, IMetadataFileReferenceCache metadataFileReferenceCache)
public ScriptProjectSystem(OmniSharpWorkspace workspace, IOmniSharpEnvironment env, ILoggerFactory loggerFactory, IMetadataFileReferenceCache metadataFileReferenceCache, IAssemblyLoader assemblyLoader)
{
_metadataFileReferenceCache = metadataFileReferenceCache;
_workspace = workspace;
_env = env;
_loggerFactory = loggerFactory;
_assemblyLoader = assemblyLoader;
_logger = loggerFactory.CreateLogger<ScriptProjectSystem>();
_projects = new Dictionary<string, ProjectInfo>();
_compilationOptions = new Lazy<CSharpCompilationOptions>(CreateCompilation);
}

public string Key => "Script";
Expand Down Expand Up @@ -121,18 +143,38 @@ public void Initalize(IConfiguration configuration)

var commonReferences = new HashSet<MetadataReference>();

// if we have no context, then we also have no dependencies
// we can assume desktop framework
// and add mscorlib

if (runtimeContexts == null || runtimeContexts.Any() == false)
{
_logger.LogInformation("Unable to find project context for CSX files. Will default to non-context usage.");
_logger.LogInformation($"Unable to find project context for CSX files. Will default to non-context usage for target framework {_targetFrameWork.Value}");

// We are running in the context of a .Net Core app.
if (_targetFrameWork.Value.StartsWith(".NETCoreApp"))
{
var runtimeId = RuntimeEnvironment.GetRuntimeIdentifier();
var inheritedAssemblyNames = DependencyContext.Default.GetRuntimeAssemblyNames(runtimeId).Where(x =>
x.FullName.StartsWith("system.", StringComparison.OrdinalIgnoreCase) ||
x.FullName.StartsWith("microsoft.codeanalysis", StringComparison.OrdinalIgnoreCase) ||
x.FullName.StartsWith("mscorlib", StringComparison.OrdinalIgnoreCase));

foreach (var inheritedAssemblyName in inheritedAssemblyNames)
{
var assembly = _assemblyLoader.Load(inheritedAssemblyName);
AddMetadataReference(commonReferences, assembly.Location);
}
}
else
{

// Assume desktop framework.
AddMetadataReference(commonReferences, typeof(object).GetTypeInfo().Assembly.Location);
AddMetadataReference(commonReferences, typeof(Enumerable).GetTypeInfo().Assembly.Location);

AddMetadataReference(commonReferences, typeof(object).GetTypeInfo().Assembly.Location);
AddMetadataReference(commonReferences, typeof(Enumerable).GetTypeInfo().Assembly.Location);
inheritedCompileLibraries.AddRange(DependencyContext.Default.CompileLibraries.Where(x =>
x.Name.ToLowerInvariant().StartsWith("system.runtime")));
}


inheritedCompileLibraries.AddRange(DependencyContext.Default.CompileLibraries.Where(x =>
x.Name.ToLowerInvariant().StartsWith("system.runtime")));
}
// otherwise we will grab dependencies for the script from the runtime context
else
Expand Down Expand Up @@ -181,7 +223,7 @@ public void Initalize(IConfiguration configuration)
name: csxFileName,
assemblyName: $"{csxFileName}.dll",
language: LanguageNames.CSharp,
compilationOptions: CompilationOptions.Value,
compilationOptions: _compilationOptions.Value,
metadataReferences: commonReferences,
parseOptions: ParseOptions,
isSubmission: true,
Expand Down