Skip to content

Commit 452e8e8

Browse files
committed
fix(manifest): keep MSBuild error codes in .NET evaluation failures
MSBuild keeps an evaluation error's code out of the exception message, so evaluation and solution-parse failures reached the resolution report without it and were grouped as other failures. Prefix the code so they group as project configuration problems. Inner builds now come from TargetFrameworks, split as the cross-targeting targets do, rather than from restore's list, which only mattered while the tool built projects. Also tidies helpers and tests the evaluation-only change left with one caller.
1 parent 00d157b commit 452e8e8

7 files changed

Lines changed: 38 additions & 47 deletions

File tree

‎src/commands/manifest/scripts/assemble.test.mts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,7 @@ describe('dotnet records → per-framework sidecar', () => {
658658
'projectSrc\tLib.csproj\t/abs/Legacy.cs\tnetstandard2.0',
659659
'projectTgt\tLib.csproj\t/abs/bin/net8.0/Lib.dll\tnet8.0',
660660
'projectInfo\tLib.csproj\tnet8.0\tTargetFrameworkMoniker\t.NETCoreApp,Version=v8.0',
661-
'projectInfo\tLib.csproj\tnet8.0\tDefineConstant\tNET8_0_OR_GREATER',
661+
'projectInfo\tLib.csproj\tnet8.0\tDefineConstant\tDEBUG',
662662
'projectInfo\tLib.csproj\tnet8.0\tFrameworkReference\tMicrosoft.NETCore.App',
663663
'projectInfo\tLib.csproj\tnet8.0\tNullable\tenable',
664664
'projectUsing\tLib.csproj\tnet8.0\tSystem.Linq\t\t0',
@@ -696,7 +696,7 @@ describe('dotnet records → per-framework sidecar', () => {
696696
nullable: 'enable',
697697
assemblyName: undefined,
698698
frameworkReferences: ['Microsoft.NETCore.App'],
699-
defineConstants: ['NET8_0_OR_GREATER'],
699+
defineConstants: ['DEBUG'],
700700
usings: [
701701
{ namespace: 'System.Linq' },
702702
{ namespace: 'System.Math', static: true },

‎src/commands/manifest/scripts/dotnet-tool/FactsRunner.cs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ private bool IsExcludedProject(string path) {
155155
_solution = new SolutionContext(entry, _opts.GlobalProperties);
156156
return _solution.Members.Keys.OrderBy(p => p, StringComparer.Ordinal).ToList();
157157
} catch (Exception e) when (e is InvalidProjectFileException or IOException or JsonException) {
158-
_records.Failure(Rel(entry), $"could not read the solution: {Text.FirstLine(e.Message)}", "");
158+
_records.Failure(Rel(entry), $"could not read the solution: {ErrorText(e)}", "");
159159
return null;
160160
}
161161
}
@@ -211,13 +211,7 @@ private IEnumerable<string> ProjectReferences(Project project, bool innerBuilds)
211211
}
212212
}
213213
}
214-
return builds
215-
.SelectMany(b => b.GetItems("ProjectReference"))
216-
.Select(item => item.GetMetadataValue("FullPath"))
217-
.Where(full => !string.IsNullOrEmpty(full) && File.Exists(full))
218-
.Select(full => Path.GetFullPath(full))
219-
.Distinct(StringComparer.Ordinal)
220-
.ToList();
214+
return FullPaths(builds.SelectMany(b => b.GetItems("ProjectReference"))).Where(File.Exists).ToList();
221215
}
222216

223217
private void RunRestore() {
@@ -281,12 +275,18 @@ private Project LoadInnerBuild(Project project, string targetFramework) {
281275
return Load(collection, info.FullPath, info.GlobalProperties);
282276
} catch (InvalidProjectFileException e) {
283277
if (!info.Excluded) {
284-
_records.Unscannable("", $"could not evaluate the project: {Text.FirstLine(e.Message)}", info.Key);
278+
_records.Unscannable("", $"could not evaluate the project: {ErrorText(e)}", info.Key);
285279
}
286280
return null;
287281
}
288282
}
289283

