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

Support for processing metaproj references from a solution file #1259

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using System;
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using System;
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.Build.Framework;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using System.IO;
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.IO;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
using Newtonsoft.Json;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using System;
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;

namespace NuGet.Build.Tasks
{
/// <summary>
/// Convert .metaproj paths to project paths.
/// </summary>
public class GetRestoreSolutionProjectsTask : Task
{
private const string MetaProjExtension = ".metaproj";

/// <summary>
/// Solution project references.
/// </summary>
[Required]
public ITaskItem[] ProjectReferences { get; set; }

/// <summary>
/// Root path used for resolving the absolute project paths.
/// </summary>
[Required]
public string SolutionFilePath { get; set; }

/// <summary>
/// Output items
/// </summary>
[Output]
public ITaskItem[] OutputProjectReferences { get; set; }

public override bool Execute()
{
// Log inputs
var log = new MSBuildLogger(Log);
log.LogDebug($"(in) ProjectReferences '{string.Join(";", ProjectReferences.Select(p => p.ItemSpec))}'");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can ItemSpec be null or empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

It can't be null

Copy link
Contributor

Choose a reason for hiding this comment

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

Then it's inconsistent with line 50 where you check the same value for null.

Copy link
Member Author

Choose a reason for hiding this comment

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

It checks for empty also

log.LogDebug($"(in) SolutionFilePath '{SolutionFilePath}'");

var entries = new List<ITaskItem>();
var parentDirectory = Path.GetDirectoryName(SolutionFilePath);

foreach (var project in ProjectReferences)
{
if (string.IsNullOrEmpty(project.ItemSpec))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it fail or warn on invalid value?

Copy link
Member Author

@emgarten emgarten Mar 23, 2017

Choose a reason for hiding this comment

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

It should not be, but if it is the error/warning would be very unhelpful since it would not refer to anything. This protects against that.

{
continue;
}

var projectPath = Path.GetFullPath(Path.Combine(parentDirectory, project.ItemSpec));

// Check for the metaproj extension, this is auto generated by solutions instead of
// the actual project path when build order is set. For the purpose of restore
// the order is not important so we remove the extension to get the real project path.
if (projectPath.EndsWith(MetaProjExtension, StringComparison.OrdinalIgnoreCase))
{
// Remove .metaproj from the path.
projectPath = projectPath.Substring(0, projectPath.Length - MetaProjExtension.Length);
Copy link
Contributor

Choose a reason for hiding this comment

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

can you instead do Path.Combine(Path.GetDirectoryName(file), Path.GetFileNameWithoutExtension(file) )

Copy link
Member Author

Choose a reason for hiding this comment

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

I did that initially but simplified to this. We know here exactly what needs to happen, an extra helper isn't needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Either way would work fine though

Copy link
Contributor

Choose a reason for hiding this comment

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

i'd prefer being more verbose and call the helpers but your call.


In reply to: 107747427 [](ancestors = 107747427)

}

// Clone items using the modified path
var newEntry = new TaskItem(projectPath, project.CloneCustomMetadata());
entries.Add(newEntry);
}

OutputProjectReferences = entries.ToArray();

return true;
}
}
}
29 changes: 17 additions & 12 deletions src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<UsingTask TaskName="NuGet.Build.Tasks.GetRestorePackageReferencesTask" AssemblyFile="$(RestoreTaskAssemblyFile)" />
<UsingTask TaskName="NuGet.Build.Tasks.GetRestoreDotnetCliToolsTask" AssemblyFile="$(RestoreTaskAssemblyFile)" />
<UsingTask TaskName="NuGet.Build.Tasks.GetRestoreProjectFrameworks" AssemblyFile="$(RestoreTaskAssemblyFile)" />
<UsingTask TaskName="NuGet.Build.Tasks.GetRestoreSolutionProjectsTask" AssemblyFile="$(RestoreTaskAssemblyFile)" />

<!--
============================================================
Expand Down Expand Up @@ -130,7 +131,7 @@ Copyright (c) .NET Foundation. All rights reserved.
RestoreRecursive="$(RestoreRecursive)" />
</Target>

<!--
<!--
============================================================
_LoadRestoreGraphEntryPoints
Find project entry points and load them into items.
Expand All @@ -143,15 +144,20 @@ Copyright (c) .NET Foundation. All rights reserved.
<RestoreGraphProjectInputItems Include="$(RestoreGraphProjectInput)" />
</ItemGroup>

