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 csproj-based scripts #980

Merged
merged 4 commits into from
Oct 13, 2017
Merged

Added support for csproj-based scripts #980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
94b9dd6
Added support for csproj-based scripts
seesharper Oct 12, 2017
c1cfb98
Merge remote-tracking branch 'upstream/master'
seesharper Oct 12, 2017
23d5447
Redirect to Microsoft.Extensions.Logging 1.1.0
seesharper Oct 12, 2017
6280c3a
LogContext (OmniSharp.Script)
seesharper Oct 12, 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
5 changes: 3 additions & 2 deletions build/Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<PropertyGroup>
<DotnetScriptNuGetMetadataResolver>2.0.3</DotnetScriptNuGetMetadataResolver>
<DotnetScriptDependencyModelVersion>0.1.0</DotnetScriptDependencyModelVersion>
<DotnetScriptDependencyModelNuGetVersion>0.1.0</DotnetScriptDependencyModelNuGetVersion>
<MicrosoftAspNetCoreDiagnosticsVersion>1.1.0</MicrosoftAspNetCoreDiagnosticsVersion>
<MicrosoftAspNetCoreHostingVersion>1.1.0</MicrosoftAspNetCoreHostingVersion>
<MicrosoftAspNetCoreHttpFeaturesVersion>1.1.0</MicrosoftAspNetCoreHttpFeaturesVersion>
Expand All @@ -25,7 +26,7 @@
<MicrosoftExtensionsConfigurationEnvironmentVariablesVersion>1.1.0</MicrosoftExtensionsConfigurationEnvironmentVariablesVersion>
<MicrosoftExtensionsConfigurationJsonVersion>1.1.0</MicrosoftExtensionsConfigurationJsonVersion>
<MicrosoftExtensionsDependencyInjectionVersion>1.1.0</MicrosoftExtensionsDependencyInjectionVersion>
<MicrosoftExtensionsDependencyModelVersion>1.1.0</MicrosoftExtensionsDependencyModelVersion>
<MicrosoftExtensionsDependencyModelVersion>2.0.0</MicrosoftExtensionsDependencyModelVersion>
<MicrosoftExtensionsFileProvidersPhysicalVersion>1.1.0</MicrosoftExtensionsFileProvidersPhysicalVersion>
<MicrosoftExtensionsFileSystemGlobbingVersion>1.1.0</MicrosoftExtensionsFileSystemGlobbingVersion>
<MicrosoftExtensionsLoggingVersion>1.1.0</MicrosoftExtensionsLoggingVersion>
Expand Down
3 changes: 2 additions & 1 deletion src/OmniSharp.Script/OmniSharp.Script.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Dotnet.Script.NuGetMetadataResolver" Version="$(DotnetScriptNuGetMetadataResolver)" />
<PackageReference Include="Dotnet.Script.DependencyModel" Version="$(DotnetScriptDependencyModelVersion)" />
<PackageReference Include="Dotnet.Script.DependencyModel.NuGet" Version="$(DotnetScriptDependencyModelNuGetVersion)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Scripting" Version="$(MicrosoftCodeAnalysisCSharpScriptingVersion)" />
<PackageReference Include="System.ValueTuple" Version="$(SystemValueTupleVersion)" />
</ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/OmniSharp.Script/ScriptHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using System;
using System.Collections.Generic;
using System.Reflection;
using Dotnet.Script.NuGetMetadataResolver;
using Dotnet.Script.DependencyModel.NuGet;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Scripting;
Expand Down
140 changes: 64 additions & 76 deletions src/OmniSharp.Script/ScriptProjectSystem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using Dotnet.Script.NuGetMetadataResolver;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.DotNet.ProjectModel;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyModel;
using Microsoft.Extensions.Logging;
using OmniSharp.Models.WorkspaceInformation;
using OmniSharp.Services;
using Dotnet.Script.DependencyModel.Compilation;
using LogLevel = Dotnet.Script.DependencyModel.Logging.LogLevel;

