From 95a108d95113c57179bfc04f02e3c3dd5e55dee0 Mon Sep 17 00:00:00 2001 From: nickdalt Date: Sun, 19 Apr 2020 14:40:47 +0100 Subject: [PATCH 1/5] Add support for C++ libs into sourcelink --- src/SourceLink.Common/MergeSourceLinkFiles.cs | 167 ++++++++++++++++++ .../build/Microsoft.SourceLink.Common.targets | 32 +++- 2 files changed, 193 insertions(+), 6 deletions(-) create mode 100644 src/SourceLink.Common/MergeSourceLinkFiles.cs diff --git a/src/SourceLink.Common/MergeSourceLinkFiles.cs b/src/SourceLink.Common/MergeSourceLinkFiles.cs new file mode 100644 index 00000000..8a93a314 --- /dev/null +++ b/src/SourceLink.Common/MergeSourceLinkFiles.cs @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.Collections.Generic; +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; +using System.Runtime.Serialization; +using System.Runtime.Serialization.Json; +using System; + +namespace Microsoft.SourceLink.Common +{ + /// + /// Used for serialization of the sourcelink files. Only supports the basic elements generated in this project. + /// + [DataContract] + public class SourceLinks + { + [DataMember] + public Dictionary documents = new Dictionary(); + } + + public sealed class MergeSourceLinkFiles : Task + { + /// + /// The name/path of the sourcelink file that we will merge into. + /// + [Required, NotNull] + public string? SourceLinkFile { get; set; } + + /// + /// Collection of all the library directories that will be searched for lib files. + /// + [Required, NotNull] + public string[]? AdditionalLibraryDirectories { get; set; } + + /// + /// Collection of all the libs that we will link to. + /// + [Required, NotNull] + public string[]? AdditionalDependencies { get; set; } + + /// + /// Collection of solution referenced import libraries. + /// + [Required, NotNull] + public string[]? ImportLibraries { get; set; } + + public override bool Execute() + { + try + { + var additionalSourceLinks = new List(); + + // Read the original sourcelink file + var sourceLink = ReadSourceLinkFile(SourceLinkFile); + + //// Throughout we expect that the sourcelink files for a lib is alongside + //// the lib with the extension sourcelink.json instead of lib. + + // For import libraries we always have the full path to the lib. This shouldn't be needed since + // the path will be common to the dll/exe project. We have this in case there are out of tree + // references to library projects. + foreach (var importLib in ImportLibraries) + { + string sourceLinkName = Path.ChangeExtension(importLib, "sourcelink.json"); + if (File.Exists(sourceLinkName)) + { + Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); + additionalSourceLinks.Add(ReadSourceLinkFile(sourceLinkName)); + } + } + + // Try and find sourcelink files for each lib + foreach (var dependency in AdditionalDependencies) + { + string sourceLinkName = Path.ChangeExtension(dependency, "sourcelink.json"); + if (Path.IsPathRooted(dependency)) + { + // If the lib path is rooted just look for the sourcelink file with the appropriate extension + // on that path. + if (File.Exists(sourceLinkName)) + { + Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); + additionalSourceLinks.Add(ReadSourceLinkFile(sourceLinkName)); + } + } + else + { + // Not-rooted, perform link like scanning of the lib directories to find the full lib path + // and then look for the sourcelink file alongside the lib with the appropriate extension. + foreach (var libDir in AdditionalLibraryDirectories) + { + string potentialPath = Path.Combine(libDir, sourceLinkName); + if (File.Exists(potentialPath)) + { + Log.LogMessage("Found additional sourcelink file '{0}'", potentialPath); + additionalSourceLinks.Add(ReadSourceLinkFile(potentialPath)); + break; + } + } + } + } + + // Merge all the sourcelinks together and write back to the original sourcelink file path + MergeSourceLinks(sourceLink, additionalSourceLinks); + WriteSourceLinkFile(sourceLink, SourceLinkFile); + return true; + } + catch (Exception ex) + { + Log.LogError("Failed to merge sourcelink files for libs with dll/exe sourcelink file - {0}", ex.Message); + } + + return false; + } + + private void MergeSourceLinks(SourceLinks sourceLink, List additionalSourceLinks) + { + foreach (var additionalSourceLink in additionalSourceLinks) + { + foreach (var document in additionalSourceLink.documents) + { + if ( !sourceLink.documents.ContainsKey(document.Key)) + { + sourceLink.documents.Add(document.Key, document.Value); + Log.LogMessage("Additional sourcelink document {0}: {1}", document.Key, document.Value); + } + else + { + Log.LogMessage("Sourcelink document {0} already exists", document.Key); + } + } + } + } + + private static SourceLinks ReadSourceLinkFile(string path) + { + DataContractJsonSerializerSettings settings = new DataContractJsonSerializerSettings + { + UseSimpleDictionaryFormat = true + }; + + var sourceLink = new SourceLinks(); + using (var fileStream = File.OpenRead(path)) + { + var serializer = new DataContractJsonSerializer(typeof(SourceLinks), settings); + return (SourceLinks)serializer.ReadObject(fileStream); + } + } + + private static void WriteSourceLinkFile(SourceLinks sourceLink, string path) + { + DataContractJsonSerializerSettings settings = new DataContractJsonSerializerSettings + { + UseSimpleDictionaryFormat = true + }; + + using (var fileStream = File.OpenWrite(path)) + { + var serializer = new DataContractJsonSerializer(typeof(SourceLinks), settings); + serializer.WriteObject(fileStream, sourceLink); + } + } + } +} \ No newline at end of file diff --git a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets index 8b84623a..b0bd133a 100644 --- a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets +++ b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets @@ -3,6 +3,7 @@ + @@ -27,10 +28,18 @@ DependsOnTargets="InitializeSourceRootMappedPaths" Condition="'$(SourceRootMappedPathsFeatureSupported)' == 'true'"/> + + + <_GenerateSourceLinkFileBeforeTargets>BeforeClCompile + <_GenerateSourceLinkFileDependsOnTargets/> + + - + <_GenerateSourceLinkFileBeforeTargets>Link <_GenerateSourceLinkFileDependsOnTargets>ComputeLinkSwitches @@ -45,26 +54,37 @@ This target shall initialize SourceLinkUrl of all items that don't have it initialized yet and belong to the source control provider. --> - + - - + + %(Link.AdditionalOptions) /sourcelink:"$(SourceLink)" + + + + + From 093d114da244a52018c9b2e71e2397937825bad2 Mon Sep 17 00:00:00 2001 From: nickdalt Date: Wed, 29 Apr 2020 19:07:21 +0100 Subject: [PATCH 2/5] Changed so that rather than merging all the sourcelink files into one, multiple uses of the link /sourcelink arg are used. --- ...es.cs => FindAdditionalSourceLinkFiles.cs} | 85 +++---------------- .../build/Microsoft.SourceLink.Common.targets | 20 ++--- 2 files changed, 20 insertions(+), 85 deletions(-) rename src/SourceLink.Common/{MergeSourceLinkFiles.cs => FindAdditionalSourceLinkFiles.cs} (54%) diff --git a/src/SourceLink.Common/MergeSourceLinkFiles.cs b/src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs similarity index 54% rename from src/SourceLink.Common/MergeSourceLinkFiles.cs rename to src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs index 8a93a314..21123da5 100644 --- a/src/SourceLink.Common/MergeSourceLinkFiles.cs +++ b/src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs @@ -5,23 +5,11 @@ using System.Collections.Generic; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; -using System.Runtime.Serialization; -using System.Runtime.Serialization.Json; using System; namespace Microsoft.SourceLink.Common { - /// - /// Used for serialization of the sourcelink files. Only supports the basic elements generated in this project. - /// - [DataContract] - public class SourceLinks - { - [DataMember] - public Dictionary documents = new Dictionary(); - } - - public sealed class MergeSourceLinkFiles : Task + public sealed class FindAdditionalSourceLinkFiles : Task { /// /// The name/path of the sourcelink file that we will merge into. @@ -47,15 +35,16 @@ public sealed class MergeSourceLinkFiles : Task [Required, NotNull] public string[]? ImportLibraries { get; set; } + [Output] + public string[]? AllSourceLinkFiles { get; set; } + public override bool Execute() { + List allSourceLinkFiles = new List(); + allSourceLinkFiles.Add(SourceLinkFile); + try { - var additionalSourceLinks = new List(); - - // Read the original sourcelink file - var sourceLink = ReadSourceLinkFile(SourceLinkFile); - //// Throughout we expect that the sourcelink files for a lib is alongside //// the lib with the extension sourcelink.json instead of lib. @@ -68,7 +57,7 @@ public override bool Execute() if (File.Exists(sourceLinkName)) { Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); - additionalSourceLinks.Add(ReadSourceLinkFile(sourceLinkName)); + allSourceLinkFiles.Add(sourceLinkName); } } @@ -83,7 +72,7 @@ public override bool Execute() if (File.Exists(sourceLinkName)) { Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); - additionalSourceLinks.Add(ReadSourceLinkFile(sourceLinkName)); + allSourceLinkFiles.Add(sourceLinkName); } } else @@ -96,72 +85,22 @@ public override bool Execute() if (File.Exists(potentialPath)) { Log.LogMessage("Found additional sourcelink file '{0}'", potentialPath); - additionalSourceLinks.Add(ReadSourceLinkFile(potentialPath)); + allSourceLinkFiles.Add(potentialPath); break; } } } } - // Merge all the sourcelinks together and write back to the original sourcelink file path - MergeSourceLinks(sourceLink, additionalSourceLinks); - WriteSourceLinkFile(sourceLink, SourceLinkFile); + AllSourceLinkFiles = allSourceLinkFiles.ToArray(); return true; } catch (Exception ex) { - Log.LogError("Failed to merge sourcelink files for libs with dll/exe sourcelink file - {0}", ex.Message); + Log.LogError("Failed to find sourcelink files for libs with dll/exe sourcelink file - {0}", ex.Message); } return false; } - - private void MergeSourceLinks(SourceLinks sourceLink, List additionalSourceLinks) - { - foreach (var additionalSourceLink in additionalSourceLinks) - { - foreach (var document in additionalSourceLink.documents) - { - if ( !sourceLink.documents.ContainsKey(document.Key)) - { - sourceLink.documents.Add(document.Key, document.Value); - Log.LogMessage("Additional sourcelink document {0}: {1}", document.Key, document.Value); - } - else - { - Log.LogMessage("Sourcelink document {0} already exists", document.Key); - } - } - } - } - - private static SourceLinks ReadSourceLinkFile(string path) - { - DataContractJsonSerializerSettings settings = new DataContractJsonSerializerSettings - { - UseSimpleDictionaryFormat = true - }; - - var sourceLink = new SourceLinks(); - using (var fileStream = File.OpenRead(path)) - { - var serializer = new DataContractJsonSerializer(typeof(SourceLinks), settings); - return (SourceLinks)serializer.ReadObject(fileStream); - } - } - - private static void WriteSourceLinkFile(SourceLinks sourceLink, string path) - { - DataContractJsonSerializerSettings settings = new DataContractJsonSerializerSettings - { - UseSimpleDictionaryFormat = true - }; - - using (var fileStream = File.OpenWrite(path)) - { - var serializer = new DataContractJsonSerializer(typeof(SourceLinks), settings); - serializer.WriteObject(fileStream, sourceLink); - } - } } } \ No newline at end of file diff --git a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets index b0bd133a..af7cbc49 100644 --- a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets +++ b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets @@ -3,7 +3,7 @@ - + @@ -64,27 +64,23 @@ + + + + + - %(Link.AdditionalOptions) /sourcelink:"$(SourceLink)" + %(Link.AdditionalOptions) @(SourceLinks->'/sourcelink:"%(Identity)"', ' ') - - - - - From 1742dcad39a2e2accc8300288d531e654fb974cd Mon Sep 17 00:00:00 2001 From: nickdalt Date: Sun, 19 Apr 2020 14:40:47 +0100 Subject: [PATCH 3/5] Add support for C++ libs into sourcelink --- src/SourceLink.Common/MergeSourceLinkFiles.cs | 167 ++++++++++++++++++ .../build/Microsoft.SourceLink.Common.targets | 32 +++- 2 files changed, 193 insertions(+), 6 deletions(-) create mode 100644 src/SourceLink.Common/MergeSourceLinkFiles.cs diff --git a/src/SourceLink.Common/MergeSourceLinkFiles.cs b/src/SourceLink.Common/MergeSourceLinkFiles.cs new file mode 100644 index 00000000..8a93a314 --- /dev/null +++ b/src/SourceLink.Common/MergeSourceLinkFiles.cs @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.Collections.Generic; +using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; +using System.Runtime.Serialization; +using System.Runtime.Serialization.Json; +using System; + +namespace Microsoft.SourceLink.Common +{ + /// + /// Used for serialization of the sourcelink files. Only supports the basic elements generated in this project. + /// + [DataContract] + public class SourceLinks + { + [DataMember] + public Dictionary documents = new Dictionary(); + } + + public sealed class MergeSourceLinkFiles : Task + { + /// + /// The name/path of the sourcelink file that we will merge into. + /// + [Required, NotNull] + public string? SourceLinkFile { get; set; } + + /// + /// Collection of all the library directories that will be searched for lib files. + /// + [Required, NotNull] + public string[]? AdditionalLibraryDirectories { get; set; } + + /// + /// Collection of all the libs that we will link to. + /// + [Required, NotNull] + public string[]? AdditionalDependencies { get; set; } + + /// + /// Collection of solution referenced import libraries. + /// + [Required, NotNull] + public string[]? ImportLibraries { get; set; } + + public override bool Execute() + { + try + { + var additionalSourceLinks = new List(); + + // Read the original sourcelink file + var sourceLink = ReadSourceLinkFile(SourceLinkFile); + + //// Throughout we expect that the sourcelink files for a lib is alongside + //// the lib with the extension sourcelink.json instead of lib. + + // For import libraries we always have the full path to the lib. This shouldn't be needed since + // the path will be common to the dll/exe project. We have this in case there are out of tree + // references to library projects. + foreach (var importLib in ImportLibraries) + { + string sourceLinkName = Path.ChangeExtension(importLib, "sourcelink.json"); + if (File.Exists(sourceLinkName)) + { + Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); + additionalSourceLinks.Add(ReadSourceLinkFile(sourceLinkName)); + } + } + + // Try and find sourcelink files for each lib + foreach (var dependency in AdditionalDependencies) + { + string sourceLinkName = Path.ChangeExtension(dependency, "sourcelink.json"); + if (Path.IsPathRooted(dependency)) + { + // If the lib path is rooted just look for the sourcelink file with the appropriate extension + // on that path. + if (File.Exists(sourceLinkName)) + { + Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); + additionalSourceLinks.Add(ReadSourceLinkFile(sourceLinkName)); + } + } + else + { + // Not-rooted, perform link like scanning of the lib directories to find the full lib path + // and then look for the sourcelink file alongside the lib with the appropriate extension. + foreach (var libDir in AdditionalLibraryDirectories) + { + string potentialPath = Path.Combine(libDir, sourceLinkName); + if (File.Exists(potentialPath)) + { + Log.LogMessage("Found additional sourcelink file '{0}'", potentialPath); + additionalSourceLinks.Add(ReadSourceLinkFile(potentialPath)); + break; + } + } + } + } + + // Merge all the sourcelinks together and write back to the original sourcelink file path + MergeSourceLinks(sourceLink, additionalSourceLinks); + WriteSourceLinkFile(sourceLink, SourceLinkFile); + return true; + } + catch (Exception ex) + { + Log.LogError("Failed to merge sourcelink files for libs with dll/exe sourcelink file - {0}", ex.Message); + } + + return false; + } + + private void MergeSourceLinks(SourceLinks sourceLink, List additionalSourceLinks) + { + foreach (var additionalSourceLink in additionalSourceLinks) + { + foreach (var document in additionalSourceLink.documents) + { + if ( !sourceLink.documents.ContainsKey(document.Key)) + { + sourceLink.documents.Add(document.Key, document.Value); + Log.LogMessage("Additional sourcelink document {0}: {1}", document.Key, document.Value); + } + else + { + Log.LogMessage("Sourcelink document {0} already exists", document.Key); + } + } + } + } + + private static SourceLinks ReadSourceLinkFile(string path) + { + DataContractJsonSerializerSettings settings = new DataContractJsonSerializerSettings + { + UseSimpleDictionaryFormat = true + }; + + var sourceLink = new SourceLinks(); + using (var fileStream = File.OpenRead(path)) + { + var serializer = new DataContractJsonSerializer(typeof(SourceLinks), settings); + return (SourceLinks)serializer.ReadObject(fileStream); + } + } + + private static void WriteSourceLinkFile(SourceLinks sourceLink, string path) + { + DataContractJsonSerializerSettings settings = new DataContractJsonSerializerSettings + { + UseSimpleDictionaryFormat = true + }; + + using (var fileStream = File.OpenWrite(path)) + { + var serializer = new DataContractJsonSerializer(typeof(SourceLinks), settings); + serializer.WriteObject(fileStream, sourceLink); + } + } + } +} \ No newline at end of file diff --git a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets index 9a7f4d59..268b2257 100644 --- a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets +++ b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets @@ -4,6 +4,7 @@ + @@ -28,10 +29,18 @@ DependsOnTargets="InitializeSourceRootMappedPaths" Condition="'$(SourceRootMappedPathsFeatureSupported)' == 'true'"/> + + + <_GenerateSourceLinkFileBeforeTargets>BeforeClCompile + <_GenerateSourceLinkFileDependsOnTargets/> + + - + <_GenerateSourceLinkFileBeforeTargets>Link <_GenerateSourceLinkFileDependsOnTargets>ComputeLinkSwitches @@ -46,26 +55,37 @@ This target shall initialize SourceLinkUrl of all items that don't have it initialized yet and belong to the source control provider. --> - + - - + + %(Link.AdditionalOptions) /sourcelink:"$(SourceLink)" + + + + + From 02c66c7166085aea5f41e1d9c72963ce8cd469da Mon Sep 17 00:00:00 2001 From: nickdalt Date: Wed, 29 Apr 2020 19:07:21 +0100 Subject: [PATCH 4/5] Changed so that rather than merging all the sourcelink files into one, multiple uses of the link /sourcelink arg are used. --- ...es.cs => FindAdditionalSourceLinkFiles.cs} | 85 +++---------------- .../build/Microsoft.SourceLink.Common.targets | 20 ++--- 2 files changed, 20 insertions(+), 85 deletions(-) rename src/SourceLink.Common/{MergeSourceLinkFiles.cs => FindAdditionalSourceLinkFiles.cs} (54%) diff --git a/src/SourceLink.Common/MergeSourceLinkFiles.cs b/src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs similarity index 54% rename from src/SourceLink.Common/MergeSourceLinkFiles.cs rename to src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs index 8a93a314..21123da5 100644 --- a/src/SourceLink.Common/MergeSourceLinkFiles.cs +++ b/src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs @@ -5,23 +5,11 @@ using System.Collections.Generic; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; -using System.Runtime.Serialization; -using System.Runtime.Serialization.Json; using System; namespace Microsoft.SourceLink.Common { - /// - /// Used for serialization of the sourcelink files. Only supports the basic elements generated in this project. - /// - [DataContract] - public class SourceLinks - { - [DataMember] - public Dictionary documents = new Dictionary(); - } - - public sealed class MergeSourceLinkFiles : Task + public sealed class FindAdditionalSourceLinkFiles : Task { /// /// The name/path of the sourcelink file that we will merge into. @@ -47,15 +35,16 @@ public sealed class MergeSourceLinkFiles : Task [Required, NotNull] public string[]? ImportLibraries { get; set; } + [Output] + public string[]? AllSourceLinkFiles { get; set; } + public override bool Execute() { + List allSourceLinkFiles = new List(); + allSourceLinkFiles.Add(SourceLinkFile); + try { - var additionalSourceLinks = new List(); - - // Read the original sourcelink file - var sourceLink = ReadSourceLinkFile(SourceLinkFile); - //// Throughout we expect that the sourcelink files for a lib is alongside //// the lib with the extension sourcelink.json instead of lib. @@ -68,7 +57,7 @@ public override bool Execute() if (File.Exists(sourceLinkName)) { Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); - additionalSourceLinks.Add(ReadSourceLinkFile(sourceLinkName)); + allSourceLinkFiles.Add(sourceLinkName); } } @@ -83,7 +72,7 @@ public override bool Execute() if (File.Exists(sourceLinkName)) { Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); - additionalSourceLinks.Add(ReadSourceLinkFile(sourceLinkName)); + allSourceLinkFiles.Add(sourceLinkName); } } else @@ -96,72 +85,22 @@ public override bool Execute() if (File.Exists(potentialPath)) { Log.LogMessage("Found additional sourcelink file '{0}'", potentialPath); - additionalSourceLinks.Add(ReadSourceLinkFile(potentialPath)); + allSourceLinkFiles.Add(potentialPath); break; } } } } - // Merge all the sourcelinks together and write back to the original sourcelink file path - MergeSourceLinks(sourceLink, additionalSourceLinks); - WriteSourceLinkFile(sourceLink, SourceLinkFile); + AllSourceLinkFiles = allSourceLinkFiles.ToArray(); return true; } catch (Exception ex) { - Log.LogError("Failed to merge sourcelink files for libs with dll/exe sourcelink file - {0}", ex.Message); + Log.LogError("Failed to find sourcelink files for libs with dll/exe sourcelink file - {0}", ex.Message); } return false; } - - private void MergeSourceLinks(SourceLinks sourceLink, List additionalSourceLinks) - { - foreach (var additionalSourceLink in additionalSourceLinks) - { - foreach (var document in additionalSourceLink.documents) - { - if ( !sourceLink.documents.ContainsKey(document.Key)) - { - sourceLink.documents.Add(document.Key, document.Value); - Log.LogMessage("Additional sourcelink document {0}: {1}", document.Key, document.Value); - } - else - { - Log.LogMessage("Sourcelink document {0} already exists", document.Key); - } - } - } - } - - private static SourceLinks ReadSourceLinkFile(string path) - { - DataContractJsonSerializerSettings settings = new DataContractJsonSerializerSettings - { - UseSimpleDictionaryFormat = true - }; - - var sourceLink = new SourceLinks(); - using (var fileStream = File.OpenRead(path)) - { - var serializer = new DataContractJsonSerializer(typeof(SourceLinks), settings); - return (SourceLinks)serializer.ReadObject(fileStream); - } - } - - private static void WriteSourceLinkFile(SourceLinks sourceLink, string path) - { - DataContractJsonSerializerSettings settings = new DataContractJsonSerializerSettings - { - UseSimpleDictionaryFormat = true - }; - - using (var fileStream = File.OpenWrite(path)) - { - var serializer = new DataContractJsonSerializer(typeof(SourceLinks), settings); - serializer.WriteObject(fileStream, sourceLink); - } - } } } \ No newline at end of file diff --git a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets index 268b2257..f6c69796 100644 --- a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets +++ b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets @@ -4,7 +4,7 @@ - + @@ -65,27 +65,23 @@ + + + + + - %(Link.AdditionalOptions) /sourcelink:"$(SourceLink)" + %(Link.AdditionalOptions) @(SourceLinks->'/sourcelink:"%(Identity)"', ' ') - - - - - From 11db36087d2a56522b03e1953db36f0e3955d735 Mon Sep 17 00:00:00 2001 From: nickdalt Date: Mon, 6 Sep 2021 10:40:07 +0100 Subject: [PATCH 5/5] Review updates, Added unit tests for FindAdditionalSourceLinkFiles Tweaks to inheritance to handle Task type confusion in VS2022 --- src/Common/GetSourceLinkUrlGitTask.cs | 4 +- src/Common/TranslateRepositoryUrlGitTask.cs | 3 +- .../RepositoryTask.cs | 3 +- .../FindAdditionalSourceLinkFilesTests.cs | 136 ++++++++++++++++++ .../FindAdditionalSourceLinkFiles.cs | 35 +++-- .../GenerateSourceLinkFile.cs | 3 +- .../SourceLinkHasSingleProvider.cs | 3 +- .../build/Microsoft.SourceLink.Common.targets | 6 +- 8 files changed, 169 insertions(+), 24 deletions(-) create mode 100644 src/SourceLink.Common.UnitTests/FindAdditionalSourceLinkFilesTests.cs diff --git a/src/Common/GetSourceLinkUrlGitTask.cs b/src/Common/GetSourceLinkUrlGitTask.cs index 203fb7f7..9463993f 100644 --- a/src/Common/GetSourceLinkUrlGitTask.cs +++ b/src/Common/GetSourceLinkUrlGitTask.cs @@ -4,15 +4,13 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.Build.Framework; -using Microsoft.Build.Utilities; namespace Microsoft.Build.Tasks.SourceControl { - public abstract class GetSourceLinkUrlGitTask : Task + public abstract class GetSourceLinkUrlGitTask : Utilities.Task { private const string SourceControlName = "git"; protected const string NotApplicableValue = "N/A"; diff --git a/src/Common/TranslateRepositoryUrlGitTask.cs b/src/Common/TranslateRepositoryUrlGitTask.cs index f09822ef..76b57083 100644 --- a/src/Common/TranslateRepositoryUrlGitTask.cs +++ b/src/Common/TranslateRepositoryUrlGitTask.cs @@ -6,11 +6,10 @@ using System.Collections.Generic; using System.Linq; using Microsoft.Build.Framework; -using Microsoft.Build.Utilities; namespace Microsoft.Build.Tasks.SourceControl { - public class TranslateRepositoryUrlsGitTask : Task + public class TranslateRepositoryUrlsGitTask : Utilities.Task { private const string SourceControlName = "git"; diff --git a/src/Microsoft.Build.Tasks.Git/RepositoryTask.cs b/src/Microsoft.Build.Tasks.Git/RepositoryTask.cs index 62aa8cf2..1cbaf870 100644 --- a/src/Microsoft.Build.Tasks.Git/RepositoryTask.cs +++ b/src/Microsoft.Build.Tasks.Git/RepositoryTask.cs @@ -7,11 +7,10 @@ using System.IO; using System.Runtime.CompilerServices; using Microsoft.Build.Framework; -using Microsoft.Build.Utilities; namespace Microsoft.Build.Tasks.Git { - public abstract class RepositoryTask : Task + public abstract class RepositoryTask : Utilities.Task { // Include the assembly version in the key to avoid conflicts with other SourceLink versions. private static readonly string s_cacheKeyPrefix = $"3AE29AB7-AE6B-48BA-9851-98A15ED51C94:{typeof(RepositoryTask).Assembly.GetName().Version}:"; diff --git a/src/SourceLink.Common.UnitTests/FindAdditionalSourceLinkFilesTests.cs b/src/SourceLink.Common.UnitTests/FindAdditionalSourceLinkFilesTests.cs new file mode 100644 index 00000000..80d630da --- /dev/null +++ b/src/SourceLink.Common.UnitTests/FindAdditionalSourceLinkFilesTests.cs @@ -0,0 +1,136 @@ +// Licensed to the.NET Foundation under one or more agreements. +// The.NET Foundation licenses this file to you under the MIT license. +// See the License.txt file in the project root for more information. + +using System.IO; +using TestUtilities; + +using Xunit; + +namespace Microsoft.SourceLink.Common.UnitTests +{ + public class FindAdditionalSourceLinkFilesTests + { + [Fact] + public void NoSourceLinkFilesExpected() + { + var task = new FindAdditionalSourceLinkFiles() + { + SourceLinkFile = "merged.sourcelink.json", + ImportLibraries = new string[] { }, + AdditionalDependencies = new string[] { } + + }; + + bool result = task.Execute(); + + Assert.NotNull(task.AllSourceLinkFiles); + Assert.Single(task.AllSourceLinkFiles); + Assert.True(result); + } + + [Fact] + public void FoundSourceLinkForImportLib() + { + string testLib = "test.lib"; + string testSourceLink = "test.sourcelink.json"; + + using var temp = new TempRoot(); + var root = temp.CreateDirectory(); + var testDir = root.CreateDirectory("FoundSourceLinkForImportLib"); + var testLibFile = root.CreateFile(Path.Combine(testDir.Path, testLib)); + var testSourceLinkFile = root.CreateFile(Path.Combine(testDir.Path, testSourceLink)); + var task = new FindAdditionalSourceLinkFiles() + { + SourceLinkFile = "merged.sourcelink.json", + ImportLibraries = new string[] { testLibFile.Path }, + AdditionalDependencies = new string[] { } + + }; + + bool result = task.Execute(); + Assert.NotNull(task.AllSourceLinkFiles); + Assert.NotEmpty(task.AllSourceLinkFiles); +#pragma warning disable CS8602 // Dereference of a possibly null reference - previously checked + Assert.Equal(testSourceLinkFile.Path, task.AllSourceLinkFiles[1]); +#pragma warning restore CS8602 // Dereference of a possibly null reference. + Assert.True(result); + } + + [Fact] + public void FoundSourceLinkForNonRootedAdditionalDependency() + { + string testLib = "test.lib"; + string testSourceLink = "test.sourcelink.json"; + + using var temp = new TempRoot(); + var root = temp.CreateDirectory(); + var testDir = root.CreateDirectory("FoundSourceLinkForNonRootedAdditionalDependency"); + var testLibFile = root.CreateFile(Path.Combine(testDir.Path, testLib)); + var testSourceLinkFile = root.CreateFile(Path.Combine(testDir.Path, testSourceLink)); + var task = new FindAdditionalSourceLinkFiles() + { + SourceLinkFile = "merged.sourcelink.json", + ImportLibraries = new string[] { }, + AdditionalDependencies = new string[] { testLib }, + AdditionalLibraryDirectories = new string[] { testDir.Path } + }; + + bool result = task.Execute(); + Assert.NotNull(task.AllSourceLinkFiles); + Assert.NotEmpty(task.AllSourceLinkFiles); +#pragma warning disable CS8602 // Dereference of a possibly null reference - previously checked + Assert.Equal(testSourceLinkFile.Path, task.AllSourceLinkFiles[1]); +#pragma warning restore CS8602 // Dereference of a possibly null reference. + Assert.True(result); + } + + [Fact] + public void FoundSourceLinkForRootedAdditionalDependency() + { + string testLib = "test.lib"; + string testSourceLink = "test.sourcelink.json"; + + using var temp = new TempRoot(); + var root = temp.CreateDirectory(); + var testDir = root.CreateDirectory("FoundSourceLinkForRootedAdditionalDependency"); + var testLibFile = root.CreateFile(Path.Combine(testDir.Path, testLib)); + var testSourceLinkFile = root.CreateFile(Path.Combine(testDir.Path, testSourceLink)); + var task = new FindAdditionalSourceLinkFiles() + { + SourceLinkFile = "merged.sourcelink.json", + ImportLibraries = new string[] { }, + AdditionalDependencies = new string[] { testLibFile.Path }, + AdditionalLibraryDirectories = new string[] { } + }; + + bool result = task.Execute(); + + Assert.NotNull(task.AllSourceLinkFiles); + Assert.NotEmpty(task.AllSourceLinkFiles); +#pragma warning disable CS8602 // Dereference of a possibly null reference - previously checked + Assert.Equal(testSourceLinkFile.Path, task.AllSourceLinkFiles[1]); +#pragma warning restore CS8602 // Dereference of a possibly null reference. + Assert.True(result); + } + + [Fact] + public void SourceLinkError() + { + var task = new FindAdditionalSourceLinkFiles() + { + SourceLinkFile = "merged.sourcelink.json", + ImportLibraries = new string[] { }, +#pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type - deliberate to cause error + AdditionalDependencies = new string[] { null }, +#pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type. + AdditionalLibraryDirectories = new string[] { @"C:\Does\Not\Exist" } + }; + + bool result = task.Execute(); + Assert.NotNull(task.AllSourceLinkFiles); + Assert.Empty(task.AllSourceLinkFiles); + Assert.False(result); + } + } +} diff --git a/src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs b/src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs index 21123da5..deff873f 100644 --- a/src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs +++ b/src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs @@ -1,15 +1,14 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System.Diagnostics.CodeAnalysis; -using System.IO; -using System.Collections.Generic; using Microsoft.Build.Framework; -using Microsoft.Build.Utilities; using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.IO; namespace Microsoft.SourceLink.Common { - public sealed class FindAdditionalSourceLinkFiles : Task + public sealed class FindAdditionalSourceLinkFiles : Build.Utilities.Task { /// /// The name/path of the sourcelink file that we will merge into. @@ -45,7 +44,7 @@ public override bool Execute() try { - //// Throughout we expect that the sourcelink files for a lib is alongside + //// Throughout we expect that the sourcelink file for a lib is alongside //// the lib with the extension sourcelink.json instead of lib. // For import libraries we always have the full path to the lib. This shouldn't be needed since @@ -56,7 +55,11 @@ public override bool Execute() string sourceLinkName = Path.ChangeExtension(importLib, "sourcelink.json"); if (File.Exists(sourceLinkName)) { - Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); + if (BuildEngine != null) + { + Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); + } + allSourceLinkFiles.Add(sourceLinkName); } } @@ -71,7 +74,11 @@ public override bool Execute() // on that path. if (File.Exists(sourceLinkName)) { - Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); + if (BuildEngine != null) + { + Log.LogMessage("Found additional sourcelink file '{0}'", sourceLinkName); + } + allSourceLinkFiles.Add(sourceLinkName); } } @@ -84,7 +91,11 @@ public override bool Execute() string potentialPath = Path.Combine(libDir, sourceLinkName); if (File.Exists(potentialPath)) { - Log.LogMessage("Found additional sourcelink file '{0}'", potentialPath); + if (BuildEngine != null) + { + Log.LogMessage("Found additional sourcelink file '{0}'", potentialPath); + } + allSourceLinkFiles.Add(potentialPath); break; } @@ -97,7 +108,11 @@ public override bool Execute() } catch (Exception ex) { - Log.LogError("Failed to find sourcelink files for libs with dll/exe sourcelink file - {0}", ex.Message); + AllSourceLinkFiles = new string[] { }; + if (BuildEngine != null) + { + Log.LogError("Failed to find sourcelink files for libs with dll/exe sourcelink file - {0}", ex.Message); + } } return false; diff --git a/src/SourceLink.Common/GenerateSourceLinkFile.cs b/src/SourceLink.Common/GenerateSourceLinkFile.cs index 124c937a..4a968b6e 100644 --- a/src/SourceLink.Common/GenerateSourceLinkFile.cs +++ b/src/SourceLink.Common/GenerateSourceLinkFile.cs @@ -9,11 +9,10 @@ using System.Text; using Microsoft.Build.Framework; using Microsoft.Build.Tasks.SourceControl; -using Microsoft.Build.Utilities; namespace Microsoft.SourceLink.Common { - public sealed class GenerateSourceLinkFile : Task + public sealed class GenerateSourceLinkFile : Build.Utilities.Task { [Required, NotNull] public ITaskItem[]? SourceRoots { get; set; } diff --git a/src/SourceLink.Common/SourceLinkHasSingleProvider.cs b/src/SourceLink.Common/SourceLinkHasSingleProvider.cs index 98a69084..a14c5dd1 100644 --- a/src/SourceLink.Common/SourceLinkHasSingleProvider.cs +++ b/src/SourceLink.Common/SourceLinkHasSingleProvider.cs @@ -4,11 +4,10 @@ using System; using Microsoft.Build.Framework; -using Microsoft.Build.Utilities; namespace Microsoft.SourceLink.Common { - public sealed class SourceLinkHasSingleProvider : Task + public sealed class SourceLinkHasSingleProvider : Build.Utilities.Task { public string? ProviderTargets { get; set; } diff --git a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets index f6c69796..1dbeaa26 100644 --- a/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets +++ b/src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets @@ -32,7 +32,7 @@ - + <_GenerateSourceLinkFileBeforeTargets>BeforeClCompile <_GenerateSourceLinkFileDependsOnTargets/> @@ -40,7 +40,7 @@ - + <_GenerateSourceLinkFileBeforeTargets>Link <_GenerateSourceLinkFileDependsOnTargets>ComputeLinkSwitches @@ -73,7 +73,7 @@ - + %(Link.AdditionalOptions) @(SourceLinks->'/sourcelink:"%(Identity)"', ' ')