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.

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

Add support for C++ libs into sourcelink #605

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
106 changes: 106 additions & 0 deletions src/SourceLink.Common/FindAdditionalSourceLinkFiles.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// 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;

namespace Microsoft.SourceLink.Common
{
public sealed class FindAdditionalSourceLinkFiles : Task
{
/// <summary>
/// The name/path of the sourcelink file that we will merge into.
/// </summary>
[Required, NotNull]
public string? SourceLinkFile { get; set; }

/// <summary>
/// Collection of all the library directories that will be searched for lib files.
/// </summary>
[Required, NotNull]
public string[]? AdditionalLibraryDirectories { get; set; }

/// <summary>
/// Collection of all the libs that we will link to.
/// </summary>
[Required, NotNull]
public string[]? AdditionalDependencies { get; set; }

/// <summary>
/// Collection of solution referenced import libraries.
/// </summary>
[Required, NotNull]
public string[]? ImportLibraries { get; set; }

[Output]
public string[]? AllSourceLinkFiles { get; set; }

public override bool Execute()
{
List<string> allSourceLinkFiles = new List<string>();
allSourceLinkFiles.Add(SourceLinkFile);

try
{
//// Throughout we expect that the sourcelink files for a lib is alongside
Copy link
Member

Choose a reason for hiding this comment

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

s

typo: file

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

//// 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);
allSourceLinkFiles.Add(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);
allSourceLinkFiles.Add(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);
allSourceLinkFiles.Add(potentialPath);
break;
}
}
}
}

AllSourceLinkFiles = allSourceLinkFiles.ToArray();
return true;
}
catch (Exception ex)
{
Log.LogError("Failed to find sourcelink files for libs with dll/exe sourcelink file - {0}", ex.Message);
}

return false;
}
}
}
28 changes: 22 additions & 6 deletions src/SourceLink.Common/build/Microsoft.SourceLink.Common.targets
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<Import Project="InitializeSourceControlInformation.targets"/>

<UsingTask TaskName="Microsoft.SourceLink.Common.GenerateSourceLinkFile" AssemblyFile="$(_MicrosoftSourceLinkCommonAssemblyFile)"/>
<UsingTask TaskName="Microsoft.SourceLink.Common.FindAdditionalSourceLinkFiles" AssemblyFile="$(_MicrosoftSourceLinkCommonAssemblyFile)"/>

<Target Name="_SetSourceLinkFilePath">
<PropertyGroup>
Expand All @@ -27,10 +28,18 @@
DependsOnTargets="InitializeSourceRootMappedPaths"
Condition="'$(SourceRootMappedPathsFeatureSupported)' == 'true'"/>

<!--
Add compiler targets: C++ generates sourcelink file only for static libs.
-->
<PropertyGroup Condition="'$(Language)' == 'C++' and $(ConfigurationType) == 'StaticLibrary'">
Copy link
Member

@tmat tmat Aug 12, 2021

Choose a reason for hiding this comment

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

$(ConfigurationType)

Missing ' ' around $(ConfigurationType).

Copy link
Author

Choose a reason for hiding this comment

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

Done

<_GenerateSourceLinkFileBeforeTargets>BeforeClCompile</_GenerateSourceLinkFileBeforeTargets>
<_GenerateSourceLinkFileDependsOnTargets/>
</PropertyGroup>

<!--
Add compiler targets: C++ generates PDB with SourceLink in Link phase.
-->
<PropertyGroup Condition="'$(Language)' == 'C++'">
<PropertyGroup Condition="'$(Language)' == 'C++' and $(ConfigurationType) != 'StaticLibrary'">
Copy link
Member

@tmat tmat Aug 12, 2021

Choose a reason for hiding this comment

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

$(ConfigurationType)

Missing ' ' around $(ConfigurationType).

Copy link
Author

Choose a reason for hiding this comment

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

Done

<_GenerateSourceLinkFileBeforeTargets>Link</_GenerateSourceLinkFileBeforeTargets>
<_GenerateSourceLinkFileDependsOnTargets>ComputeLinkSwitches</_GenerateSourceLinkFileDependsOnTargets>
</PropertyGroup>
Expand All @@ -45,20 +54,27 @@
This target shall initialize SourceLinkUrl of all items that don't have it initialized yet and belong to the source control provider.
-->
<Target Name="_GenerateSourceLinkFile"
DependsOnTargets="_SetSourceLinkFilePath;$(_GenerateSourceLinkFileDependsOnTargets);_InitializeSourceRootMappedPathsOpt;$(SourceLinkUrlInitializerTargets)"
DependsOnTargets="_SetSourceLinkFilePath;$(_GenerateSourceLinkFileDependsOnTargets); _InitializeSourceRootMappedPathsOpt;$(SourceLinkUrlInitializerTargets)"
Condition="'$(EnableSourceLink)' == 'true' and '$(DebugType)' != 'none'"
Outputs="$(SourceLink)">

<Microsoft.SourceLink.Common.GenerateSourceLinkFile SourceRoots="@(SourceRoot)" OutputFile="$(SourceLink)" />

<ItemGroup>
<FileWrites Include="$(SourceLink)" />
</ItemGroup>

<!-- C++ Link task currently doesn't recognize SourceLink property -->
<ItemGroup Condition="'$(Language)' == 'C++'">
<!-- Locate any additional sourcelink files associated with static libs -->
<Microsoft.SourceLink.Common.FindAdditionalSourceLinkFiles SourceLinkFile="$(SourceLink)" AdditionalLibraryDirectories="%(Link.AdditionalLibraryDirectories)" AdditionalDependencies="%(Link.AdditionalDependencies)" ImportLibraries="@(ProjectReferenceToLink)">
<Output TaskParameter="AllSourceLinkFiles" ItemName="SourceLinks" />
</Microsoft.SourceLink.Common.FindAdditionalSourceLinkFiles>

<!-- C++ Link task currently doesn't recognize SourceLink property only add this for non-static libs since lib doesn't
understand /sourcelink
-->
<ItemGroup Condition="'$(Language)' == 'C++' and $(ConfigurationType) != 'StaticLibrary'">
Copy link
Member

Choose a reason for hiding this comment

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

$(ConfigurationType)

Missing ' ' around $(ConfigurationType).

Copy link
Author

Choose a reason for hiding this comment

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

Done

<Link Update="@(Link)">
<AdditionalOptions>%(Link.AdditionalOptions) /sourcelink:"$(SourceLink)"</AdditionalOptions>
<AdditionalOptions>%(Link.AdditionalOptions) @(SourceLinks->'/sourcelink:&quot;%(Identity)&quot;', ' ')</AdditionalOptions>
Copy link
Member

Choose a reason for hiding this comment

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

Identity

The values need to be escaped before they can be used as command line arguments. Perhaps, for simplicity of msbuild, FindAdditionalSourceLinkFiles could instead produce the command line string instead of individual items?

</Link>
</ItemGroup>
</Target>
Expand Down