Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,4 @@ jobs:
if: success() && matrix.os == 'ubuntu-latest'
with:
name: artifacts
path: src/dotnet-affected/bin/Debug/net7.0/
path: src/dotnet-affected/bin/Debug/net8.0/
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<Project>
<PropertyGroup>
<TargetFrameworks>net7.0;net6.0;netcoreapp3.1</TargetFrameworks>
<TargetFrameworks>net8.0;net7.0;net6.0</TargetFrameworks>

<LangVersion>9.0</LangVersion>

Expand Down
8 changes: 8 additions & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,12 @@
<PackageVersion Include="System.CodeDom" Version="7.0.0" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net8.0'">
<PackageVersion Include="Microsoft.Build" Version="17.5.0" />
<PackageVersion Include="Microsoft.Build.Framework" Version="17.5.0" />
<PackageVersion Include="Microsoft.Build.Utilities.Core" Version="17.5.0" />
<PackageVersion Include="System.Configuration.ConfigurationManager" Version="8.0.0" />
<PackageVersion Include="System.CodeDom" Version="8.0.0" />
</ItemGroup>

</Project>
2 changes: 1 addition & 1 deletion global.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"sdk": {
"version": "7.0.101",
"version": "8.0.100",
"allowPrerelease": true
}
}
4 changes: 1 addition & 3 deletions src/DotnetAffected.Core/AffectedExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ public AffectedExecutor(
}

/// <inheritdoc />
public AffectedSummary Execute() => GitChangesProvider.MsBuildFileSystemSupported
? new AffectedProcessor().Process(_context)
: new AffectedProcessorLegacy().Process(_context);
public AffectedSummary Execute() => new AffectedProcessor().Process(_context);
}
}
85 changes: 2 additions & 83 deletions src/DotnetAffected.Core/GitChangesProvider.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
using DotnetAffected.Abstractions;
using DotnetAffected.Core.FileSystem;
using LibGit2Sharp;
using Microsoft.Build.Construction;
using Microsoft.Build.Definition;
using Microsoft.Build.Evaluation;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Runtime.InteropServices;
using System.Text;
using System.Xml;

