Always use latest targeting packs #36718
Conversation
- also, only update latest `@(KnownFrameworkReference)` items - previously the updates applied e.g. to all Microsoft.NETCore.App items nit: expand `@(KnownFrameworkReference)` comments
ghost
added
the
area-infrastructure
|
Issue noticed in Servicing Readiness Exercise
|
- `<Copy />` task will create directories - avoid leftover files from a previous build - avoid confusing SDK with an empty targeting pack directory - prevented ASP.NET targeting pack download when not building it (see #36718)
| <ItemGroup> | ||
| <!-- Use the same NETCore shared framework as repo built against except when building product code in servicing. --> | ||
| <KnownFrameworkReference | ||
| Update="@(KnownFrameworkReference->WithMetadataValue('Identity', 'Microsoft.NETCore.App')->WithMetadataValue('TargetFramework', '${DefaultNetCoreTargetFramework}'))" |
There was a problem hiding this comment.
@rainersigwald shouldn't this construct only update the item with the exact metadata described❔ I could swear I tested this out w/ earlier SDKs et cetera and saw it working correctly. Definitely updates all items named Microsoft.NETCore.App now.
My preference here would be to avoid repeating the '%(TargetFramework)' == '${DefaultNetCoreTargetFramework}' on all metadata updates.
There was a problem hiding this comment.
Yeah, I would have expected that to work. @benvillalobos can you take a look at this?
There was a problem hiding this comment.
Should I wait to merge this PR @benvillalobos @rainersigwald❔
There was a problem hiding this comment.
I also would have expected this to work. I can dig into this tomorrow and get back to you. I wouldn't consider it a blocker unless you're worried about perf / potentially updating it again
There was a problem hiding this comment.
- Please find attached a sample Directory.Build.targets file after we've used the Directory.Build.targets.in template. The final file contains no curly braces.
Directory.Build.targets.txt - The
@(KnownFrameworkReference)item group consists almost entirely of overlapping groups e.g. there are 4 TFM-specific entries for `Microsoft.NETCore.App:
- When using
->WithMetadataValue('TargetFramework', '${DefaultNetCoreTargetFramework}'))(actually->WithMetadataValue('TargetFramework', 'net6.0'))), the result was all fourMicrosoft.NETCore.Appitems were updated.
There was a problem hiding this comment.
@dougbu do you know what version of the sdk you saw this working prior?
I've filed an issue for it here: dotnet/msbuild#6880
There was a problem hiding this comment.
I switched to use the double WithMetadataValue(...) in Update way back in 5fd1db2, December 2020. My old friend git switch --detach 5fd1db26257 showed me we were using the 6.0.100-alpha.1.20523.3 back then.
There was a problem hiding this comment.
Oh my. I remember doing a lot of .binlog checking while working on that. But, I just changed the indentation of the relevant Directory.Build.targets.in bits. Looking back…
There was a problem hiding this comment.
Introduced in 219ecd6, October 2020. SDK at that time was also 6.0.100-alpha.1.20523.3 (surprisingly)
| <TargetingPackVersion Condition=" '$(IsServicingBuild)' != 'true' AND | ||
| '$(TargetLatestDotNetRuntime)' != 'false' ">${MicrosoftNETCoreAppRefVersion}</TargetingPackVersion> |
There was a problem hiding this comment.
I don't remember the history of this Condition but it seems to do more harm than good these days. Same for the similar Condition in the next @(KnownFrameworkReference) update.
- `<Copy />` task (well, most tasks though not `tar` commands) will create directories - avoid leftover files from a previous build - avoid confusing SDK with an empty targeting pack directory - prevented ASP.NET targeting pack download when not building it (see #36718)
- backport of #36719 Clear layout directories before copies (#36719) - `<Copy />` task (well, most tasks though not `tar` commands) will create directories - avoid leftover files from a previous build - avoid confusing SDK with an empty targeting pack directory - prevented ASP.NET targeting pack download when not building it (see #36718)
- backport of #36719 Clear layout directories before copies (#36719) - `<Copy />` task (well, most tasks though not `tar` commands) will create directories - avoid leftover files from a previous build - avoid confusing SDK with an empty targeting pack directory - prevented ASP.NET targeting pack download when not building it (see #36718)
- backport of #36719 Clear layout directories before copies (#36719) - `<Copy />` task (well, most tasks though not `tar` commands) will create directories - avoid leftover files from a previous build - avoid confusing SDK with an empty targeting pack directory - prevented ASP.NET targeting pack download when not building it (see #36718)
- backport of #36719 Clear layout directories before copies (#36719) - `<Copy />` task (well, most tasks though not `tar` commands) will create directories - avoid leftover files from a previous build - avoid confusing SDK with an empty targeting pack directory - prevented ASP.NET targeting pack download when not building it (see #36718)
- backport of #36719 Clear layout directories before copies (#36719) - `<Copy />` task (well, most tasks though not `tar` commands) will create directories - avoid leftover files from a previous build - avoid confusing SDK with an empty targeting pack directory - prevented ASP.NET targeting pack download when not building it (see #36718)
@(KnownFrameworkReference)itemsnit: expand
@(KnownFrameworkReference)comments