284+
// MSBuild keeps an error's code out of its message.
285+
private static string ErrorText(Exception e) {
286+
var line = Text.FirstLine(e.Message);
287+
return e is InvalidProjectFileException { ErrorCode: { Length: > 0 } code } ? $"{code}: {line}" : line;
288+
}
289+
290290
// Restore errors not already reported from an assets file, then the projects
291291
// left without current assets and without errors of their own.
292292
private void ReportRestoreErrors() {

‎src/commands/manifest/scripts/dotnet-tool/Patterns.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ public bool Matches(IReadOnlyCollection<string> names) {
2222
return _includes.Count == 0 || _includes.Any(p => names.Any(p.IsMatch));
2323
}
2424

25+
public bool Matches(string name) => Matches(new[] { name });
26+
2527
private static List<Regex> Parse(string csv) {
2628
return csv
2729
.Split(',')

‎src/commands/manifest/scripts/dotnet-tool/Reader.cs‎

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,10 @@ string[] Unscannable(string? detail) {
162162
var assets = new Assets(PackageResolver(lockFile), libraries, projectKeys, projectNames);
163163

164164
var isTest = IsTestProject(project);
165-
var frameworks = new List<string>();
166165
foreach (var target in NuGetShim.Seq<LockFileTarget>(lockFile, "Targets")) {
167-
var alias = EmitTarget(info, spec, target, assets, isTest);
168-
if (alias != null && string.IsNullOrEmpty(target.RuntimeIdentifier)) frameworks.Add(alias);
166+
EmitTarget(info, spec, target, assets, isTest);
169167
}
170-
if (_opts.WithFiles) EmitPaths(info, project, frameworks);
168+
if (_opts.WithFiles) EmitPaths(info, project);
171169
return references;
172170
}
173171

@@ -227,9 +225,8 @@ private static bool IsTestProject(Project project) {
227225
|| project.GetPropertyValue("ProjectTypeGuids").Contains(TestProjectTypeGuid, StringComparison.OrdinalIgnoreCase);
228226
}
229227

230-
// One resolution root per (target framework [/ runtime], prod|dev). Returns
231-
// the target framework alias when the target was read.
232-
private string? EmitTarget(ProjectInfo info, PackageSpec spec, LockFileTarget target, Assets assets, bool isTest) {
228+
// One resolution root per (target framework [/ runtime], prod|dev).
229+
private void EmitTarget(ProjectInfo info, PackageSpec spec, LockFileTarget target, Assets assets, bool isTest) {
233230
var framework = NuGetShim.Seq<TargetFrameworkInformation>(spec, "TargetFrameworks")
234231
.FirstOrDefault(f => f.FrameworkName.Equals(target.TargetFramework));
235232
var alias = framework?.TargetAlias;
@@ -239,7 +236,7 @@ private static bool IsTestProject(Project project) {
239236
// A runtime-specific target also answers to its framework's name, so a
240237
// filter on `net8.0` covers `net8.0/win-x64`.
241238
var names = string.IsNullOrEmpty(rid) ? new[] { config } : new[] { config, alias };
242-
if (!_configs.Matches(names)) return null;
239+
if (!_configs.Matches(names)) return;
243240
if (_scanned.Add(config)) _records.Scanned(config);
244241

245242
var libraries = new Dictionary<string, LockFileTargetLibrary>(StringComparer.Ordinal);
@@ -331,7 +328,6 @@ void EmitRoot(string kind, bool prod, HashSet<string> ids, bool withFiles) {
331328
// project resolves is dev.
332329
EmitRoot("all", false, reached, _opts.WithFiles);
333330
if (!isTest) EmitRoot("prod", true, shipped, false);
334-
return alias;
335331
}
336332

337333
// Runtime assemblies (lib/, runtimes/<rid>/lib/) carry the method bodies an
@@ -454,7 +450,7 @@ private void ReadPackagesConfig(ProjectInfo info, Project project, string packag
454450
}
455451
if (packages.Count == 0) return;
456452
var config = LegacyFrameworkName(project);
457-
if (!_configs.Matches(new[] { config })) return;
453+
if (!_configs.Matches(config)) return;
458454
if (_scanned.Add(config)) _records.Scanned(config);
459455

460456
var isTest = IsTestProject(project);
@@ -528,13 +524,11 @@ private static string NormalizeSlashes(string path) {
528524

529525
// NuGet's short folder name of a build's framework, as restore derives it
530526
// from the two monikers; null when unrecognized.
531-
private static string? ShortFrameworkName(Project build) =>
532-
ShortFrameworkName(build.GetPropertyValue("TargetFrameworkMoniker"), build.GetPropertyValue("TargetPlatformMoniker"));
533-
534-
private static string? ShortFrameworkName(string moniker, string platformMoniker) {
527+
private static string? ShortFrameworkName(Project build) {
528+
var moniker = build.GetPropertyValue("TargetFrameworkMoniker");
535529
if (string.IsNullOrEmpty(moniker)) return null;
536530
try {
537-
return NuGetFramework.ParseComponents(moniker, platformMoniker).GetShortFolderName();
531+
return NuGetFramework.ParseComponents(moniker, build.GetPropertyValue("TargetPlatformMoniker")).GetShortFolderName();
538532
} catch (ArgumentException) {
539533
return null;
540534
}
@@ -547,41 +541,39 @@ private static string LegacyFrameworkName(Project project) {
547541
return string.IsNullOrEmpty(version) ? "unknown" : $"net{version.TrimStart('v').Replace(".", "")}";
548542
}
549543

550-
// The inner builds of a multi-targeting project, by target framework; none
551-
// for a project with a single framework.
544+
// Split as Microsoft.Common.CrossTargeting.targets does: trimmed, distinct
545+
// ignoring case. Empty for a single-framework project.
552546
private static List<string> Frameworks(Project project) {
553547
var multi = project.GetPropertyValue("TargetFrameworks");
554548
if (string.IsNullOrEmpty(project.GetPropertyValue("TargetFramework")) && !string.IsNullOrEmpty(multi)) {
555-
return multi.Split(';').Select(f => f.Trim()).Where(f => f.Length > 0).Distinct().ToList();
549+
return multi.Split(';').Select(f => f.Trim()).Where(f => f.Length > 0).Distinct(StringComparer.OrdinalIgnoreCase).ToList();
556550
}
557551
return new List<string>();
558552
}
559553

560554
// Per inner build of a multi-targeting project, whose outer evaluation has
561555
// no Compile items or TargetPath, or of the project's single framework.
562-
// `frameworks`: the inner builds to read, when restore names them.
563-
private void EmitPaths(ProjectInfo info, Project project, List<string>? frameworks = null) {
556+
private void EmitPaths(ProjectInfo info, Project project) {
564557
var multi = Frameworks(project);
565558
if (multi.Count == 0) {
566559
var single = project.GetPropertyValue("TargetFramework");
567560
var config = string.IsNullOrEmpty(single) ? LegacyFrameworkName(project) : single;
568-
if (_configs.Matches(new[] { config })) EmitBuild(info, project, config);
561+
if (_configs.Matches(config)) EmitBuild(info, project, config);
569562
return;
570563
}
571-
foreach (var tfm in (frameworks ?? multi).Where(f => _configs.Matches(new[] { f }))) {
564+
foreach (var tfm in multi.Where(_configs.Matches)) {
572565
Project build;
573566
try {
574567
build = LoadInnerBuild(project, tfm);
575568
} catch (InvalidProjectFileException e) {
576-
_records.Unscannable(tfm, $"could not evaluate the {tfm} build: {Text.FirstLine(e.Message)}", info.Key);
569+
_records.Unscannable(tfm, $"could not evaluate the {tfm} build: {ErrorText(e)}", info.Key);
577570
continue;
578571
}
579572
EmitBuild(info, build, tfm);
580573
}
581574
}
582575

583-
// As evaluation leaves it: no target runs, so neither the sources targets
584-
// generate nor the references and analyzers they resolve are known.
576+
// As evaluation leaves it: sources that targets generate are absent.
585577
private void EmitBuild(ProjectInfo info, Project build, string config) {
586578
var language = build.GetPropertyValue("Language");
587579
var isFSharp = string.Equals(language, "F#", StringComparison.OrdinalIgnoreCase);

‎src/commands/manifest/scripts/resolution-report-nuget.mts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ const NOT_FOUND_CODES = new Set(['NU1100', 'NU1101', 'NU1102', 'NU1103'])
1111
const SOURCE_CODES = new Set(['NU1301', 'NU1302'])
1212

1313
// The socket-facts-dotnet tool reports restore errors as `<code>: <message>`,
14-
// and some findings as `<finding>: <code>: <message>`; its findings without a
15-
// code count as other failures.
14+
// and evaluation errors as `<finding>: <code>: <message>`; its findings
15+
// without a code count as other failures.
1616
export function classifyNugetFailure(detail: string): FailureCategory {
1717
const code = /(?:^|: )(NU\d{4}|NETSDK\d{4}|MSB\d{4}):/.exec(detail || '')?.[1]
1818
if (!code) {

‎src/commands/manifest/scripts/resolution-report-nuget.test.mts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@ const DETAILS_BY_CATEGORY = {
1212
'NETSDK1147: To build this project, the following workloads must be installed: maui',
1313
"MSB4236: The SDK 'Microsoft.Build.Traversal/2.0.24' specified could not be found.",
1414
'could not read the solution: MSB5009: Error parsing the nested project section in solution file.',
15+
'could not evaluate the project: MSB4019: The imported project "/repo/x.props" was not found. /repo/App.csproj',
1516
],
1617
'not-found': [
1718
'NU1101: Unable to find package Foo. No packages exist with this id in source(s): nuget.org',
1819
'NU1102: Unable to find package Foo with version (>= 9.0.0)',
1920
],
2021
other: [
21-
'could not evaluate the project: The imported project "x.props" was not found.',
2222
'obj/project.assets.json belongs to B.csproj: the projects share an intermediate output directory, so restore output cannot be attributed',
2323
'runtime assemblies restore listed are missing from the package folders (lib/net8.0/Foo.dll); re-run `dotnet restore`',
2424
],

‎src/commands/manifest/scripts/test/dotnet-compat/assert-facts.py‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"""
88
import json
99
import os
10+
import re
1011
import sys
1112

1213
WORK, SDK, CPM_ENTRY = sys.argv[1], sys.argv[2], sys.argv[3]
@@ -129,7 +130,7 @@ def default_phase():
129130
check('Demo.Ext.DoesNotExist' in log and 'NU1101' in log, 'missing package reported')
130131
with open(os.path.join(WORK, 'broken.log')) as f:
131132
log = f.read()
132-
check('could not evaluate the project' in log, 'evaluation failure reported')
133+
check(re.search(r'could not evaluate the project: MSB\d{4}: ', log), 'evaluation failure reported with its code')
133134

134135

135136
def records(name):
@@ -144,10 +145,6 @@ def failures(rows):
144145
return [r for r in rows if r[0] == 'failure']
145146

146147

147-
def info_rows(rows, project, name):
148-
return [r for r in rows if r[0] == 'projectInfo' and r[1] == project and r[3] == name]
149-
150-
151148
def records_phase():
152149
basic = records('basic.tsv')
153150
check(not failures(basic), f'basic failures {failures(basic)}')
@@ -179,14 +176,14 @@ def records_phase():
179176
check({'src/App/App.csproj', 'src/Extra/Extra.csproj', 'src/Lib/Lib.csproj'} <= unknown, f'unrestored projects reported {sorted(unknown)}')
180177

181178
evaluation = records('evaluation.tsv')
182-
problems = [r for r in evaluation if r[0] in ('failure', 'unscannable')]
183-
check(not problems, f'evaluation problems {problems}')
179+
faults = [r for r in evaluation if r[0] in ('failure', 'unscannable')]
180+
check(not faults, f'evaluation faults {faults}')
184181
lib = [r[2] for r in evaluation if r[0] == 'projectSrc' and r[1] == 'Lib/Lib.csproj']
185182
check(len(lib) == 1 and lib[0].endswith('/Lib/Class.cs'), f'sources of a project failing any target run {lib}')
186183
check(any(r[0] == 'project' and r[1] == 'Build.csproj' for r in evaluation), 'compiler-less aggregator recorded')
187184

188185
langs = records('langs.tsv')
189-
vb = {r[4] for r in info_rows(langs, 'Vb.vbproj', 'DefineConstant')}
186+
vb = {r[4] for r in langs if r[0] == 'projectInfo' and r[1] == 'Vb.vbproj' and r[3] == 'DefineConstant'}
190187
check({'DEBUG=-1', 'TRACE=-1'} <= vb, f'VB compiler constants {vb}')
191188
fs = [r[2] for r in langs if r[0] == 'projectSrc' and r[1] == '../Fs/Fs.fsproj']
192189
check([os.path.basename(f) for f in fs] == ['First.fs', 'Library.fs'], f'F# sources in compile order {fs}')

0 commit comments

Comments
 (0)