namespace DotnetAffected.Core
{
Expand All @@ -21,22 +16,6 @@ public class GitChangesProvider : IChangesProvider
{
internal static readonly bool IsWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);

private static readonly Lazy<bool> ResolveMsBuildFileSystemSupported = new Lazy<bool>(() =>
{
var versionInfo = FileVersionInfo.GetVersionInfo(typeof(Project).Assembly.Location);
if (versionInfo.FileMajorPart > 16)
return true;
if (versionInfo.FileMajorPart < 16)
return false;
return versionInfo.FileMinorPart >= 10;
});

/// <summary>
/// When true, the build system supports virtual filesystem which means nested Directory.Packages.props files
/// are supported in central package management.
/// </summary>
public static bool MsBuildFileSystemSupported => ResolveMsBuildFileSystemSupported.Value;

/// <inheritdoc />
public IEnumerable<string> GetChangedFiles(string directory, string from, string to)
{
Expand All @@ -52,7 +31,7 @@ public IEnumerable<string> GetChangedFiles(string directory, string from, string
bool fallbackToHead)
{
var project = LoadProject(directory, pathToFile, commitRef, fallbackToHead);
if (project is null && MsBuildFileSystemSupported)
if (project is null)
{
var fi = new FileInfo(pathToFile);
var parent = fi.Directory?.Parent?.FullName;
Expand All @@ -67,9 +46,7 @@ public IEnumerable<string> GetChangedFiles(string directory, string from, string
/// <inheritdoc />
public Project? LoadProject(string directory, string pathToFile, string? commitRef, bool fallbackToHead)
{
return MsBuildFileSystemSupported
? LoadProjectCore(directory, pathToFile, commitRef, fallbackToHead)
: LoadProjectLegacy(directory, pathToFile, commitRef, fallbackToHead);
return LoadProjectCore(directory, pathToFile, commitRef, fallbackToHead);
}

private Project? LoadProjectCore(string directory, string pathToFile, string? commitRef, bool fallbackToHead)
Expand All @@ -96,64 +73,6 @@ public IEnumerable<string> GetChangedFiles(string directory, string from, string
return fs.FileExists(pathToFile) ? fs.CreateProjectAndEagerLoadChildren(pathToFile) : null;
}

private Project? LoadProjectLegacy(string directory, string pathToFile, string? commitRef, bool fallbackToHead)
{
Commit? commit;

using var repository = new Repository(directory);

if (string.IsNullOrWhiteSpace(commitRef))
commit = fallbackToHead ? repository.Head.Tip : null;
else
commit = GetCommitOrThrow(repository, commitRef);

Stream GenerateStreamFromString(string s)
{
var stream = new MemoryStream();
var writer = new StreamWriter(stream);
writer.Write(s);
writer.Flush();
stream.Position = 0;
return stream;
}

var projectCollection = new ProjectCollection();

if (commit is null)
{
var path = Path.Combine(directory, pathToFile);
if (!File.Exists(path)) return null;

using var reader = new XmlTextReader(GenerateStreamFromString(File.ReadAllText(path)));
var projectRootElement = ProjectRootElement.Create(reader);
projectRootElement.FullPath = pathToFile;
return Project.FromProjectRootElement(projectRootElement, new ProjectOptions
{
LoadSettings = ProjectLoadSettings.Default, ProjectCollection = projectCollection,
});
}
else
{
var path = IsWindows
? Path.GetRelativePath(directory, pathToFile)
.Replace('\\', '/')
: Path.GetRelativePath(directory, pathToFile);
var treeEntry = commit[path];
if (treeEntry == null) return null;

var blob = (Blob)treeEntry.Target;

using var content = new StreamReader(blob.GetContentStream(), Encoding.UTF8);
using var reader = new XmlTextReader(GenerateStreamFromString(content.ReadToEnd()));
var projectRootElement = ProjectRootElement.Create(reader);
projectRootElement.FullPath = pathToFile;
return Project.FromProjectRootElement(projectRootElement, new ProjectOptions
{
LoadSettings = ProjectLoadSettings.Default, ProjectCollection = projectCollection,
});
}
}

private static (Commit? From, Commit To) ParseRevisionRanges(
Repository repository,
string from,
Expand Down
80 changes: 0 additions & 80 deletions src/DotnetAffected.Core/Processor/AffectedProcessorLegacy.cs

This file was deleted.

3 changes: 2 additions & 1 deletion src/DotnetAffected.Core/ProjectFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ public Project CreateProject(string projectRootElementFilePath)
var loaded = ProjectCollection.GetLoadedProjects(projectRootElementFilePath);
if (loaded.Any())
return loaded.Single();

var projectRootElement = CreateProjectRootElement(projectRootElementFilePath);
// TODO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When running these tests locally I more or less always (but I have also seen it succeed I think) get this error for the net8.0 target. All other targets pass just fine:

Microsoft.Build.Exceptions.InvalidProjectFileException
A numeric comparison was attempted on "$(_TargetFrameworkVersionWithoutV)" that evaluates to "" instead of a number, in condition "'$(PublishRelease)' == '' and '$(_TargetFrameworkVersionWithoutV)' >= '8.0'".  /usr/local/share/dotnet/sdk/8.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.DefaultItems.targets
   at Microsoft.Build.Shared.ProjectErrorUtilities.ThrowInvalidProject(String errorSubCategoryResourceName, IElementLocation elementLocation, String resourceName, Object[] args)
   at Microsoft.Build.Shared.ProjectErrorUtilities.ThrowInvalidProject[T1,T2,T3](IElementLocation elementLocation, String resourceName, T1 arg0, T2 arg1, T3 arg2)
   at Microsoft.Build.Evaluation.NumericComparisonExpressionNode.BoolEvaluate(IConditionEvaluationState state, LoggingContext loggingContext)
   at Microsoft.Build.Evaluation.OperatorExpressionNode.TryBoolEvaluate(IConditionEvaluationState state, Boolean& result, LoggingContext loggingContext)
   at Microsoft.Build.Evaluation.AndExpressionNode.BoolEvaluate(IConditionEvaluationState state, LoggingContext loggingContext)
   at Microsoft.Build.Evaluation.OperatorExpressionNode.TryBoolEvaluate(IConditionEvaluationState state, Boolean& result, LoggingContext loggingContext)
   at Microsoft.Build.Evaluation.GenericExpressionNode.Evaluate(IConditionEvaluationState state, LoggingContext loggingContext)
   at Microsoft.Build.Evaluation.ConditionEvaluator.EvaluateConditionCollectingConditionedProperties[P,I](String condition, ParserOptions options, Expander`2 expander, ExpanderOptions expanderOptions, Dictionary`2 conditionedPropertiesTable, String evaluationDirectory, ElementLocation elementLocation, ILoggingService loggingServices, BuildEventContext buildEventContext, IFileSystem fileSystem, ProjectRootElementCacheBase projectRootElementCache, LoggingContext loggingContext)
   at Microsoft.Build.Evaluation.Evaluator`4.EvaluateConditionCollectingConditionedProperties(ProjectElement element, String condition, ExpanderOptions expanderOptions, ParserOptions parserOptions, ProjectRootElementCacheBase projectRootElementCache)
   at Microsoft.Build.Evaluation.Evaluator`4.EvaluatePropertyGroupElement(ProjectPropertyGroupElement propertyGroupElement)
   at Microsoft.Build.Evaluation.Evaluator`4.PerformDepthFirstPass(ProjectRootElement currentProjectOrImport)
   at Microsoft.Build.Evaluation.Evaluator`4.EvaluateImportElement(String directoryOfImportingFile, ProjectImportElement importElement)
   at Microsoft.Build.Evaluation.Evaluator`4.PerformDepthFirstPass(ProjectRootElement currentProjectOrImport)
   at Microsoft.Build.Evaluation.Evaluator`4.EvaluateImportElement(String directoryOfImportingFile, ProjectImportElement importElement)
   at Microsoft.Build.Evaluation.Evaluator`4.PerformDepthFirstPass(ProjectRootElement currentProjectOrImport)
   at Microsoft.Build.Evaluation.Evaluator`4.EvaluateImportElement(String directoryOfImportingFile, ProjectImportElement importElement)
   at Microsoft.Build.Evaluation.Evaluator`4.PerformDepthFirstPass(ProjectRootElement currentProjectOrImport)
   at Microsoft.Build.Evaluation.Evaluator`4.Evaluate()
   at Microsoft.Build.Evaluation.Evaluator`4.Evaluate(IEvaluatorData`4 data, Project project, ProjectRootElement root, ProjectLoadSettings loadSettings, Int32 maxNodeCount, PropertyDictionary`1 environmentProperties, ILoggingService loggingService, IItemFactory`2 itemFactory, IToolsetProvider toolsetProvider, IDirectoryCacheFactory directoryCacheFactory, ProjectRootElementCacheBase projectRootElementCache, BuildEventContext buildEventContext, ISdkResolverService sdkResolverService, Int32 submissionId, EvaluationContext evaluationContext, Boolean interactive)
   at Microsoft.Build.Evaluation.Project.ProjectImpl.Reevaluate(ILoggingService loggingServiceForEvaluation, ProjectLoadSettings loadSettings, EvaluationContext evaluationContext)
   at Microsoft.Build.Evaluation.Project.ProjectImpl.ReevaluateIfNecessary(ILoggingService loggingServiceForEvaluation, ProjectLoadSettings loadSettings, EvaluationContext evaluationContext)
   at Microsoft.Build.Evaluation.Project.ProjectImpl.Initialize(IDictionary`2 globalProperties, String toolsVersion, String subToolsetVersion, ProjectLoadSettings loadSettings, EvaluationContext evaluationContext, Boolean interactive)
   at Microsoft.Build.Evaluation.Project.FromProjectRootElement(ProjectRootElement rootElement, ProjectOptions options)
   at DotnetAffected.Core.ProjectFactory.CreateProject(String projectRootElementFilePath) in /Users/trond/Code/dotnet-affected/src/DotnetAffected.Core/ProjectFactory.cs:line 48
   at DotnetAffected.Core.FileSystem.EagerCachingMsBuildGitFileSystem.CreateProjectAndEagerLoadChildren(String path) in /Users/trond/Code/dotnet-affected/src/DotnetAffected.Core/FileSystem/EagerCachingMsBuildGitFileSystem.cs:line 49
   at DotnetAffected.Core.GitChangesProvider.LoadProjectCore(String directory, String pathToFile, String commitRef, Boolean fallbackToHead) in /Users/trond/Code/dotnet-affected/src/DotnetAffected.Core/GitChangesProvider.cs:line 73
   at DotnetAffected.Core.GitChangesProvider.LoadProject(String directory, String pathToFile, String commitRef, Boolean fallbackToHead) in /Users/trond/Code/dotnet-affected/src/DotnetAffected.Core/GitChangesProvider.cs:line 49
   at DotnetAffected.Core.Processor.AffectedProcessor.FindChangedNugetPackages(AffectedProcessorContext context) in /Users/trond/Code/dotnet-affected/src/DotnetAffected.Core/Processor/AffectedProcessor.cs:line 114
   at DotnetAffected.Core.Processor.AffectedProcessor.DiscoverPackageChanges(AffectedProcessorContext context) in /Users/trond/Code/dotnet-affected/src/DotnetAffected.Core/Processor/AffectedProcessor.cs:line 30
   at DotnetAffected.Core.Processor.AffectedProcessorBase.Process(AffectedProcessorContext context) in /Users/trond/Code/dotnet-affected/src/DotnetAffected.Core/Processor/AffectedProcessorBase.cs:line 33
   at DotnetAffected.Core.AffectedExecutor.Execute() in /Users/trond/Code/dotnet-affected/src/DotnetAffected.Core/AffectedExecutor.cs:line 41
   at DotnetAffected.Core.Tests.BaseDotnetAffectedTest.<.ctor>b__1_0() in /Users/trond/Code/dotnet-affected/test/DotnetAffected.Core.Tests/Utils/BaseDotnetAffectedTest.cs:line 16
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at DotnetAffected.Core.Tests.BaseDotnetAffectedTest.get_AffectedSummary() in /Users/trond/Code/dotnet-affected/test/DotnetAffected.Core.Tests/Utils/BaseDotnetAffectedTest.cs:line 24
   at DotnetAffected.Core.Tests.CentralPackageManagementDetectionNestedTests.When_directory_packages_props_changes_without_dependant_projects_nothing_should_be_affected() in /Users/trond/Code/dotnet-affected/test/DotnetAffected.Core.Tests/CentralPackageManagementDetectionNestedTests.cs:line 137
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

I'm not sure why this only fails on net8.0 and not net6.0 and net7.0. What seems to solve it is to add this to the ProjectOptions below:

new ProjectOptions
{
    GlobalProperties = new Dictionary<string, string>{ {"TargetFramework", "net8.0"} },
    // ... existing ProjectOptions
}

The tests for non-net8.0 frameworks still pass with this in place but I'm not sure this is the correct fix. Any thoughts on this?

Copy link
Owner

Choose a reason for hiding this comment

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

Hi @tanordheim , as you pointed out they were failing due to missing TargetFramework.
I've added it in the sample project generation in tests and that seems to work

return Project
.FromProjectRootElement(projectRootElement, new ProjectOptions
{
Expand Down
5 changes: 3 additions & 2 deletions src/DotnetAffected.Tasks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ as they don't actually build projects, they just delegate the build to a differe
own configuration.

However, `DotnetAffected.Tasks` itself contains code to execute in build time, which
support TFMs `netcore3.1`, `net6.0` and `net7.0`.
support TFMs `net6.0`, `net7.0` and `net8.0`.

The TFM must be known so the proper TFM facing assembly is used.

Expand All @@ -265,6 +265,7 @@ In most cases it is automatically resolved using the following logic:
- If >= `17.0.0` it will resolve to `net6.0`
- Else if >= `16.11.0` it will resolve to `net5.0`
- Else it will resolve to `netcoreapp3.1`
- TODO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how true this list is at the moment, I'm not that intimately familiar with how all of this stuff works. How would you suggest I change this?


If you have issues, you can override the logic by specifically setting the `<TargetFramework>`.

Expand All @@ -278,5 +279,5 @@ If you have issues, you can override the logic by specifically setting the `<Tar
```

> `DotnetAffected.Tasks` provides MSBuild integration using `DotnetAffected.Core` under the hood.
`DotnetAffected.Core` support TFMs `netcore3.1`, `net6.0` and `net7.0`.
`DotnetAffected.Core` support TFMs `net6.0`, `net7.0` and `net8.0`.

5 changes: 2 additions & 3 deletions src/DotnetAffected.Tasks/Sdk/Sdk.props
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

<!-- TargetFramework is not set by default in all project styles, especially traversal like projects
However, we need it to properly load the assembly

We respect user input, if explicilty set in <TargetFramework>
If not, we auto-detect it and assign it to <TargetFramework>

Later in .targets, we _DotnetAffectedTargetFramework to load the assembly but if TargetFramework is not empty
we will use it, this is to allow late modification for TargetFramework down the pipe
-->
Expand All @@ -19,7 +19,6 @@
<_DotnetAffectedTargetFramework Condition="'$(_DotnetAffectedTargetFramework)' == ''">$(MicrosoftNETBuildTasksTFM)</_DotnetAffectedTargetFramework>
<_DotnetAffectedTargetFramework Condition="'$(_DotnetAffectedTargetFramework)' == '' And $([MSBuild]::VersionGreaterThanOrEquals('$(MSBuildVersion)', '17.0.0'))">net6.0</_DotnetAffectedTargetFramework>
<_DotnetAffectedTargetFramework Condition="'$(_DotnetAffectedTargetFramework)' == '' And $([MSBuild]::VersionGreaterThanOrEquals('$(MSBuildVersion)', '16.11.0'))">net5.0</_DotnetAffectedTargetFramework>
<_DotnetAffectedTargetFramework Condition="'$(_DotnetAffectedTargetFramework)' == ''">netcoreapp3.1</_DotnetAffectedTargetFramework>

<TargetFramework>$(_DotnetAffectedTargetFramework)</TargetFramework>
</PropertyGroup>
Expand Down
1 change: 0 additions & 1 deletion src/dotnet-affected/dotnet-affected.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
<PackageReference Include="Microsoft.Build.Framework" ExcludeAssets="runtime" />
<PackageReference Include="Microsoft.Build.Utilities.Core" ExcludeAssets="runtime" />
<PackageReference Include="Microsoft.Build.Locator" />
<PackageReference Condition="'$(TargetFramework)' == 'netcoreapp3.1'" Include="Microsoft.Win32.Registry" />
<PackageReference Include="System.CommandLine" />
<PackageReference Include="System.CommandLine.Rendering" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,8 @@ public static async Task PrepareTaskInfra(this TemporaryRepository repo, string
await repo.CreateTextFileAsync("ci.props", TestProjectScenarios.CiProps);

var hasImportResource = !string.IsNullOrWhiteSpace(importResource);
var isNetCoreApp31 = Utils.TargetFramework == "netcoreapp3.1";
if (!hasImportResource && !isNetCoreApp31)
return;

var ciProps = ProjectRootElement.Open(Path.Combine(repo.Path, "ci.props"))!;

if (isNetCoreApp31)
{
// "ci.props" imports DotnetAffected.Tasks as an Sdk
// <Import Project="Sdk.props" Sdk="$(DotnetAffectedNugetDir)" />
//
// When we execute the build within the test we provide the property "DotnetAffectedNugetDir" with the lib's location
// For some reason it's not working in 3.1 so we override
foreach (var importElement in ciProps.Imports)
importElement.Sdk = Utils.DotnetAffectedNugetDir;
}

if (hasImportResource)
{
var fileName = $"./{Guid.NewGuid().ToString()}.props";
Expand Down
2 changes: 0 additions & 2 deletions test/DotnetAffected.Tasks.Tests/Utils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ public static class Utils

switch (majorVersion)
{
case 3:
return "netcoreapp3.1";
case >= 5:
return $"net{majorVersion}.0";
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

<ItemGroup>
<PackageReference Include="LibGit2Sharp"/>
<PackageReference Condition="'$(TargetFramework)' == 'netcoreapp3.1'" Include="Microsoft.Win32.Registry"/>
<PackageReference Include="Microsoft.Build.Locator"/>
</ItemGroup>

Expand Down