namespace OmniSharp.Script
{
Expand All @@ -31,8 +30,7 @@ public class ScriptProjectSystem : IProjectSystem
private readonly IOmniSharpEnvironment _env;
private readonly ILogger _logger;

private readonly IScriptProjectProvider _scriptProjectProvider;
private static readonly Lazy<string> _targetFrameWork = new Lazy<string>(ResolveTargetFramework);
private readonly CompilationDependencyResolver _compilationDependencyResolver;

[ImportingConstructor]
public ScriptProjectSystem(OmniSharpWorkspace workspace, IOmniSharpEnvironment env, ILoggerFactory loggerFactory,
Expand All @@ -43,7 +41,24 @@ public ScriptProjectSystem(OmniSharpWorkspace workspace, IOmniSharpEnvironment e
_env = env;
_logger = loggerFactory.CreateLogger<ScriptProjectSystem>();
_projects = new Dictionary<string, ProjectInfo>();
_scriptProjectProvider = ScriptProjectProvider.Create(loggerFactory);

_compilationDependencyResolver = new CompilationDependencyResolver(type =>
{
// Prefix with "OmniSharp" so that we make it through the log filter.
var categoryName = $"OmniSharp.Script.{type.FullName}";
var dependencyResolverLogger = loggerFactory.CreateLogger(categoryName);
return ((level, message) =>
{
if (level == LogLevel.Debug)
{
dependencyResolverLogger.LogDebug(message);
}
if (level == LogLevel.Info)
Copy link
Member

Choose a reason for hiding this comment

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

Are there any other related log levels (that matter?)

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, that’s it. No other levels 😊

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean it will only log informational messages if the LogLevel == Info? I would expect it to send informational messages if LogLevel <= Info. Same for debug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are only two possible levels logged from Dotnet.Script.DependencyModel

  • Debug = 0
  • Info = 1

I would expect it to send informational messages if LogLevel <= Info.

Debug is the lowest level and it would sound a little strange to me to log that as a informational message?

The LogLevel that we pass into the delegate is not from Microsoft.Extensions.Logging. It is defined in Dotnet.Script.DependencyModel with only those two levels.

Dotnet.Script.DependencyModel does not depend on Microsoft.Extensions.Logging. The code here simply forwards log messages to the actual logging framework which for OmniSharp.Roslyn means Microsoft.Extensions.Logging

Did that make any sense ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, I thought these were coming from Microsoft.Extensions.Logging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Figured you might think so:) Didn’t want to introduce a shared library version problem again. Therefore we don’t reference Microsoft.Extensions.Logging.

{
dependencyResolverLogger.LogInformation(message);
}
});
});
}

public string Key => "Script";
Expand Down Expand Up @@ -73,45 +88,31 @@ public void Initalize(IConfiguration configuration)
// explicitly include System.ValueTuple
inheritedCompileLibraries.AddRange(DependencyContext.Default.CompileLibraries.Where(x =>
x.Name.ToLowerInvariant().StartsWith("system.valuetuple")));

var runtimeContexts = File.Exists(Path.Combine(_env.TargetDirectory, "project.json")) ? ProjectContext.CreateContextForEachTarget(_env.TargetDirectory) : null;

if (!bool.TryParse(configuration["enableScriptNuGetReferences"], out var enableScriptNuGetReferences))
{
enableScriptNuGetReferences = false;
}

if (enableScriptNuGetReferences && (runtimeContexts == null || runtimeContexts.Any() == false))
{
runtimeContexts = TryCreateRuntimeContextsFromScriptFiles();
}

var runtimeContext = runtimeContexts?.FirstOrDefault();
var commonReferences = new HashSet<MetadataReference>();

// if we have no context, then we also have no dependencies
var compilationDependencies = TryGetCompilationDependencies(enableScriptNuGetReferences);

// if we have no compilation dependencies
// we will assume desktop framework
// and add default CLR references
// same applies for having a context that is not a .NET Core app
AddDefaultClrMetadataReferences(runtimeContext, commonReferences);
if (runtimeContext == null)
if (!compilationDependencies.Any())
{
_logger.LogInformation("Unable to find project context for CSX files. Will default to non-context usage (Destkop CLR scripts).");
_logger.LogInformation("Unable to find dependency context for CSX files. Will default to non-context usage (Destkop CLR scripts).");
AddDefaultClrMetadataReferences(commonReferences);
}
// otherwise we will grab dependencies for the script from the runtime context
else
{
// assume the first one
_logger.LogInformation($"Found script runtime context '{runtimeContext.TargetFramework.Framework}' for '{runtimeContext.ProjectFile.ProjectFilePath}'.");

var projectExporter = runtimeContext.CreateExporter("Release");
var projectDependencies = projectExporter.GetDependencies();

// let's inject all compilation assemblies needed
var compilationAssemblies = projectDependencies.SelectMany(x => x.CompilationAssemblies);
foreach (var compilationAssembly in compilationAssemblies)
foreach (var compilationAssembly in compilationDependencies)
{
_logger.LogDebug("Discovered script compilation assembly reference: " + compilationAssembly.ResolvedPath);
AddMetadataReference(commonReferences, compilationAssembly.ResolvedPath);
_logger.LogDebug("Discovered script compilation assembly reference: " + compilationAssembly);
AddMetadataReference(commonReferences, compilationAssembly);
}
}

Expand Down Expand Up @@ -144,30 +145,40 @@ public void Initalize(IConfiguration configuration)
}
}