<!-- Solution case -->
<ItemGroup Condition=" $(MSBuildProjectFullPath.EndsWith('.metaproj')) == 'true' AND @(RestoreGraphProjectInputItems) == '' ">
<RestoreGraphProjectInputItems Include="@(ProjectReference)" />
</ItemGroup>

<!-- Project case -->
<ItemGroup Condition=" $(MSBuildProjectFullPath.EndsWith('.metaproj')) != 'true' AND @(RestoreGraphProjectInputItems) == '' ">
<RestoreGraphProjectInputItems Include="$(MSBuildProjectFullPath)" />
</ItemGroup>

<!-- Solution case -->
<GetRestoreSolutionProjectsTask
Condition=" $(MSBuildProjectFullPath.EndsWith('.metaproj')) == 'true' AND @(RestoreGraphProjectInputItems) == '' "
ProjectReferences="@(ProjectReference)"
SolutionFilePath="$(MSBuildProjectFullPath)">
<Output
TaskParameter="OutputProjectReferences"
ItemName="RestoreGraphProjectInputItems" />
</GetRestoreSolutionProjectsTask>
</Target>

<!--
Expand Down Expand Up @@ -188,7 +194,7 @@ Copyright (c) .NET Foundation. All rights reserved.
AND '%(RestoreGraphProjectInputItems.Extension)' != '.vcxitems'
AND '%(RestoreGraphProjectInputItems.Extension)' != '' " />
</ItemGroup>

<!-- No filtering -->
<ItemGroup Condition=" '$(RestoreProjectFilterMode)' != 'exclusionlist' AND '$(RestoreProjectFilterMode)' != 'inclusionlist' ">
<_FilteredRestoreGraphProjectInputItemsTmp
Expand All @@ -210,7 +216,7 @@ Copyright (c) .NET Foundation. All rights reserved.
Entry point for creating the project to project restore graph.
============================================================
-->
<Target Name="_GenerateRestoreGraph"
<Target Name="_GenerateRestoreGraph"
DependsOnTargets="_FilterRestoreGraphProjectInputItems;_GetAllRestoreProjectPathItems"
Returns="@(_RestoreGraphEntry)">
<Message Text="Generating dg file" Importance="low" />
Expand Down Expand Up @@ -321,7 +327,7 @@ Copyright (c) .NET Foundation. All rights reserved.
Returns="$(_CurrentProjectJsonPath)">
<!-- Get project.json path -->
<!-- Skip this if the project style is already set. -->
<GetRestoreProjectJsonPathTask
<GetRestoreProjectJsonPathTask
ProjectPath="$(MSBuildProjectFullPath)"
Condition=" '$(RestoreProjectStyle)' == 'ProjectJson' OR '$(RestoreProjectStyle)' == '' ">
<Output TaskParameter="ProjectJsonPath" PropertyName="_CurrentProjectJsonPath" />
Expand Down Expand Up @@ -452,12 +458,12 @@ Copyright (c) .NET Foundation. All rights reserved.
<PropertyGroup Condition=" '$(RestoreProjectStyle)' == 'PackageReference' AND '$(TargetFrameworks)' != '' ">
<_RestoreCrossTargeting>true</_RestoreCrossTargeting>
</PropertyGroup>

<!-- Determine if ContentFiles should be written by NuGet -->
<PropertyGroup Condition=" '$(RestoreProjectStyle)' == 'PackageReference' AND '$(_RestoreSkipContentFileWrite)' == '' ">
<_RestoreSkipContentFileWrite Condition=" '$(TargetFrameworks)' == '' AND '$(TargetFramework)' == '' ">true</_RestoreSkipContentFileWrite>
</PropertyGroup>

<!-- Write properties for the top level entry point -->
<ItemGroup Condition=" '$(RestoreProjectStyle)' == 'PackageReference' ">
<_RestoreGraphEntry Include="$([System.Guid]::NewGuid())">
Expand Down Expand Up @@ -607,7 +613,6 @@ Copyright (c) .NET Foundation. All rights reserved.
============================================================
-->
<Target Name="_GenerateRestoreProjectPathItemsPerFramework"
DependsOnTargets="_SplitProjectReferencesByFileExistence"
Returns="@(_RestoreProjectPathItems)">

<!-- Get the absolute paths to all projects -->
Expand Down
5 changes: 4 additions & 1 deletion src/NuGet.Core/NuGet.Build.Tasks/WriteRestoreGraphTask.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using System.IO;
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.IO;
using System.Linq;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
Expand Down