-
Notifications
You must be signed in to change notification settings - Fork 224
Flow project.json properties into the assembly. #2966
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"projects": [ | ||
"src" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
{ | ||
"title": "Hello title", | ||
"description": "Hello description", | ||
"language": "en", | ||
"copyright": "Hello copyright", | ||
"version": "1.2.3-*", | ||
"commands": { | ||
"HelloWorld": "HelloWorld" | ||
}, | ||
"frameworks": { | ||
"dnx451": { }, | ||
"dnxcore50": { | ||
"dependencies": { | ||
"System.Console": "4.0.0-*" | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,9 @@ public class CompilationProjectContext | |
public CompilationTarget Target { get; } | ||
public string ProjectDirectory { get; } | ||
public string ProjectFilePath { get; } | ||
public string Title { get; } | ||
public string Description { get; } | ||
public string Copyright { get; } | ||
public string Version { get; } | ||
public Version AssemblyFileVersion { get; } | ||
public CompilationFiles Files { get; } | ||
|
@@ -24,6 +27,9 @@ public CompilationProjectContext( | |
CompilationTarget target, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I find a long constructor very discouraging to use. You need to remember the sequence of the parameter or heavily relies on the IDE to give you a clue. I suggest to make properties settable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. devil's advocate - when a property has a getter and a setter you never know whether it is just for easy initialization or it actually being reset later. This can lead to wrong assumptions and bugs - where the code originally assumes it is only for initialization but then someone starts resetting the value. Also if you have many properties you probably have to rely on the IDE/Intellisense as well - only you can set them in any order... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Rather have a large constructor than accidentally-mutable objects... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If only C# had immutable object initializers 😄 |
||
string projectDirectory, | ||
string projectFilePath, | ||
string title, | ||
string description, | ||
string copyright, | ||
string version, | ||
Version assemblyFileVersion, | ||
bool embedInteropTypes, | ||
|
@@ -34,6 +40,9 @@ public CompilationProjectContext( | |
ProjectDirectory = projectDirectory; | ||
ProjectFilePath = projectFilePath; | ||
Files = files; | ||
Title = title; | ||
Description = description; | ||
Copyright = copyright; | ||
Version = version; | ||
AssemblyFileVersion = assemblyFileVersion; | ||
EmbedInteropTypes = embedInteropTypes; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,7 +127,7 @@ public CompilationContext CompileProject( | |
references, | ||
compilationSettings.CompilationOptions); | ||
|
||
compilation = ApplyVersionInfo(compilation, projectContext, parseOptions); | ||
compilation = ApplyProjectInfo(compilation, projectContext, parseOptions); | ||
|
||
var compilationContext = new CompilationContext( | ||
compilation, | ||
|
@@ -251,58 +251,30 @@ private CompilationModules GetCompileModules(CompilationTarget target) | |
}); | ||
} | ||
|
||
private static CSharpCompilation ApplyVersionInfo(CSharpCompilation compilation, CompilationProjectContext project, | ||
private static CSharpCompilation ApplyProjectInfo(CSharpCompilation compilation, CompilationProjectContext project, | ||
CSharpParseOptions parseOptions) | ||
{ | ||
const string assemblyFileVersionName = "System.Reflection.AssemblyFileVersionAttribute"; | ||
const string assemblyVersionName = "System.Reflection.AssemblyVersionAttribute"; | ||
const string assemblyInformationalVersion = "System.Reflection.AssemblyInformationalVersionAttribute"; | ||
|
||
var assemblyAttributes = compilation.Assembly.GetAttributes(); | ||
|
||
var foundAssemblyFileVersion = false; | ||
var foundAssemblyVersion = false; | ||
var foundAssemblyInformationalVersion = false; | ||
|
||
foreach (var assembly in assemblyAttributes) | ||
{ | ||
string attributeName = assembly.AttributeClass.ToString(); | ||
|
||
if (string.Equals(attributeName, assemblyFileVersionName, StringComparison.Ordinal)) | ||
{ | ||
foundAssemblyFileVersion = true; | ||
} | ||
else if (string.Equals(attributeName, assemblyVersionName, StringComparison.Ordinal)) | ||
{ | ||
foundAssemblyVersion = true; | ||
} | ||
else if (string.Equals(attributeName, assemblyInformationalVersion, StringComparison.Ordinal)) | ||
{ | ||
foundAssemblyInformationalVersion = true; | ||
} | ||
} | ||
|
||
var versionAttributes = new StringBuilder(); | ||
if (!foundAssemblyFileVersion) | ||
{ | ||
versionAttributes.AppendLine($"[assembly:{assemblyFileVersionName}(\"{project.AssemblyFileVersion}\")]"); | ||
} | ||
|
||
if (!foundAssemblyVersion) | ||
var projectAttributes = new Dictionary<string, string>(StringComparer.Ordinal) | ||
{ | ||
versionAttributes.AppendLine($"[assembly:{assemblyVersionName}(\"{RemovePrereleaseTag(project.Version)}\")]"); | ||
} | ||
[typeof(System.Reflection.AssemblyTitleAttribute).FullName] = project.Title, | ||
[typeof(System.Reflection.AssemblyDescriptionAttribute).FullName] = project.Description, | ||
[typeof(System.Reflection.AssemblyCopyrightAttribute).FullName] = project.Copyright, | ||
[typeof(System.Reflection.AssemblyFileVersionAttribute).FullName] = project.AssemblyFileVersion.ToString(), | ||
[typeof(System.Reflection.AssemblyVersionAttribute).FullName] = RemovePrereleaseTag(project.Version), | ||
[typeof(System.Reflection.AssemblyInformationalVersionAttribute).FullName] = project.Version | ||
}; | ||
|
||
if (!foundAssemblyInformationalVersion) | ||
{ | ||
versionAttributes.AppendLine($"[assembly:{assemblyInformationalVersion}(\"{project.Version}\")]"); | ||
} | ||
var assemblyAttributes = compilation.Assembly.GetAttributes() | ||
.Select(assemblyAttribute => assemblyAttribute.AttributeClass.ToString()); | ||
var newAttributes = string.Join(Environment.NewLine, projectAttributes | ||
.Where(projectAttribute => projectAttribute.Value != null && !assemblyAttributes.Contains(projectAttribute.Key)) | ||
.Select(projectAttribute => $"[assembly:{projectAttribute.Key}(\"{projectAttribute.Value}\")]")); | ||
|
||
if (versionAttributes.Length != 0) | ||
if (!string.IsNullOrWhiteSpace(newAttributes)) | ||
{ | ||
compilation = compilation.AddSyntaxTrees(new[] | ||
{ | ||
CSharpSyntaxTree.ParseText(versionAttributes.ToString(), parseOptions) | ||
CSharpSyntaxTree.ParseText(newAttributes, parseOptions) | ||
}); | ||
} | ||
|
||
|
@@ -361,7 +333,7 @@ private IList<SyntaxTree> GetSyntaxTrees(CompilationProjectContext project, | |
|
||
foreach (var d in dirs) | ||
{ | ||
ctx.Monitor(new FileWriteTimeCacheDependency(d)); | ||
ctx?.Monitor(new FileWriteTimeCacheDependency(d)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unit test? make it null There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is already :) I made that change because the first time I ran my test, it blew up there with a NullReferenceException. |
||
} | ||
|
||
return trees; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Reflection; | ||
using System.Runtime.Versioning; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.Dnx.Compilation.Caching; | ||
using Microsoft.Dnx.Runtime; | ||
#if DNX451 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not #if def the entire file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might want to add new tests which are not restricted to dnx451. The only reason this is #ifdef'd is that it uses Moq, which doesn't work with CoreCLR. |
||
using Moq; | ||
#endif | ||
using Xunit; | ||
|
||
namespace Microsoft.Dnx.Compilation.CSharp.Tests | ||
{ | ||
public class RoslynCompilerTest | ||
{ | ||
#if DNX451 | ||
[Fact] | ||
public void FlowsProjectPropertiesIntoAssembly() | ||
{ | ||
const string testName = "Test name"; | ||
const string testTitle = "Test title"; | ||
const string testDescription = "Test description"; | ||
const string testCopyright = "Test copyright"; | ||
const string testAssemblyFileVersion = "1.2.3.4"; | ||
const string testVersion = "1.2.3-rc1"; | ||
const string testFrameworkName = "DNX,Version=v4.5.1"; | ||
|
||
// Arrange | ||
var compilationProjectContext = new CompilationProjectContext( | ||
new CompilationTarget(testName, new FrameworkName(testFrameworkName), string.Empty, string.Empty), | ||
string.Empty, | ||
string.Empty, | ||
testTitle, | ||
testDescription, | ||
testCopyright, | ||
testVersion, | ||
new Version(testAssemblyFileVersion), | ||
false, | ||
new CompilationFiles( | ||
new List<string> { }, | ||
new List<string> { }), | ||
new Mock<ICompilerOptions>().Object); | ||
var compiler = new RoslynCompiler( | ||
new Mock<ICache>().Object, | ||
new Mock<ICacheContextAccessor>().Object, | ||
new Mock<INamedCacheDependencyProvider>().Object, | ||
new Mock<IAssemblyLoadContext>().Object, | ||
new Mock<IApplicationEnvironment>().Object, | ||
new Mock<IServiceProvider>().Object); | ||
var metadataReference = new Mock<IRoslynMetadataReference>(); | ||
metadataReference | ||
.Setup(reference => reference.MetadataReference) | ||
.Returns(MetadataReference.CreateFromFile(typeof(object).Assembly.Location)); | ||
|
||
// Act | ||
var compilationContext = compiler.CompileProject( | ||
compilationProjectContext, | ||
new List<IMetadataReference> { metadataReference.Object }, | ||
new List<ISourceReference> { }, | ||
() => new List<ResourceDescriptor> { }); | ||
|
||
// Assert | ||
var expectedAttributes = new Dictionary<string, string> | ||
{ | ||
[typeof(AssemblyTitleAttribute).FullName] = testTitle, | ||
[typeof(AssemblyDescriptionAttribute).FullName] = testDescription, | ||
[typeof(AssemblyCopyrightAttribute).FullName] = testCopyright, | ||
[typeof(AssemblyFileVersionAttribute).FullName] = testAssemblyFileVersion, | ||
[typeof(AssemblyVersionAttribute).FullName] = testVersion.Substring(0, testVersion.IndexOf('-')), | ||
[typeof(AssemblyInformationalVersionAttribute).FullName] = testVersion, | ||
}; | ||
var compilationAttributes = compilationContext.Compilation.Assembly.GetAttributes(); | ||
|
||
Assert.All(compilationAttributes, compilationAttribute => expectedAttributes[compilationAttribute.AttributeClass.ToString()].Equals( | ||
compilationAttribute.ConstructorArguments.First().Value)); | ||
} | ||
#endif | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,10 @@ | |
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
||
using System.IO; | ||
using System.Runtime.Versioning; | ||
using System.Linq; | ||
using System.Reflection; | ||
using System.Reflection.Metadata; | ||
using System.Reflection.PortableExecutable; | ||
using Microsoft.AspNet.Testing.xunit; | ||
using Microsoft.Dnx.Testing.Framework; | ||
using Newtonsoft.Json.Linq; | ||
|
@@ -108,5 +111,78 @@ public void DnuPack_ResourcesNoArgs(DnxSdk sdk) | |
|
||
TestUtils.CleanUpTestDir<DnuPackTests>(sdk); | ||
} | ||
|
||
[Theory, TraceTest] | ||
[MemberData(nameof(DnxSdks))] | ||
public void ProjectPropertiesFlowIntoAssembly(DnxSdk sdk) | ||
{ | ||
// Arrange | ||
var solution = TestUtils.GetSolution<DnuPackTests>(sdk, "AssemblyInfo"); | ||
var project = solution.GetProject("Test"); | ||
|
||
sdk.Dnu.Restore(solution.RootPath).EnsureSuccess(); | ||
|
||
// Act | ||
var result = sdk.Dnu.Pack(project.ProjectDirectory); | ||
|
||
// Assert | ||
Assert.Equal(0, result.ExitCode); | ||
|
||
var assemblyPath = result.GetAssemblyPath(sdk.TargetFramework); | ||
|
||
using (var stream = File.OpenRead(assemblyPath)) | ||
{ | ||
using (var peReader = new PEReader(stream)) | ||
{ | ||
var metadataReader = peReader.GetMetadataReader(); | ||
var assemblyDefinition = metadataReader.GetAssemblyDefinition(); | ||
var attributes = assemblyDefinition.GetCustomAttributes() | ||
.Select(handle => metadataReader.GetCustomAttribute(handle)) | ||
.ToDictionary(attribute => GetAttributeName(attribute, metadataReader), attribute => GetAttributeArgument(attribute, metadataReader)); | ||
|
||
Assert.Equal(project.Title, attributes[typeof(AssemblyTitleAttribute).Name]); | ||
Assert.Equal(project.Description, attributes[typeof(AssemblyDescriptionAttribute).Name]); | ||
Assert.Equal(project.Copyright, attributes[typeof(AssemblyCopyrightAttribute).Name]); | ||
Assert.Equal(project.AssemblyFileVersion.ToString(), attributes[typeof(AssemblyFileVersionAttribute).Name]); | ||
Assert.Equal(project.Version.ToString(), attributes[typeof(AssemblyInformationalVersionAttribute).Name]); | ||
Assert.Equal(project.Version.Version, assemblyDefinition.Version); | ||
} | ||
} | ||
|
||
TestUtils.CleanUpTestDir<DnuPackTests>(sdk); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This statement will be called only if all the asserts pass (asserts throw). Is that expected? |
||
} | ||
|
||
private string GetAttributeName(CustomAttribute attribute, MetadataReader metadataReader) | ||
{ | ||
var container = metadataReader.GetMemberReference((MemberReferenceHandle)attribute.Constructor).Parent; | ||
var name = metadataReader.GetTypeReference((TypeReferenceHandle)container).Name; | ||
return metadataReader.GetString(name); | ||
} | ||
|
||
private string GetAttributeArgument(CustomAttribute attribute, MetadataReader metadataReader) | ||
{ | ||
var signature = metadataReader.GetMemberReference((MemberReferenceHandle)attribute.Constructor).Signature; | ||
var signatureReader = metadataReader.GetBlobReader(signature); | ||
var valueReader = metadataReader.GetBlobReader(attribute.Value); | ||
|
||
valueReader.ReadUInt16(); // Skip prolog | ||
signatureReader.ReadSignatureHeader(); // Skip header | ||
|
||
int parameterCount; | ||
signatureReader.TryReadCompressedInteger(out parameterCount); | ||
|
||
signatureReader.ReadSignatureTypeCode(); // Skip return type | ||
|
||
for (int i = 0; i < parameterCount; i++) | ||
{ | ||
var signatureTypeCode = signatureReader.ReadSignatureTypeCode(); | ||
if (signatureTypeCode == SignatureTypeCode.String) | ||
{ | ||
return valueReader.ReadSerializedString(); | ||
} | ||
} | ||
|
||
return string.Empty; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to add some unicode code point here. For example U+00A9 is commonly used for copy right sign.