private void AddDefaultClrMetadataReferences(ProjectContext projectContext, HashSet<MetadataReference> commonReferences)
private string[] TryGetCompilationDependencies(bool enableScriptNuGetReferences)
{
if (projectContext == null || projectContext.TargetFramework?.Framework != ".NETCoreApp")
try
{
var assemblies = new[]
{
typeof(object).GetTypeInfo().Assembly,
typeof(Enumerable).GetTypeInfo().Assembly,
typeof(Stack<>).GetTypeInfo().Assembly,
typeof(Lazy<,>).GetTypeInfo().Assembly,
FromName("System.Runtime"),
FromName("mscorlib")
};

var references = assemblies
.Where(a => a != null)
.Select(a => a.Location)
.Distinct()
.Select(l => _metadataFileReferenceCache.GetMetadataReference(l));

foreach (var reference in references)
{
commonReferences.Add(reference);
}
return _compilationDependencyResolver.GetDependencies(_env.TargetDirectory, enableScriptNuGetReferences).ToArray();
}
catch (Exception e)
{
_logger.LogError("Failed to resolve compilation dependencies", e);
return Array.Empty<string>();
}
}

private void AddDefaultClrMetadataReferences(HashSet<MetadataReference> commonReferences)
{
var assemblies = new[]
{
typeof(object).GetTypeInfo().Assembly,
typeof(Enumerable).GetTypeInfo().Assembly,
typeof(Stack<>).GetTypeInfo().Assembly,
typeof(Lazy<,>).GetTypeInfo().Assembly,
FromName("System.Runtime"),
FromName("mscorlib")
};

var references = assemblies
.Where(a => a != null)
.Select(a => a.Location)
.Distinct()
.Select(l => _metadataFileReferenceCache.GetMetadataReference(l));

foreach (var reference in references)
{
commonReferences.Add(reference);
}

Assembly FromName(string assemblyName)
Expand All @@ -183,21 +194,6 @@ Assembly FromName(string assemblyName)
}
}

private IEnumerable<ProjectContext> TryCreateRuntimeContextsFromScriptFiles()
{
_logger.LogInformation($"Attempting to create runtime context from script files. Default target framework {_targetFrameWork.Value}");
try
{
var scriptProjectInfo = _scriptProjectProvider.CreateProject(_env.TargetDirectory, _targetFrameWork.Value);
return ProjectContext.CreateContextForEachTarget(Path.GetDirectoryName(scriptProjectInfo.PathToProjectJson));
}
catch (Exception exception)
{
_logger.LogError(exception, "Unable to create runtime context from script files.");
}
return null;
}

private void AddMetadataReference(ISet<MetadataReference> referenceCollection, string fileReference)
{
if (!File.Exists(fileReference))
Expand Down Expand Up @@ -260,13 +256,5 @@ Task<object> IProjectSystem.GetWorkspaceModelAsync(WorkspaceInformationRequest r
}
return Task.FromResult<object>(new ScriptContextModelCollection(scriptContextModels));
}

private static string ResolveTargetFramework()
{
return Assembly.GetEntryAssembly().GetCustomAttributes()
.OfType<System.Runtime.Versioning.TargetFrameworkAttribute>()
.Select(x => x.FrameworkName)
.FirstOrDefault();
}
}
}
}
4 changes: 2 additions & 2 deletions tests/app.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@

<dependentAssembly>
<assemblyIdentity name="Microsoft.Extensions.Logging" publicKeyToken="adb9793829ddae60" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.1.0"/>
<bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.0.0"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these got added because Dotnet.Script.NuGetMetadataResolver depended on 1.1.1. So, it might be that these could just be removed now.

</dependentAssembly>
<dependentAssembly>
<assemblyIdentity name="Microsoft.Extensions.Logging.Abstractions" publicKeyToken="adb9793829ddae60" culture="neutral"/>
<bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.1.0"/>
<bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.0.0"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Want me to PR removal of these redirects?

</dependentAssembly>

<dependentAssembly>
Expand Down