Skip to content

Further aspnet website build improvements #13087

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.

Sign up for GitHub

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

Closed
johnazule wants to merge 4 commits into from
Closed

Further aspnet website build improvements #13087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d01ef7e
Add Target Framework version to website metaprojects
Jan 6, 2026
c07135a
Use template strings for clarity
Jan 6, 2026
3ceeef0
Increase AspNet Error verbosity and copy facades directory in when a …
Jan 19, 2026
3813114
Improve website metaproject building by reducing double writes, add e…
Jan 21, 2026
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
28 changes: 28 additions & 0 deletions src/Build/Construction/Solution/SolutionProjectGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ private static void AddTasksToCopyAllDependenciesIntoBinDir(
string copyLocalFilesItemName = referenceItemName + "_CopyLocalFiles";
string targetFrameworkDirectoriesName = GenerateSafePropertyName(project, "_TargetFrameworkDirectories");
string fullFrameworkRefAssyPathName = GenerateSafePropertyName(project, "_FullFrameworkReferenceAssemblyPaths");
string dependsOnNetStandardPropertyName = GenerateSafePropertyName(project, "_DependsOnNETStandard");
string destinationFolder = String.Format(CultureInfo.InvariantCulture, @"$({0})\Bin\", GenerateSafePropertyName(project, "AspNetPhysicalPath"));

// This is a bit of a hack. We're actually calling the "Copy" task on all of
Expand All @@ -443,6 +444,7 @@ private static void AddTasksToCopyAllDependenciesIntoBinDir(
// files need to be copy-localed.
ProjectTaskInstance rarTask = target.AddTask("ResolveAssemblyReference", String.Format(CultureInfo.InvariantCulture, "Exists('%({0}.Identity)')", referenceItemName), null);
rarTask.SetParameter("Assemblies", "@(" + referenceItemName + "->'%(FullPath)')");
rarTask.SetParameter("AppConfigFile", "$(WebConfigFileName)");
rarTask.SetParameter("TargetFrameworkDirectories", "$(" + targetFrameworkDirectoriesName + ")");
rarTask.SetParameter("FullFrameworkFolders", "$(" + fullFrameworkRefAssyPathName + ")");
rarTask.SetParameter("SearchPaths", "{RawFileName};{TargetFrameworkDirectory};{GAC}");
Expand All @@ -451,15 +453,29 @@ private static void AddTasksToCopyAllDependenciesIntoBinDir(
rarTask.SetParameter("FindSerializationAssemblies", "true");
rarTask.SetParameter("FindRelatedFiles", "true");
rarTask.SetParameter("TargetFrameworkMoniker", project.TargetFrameworkMoniker);
rarTask.SetParameter("TargetFrameworkVersion", $"v{new FrameworkName(project.TargetFrameworkMoniker).Version}");
rarTask.AddOutputItem("CopyLocalFiles", copyLocalFilesItemName, null);

// Capture whether RAR detected a dependency on netstandard
rarTask.AddOutputProperty("DependsOnNETStandard", dependsOnNetStandardPropertyName, null);

// Copy all the copy-local files (reported by RAR) to the web project's "bin"
// directory.
ProjectTaskInstance copyTask = target.AddTask("Copy", conditionDescribingValidConfigurations, null);
copyTask.SetParameter("SourceFiles", "@(" + copyLocalFilesItemName + ")");
copyTask.SetParameter("SkipUnchangedFiles", "true");
copyTask.SetParameter(
"DestinationFiles",
String.Format(CultureInfo.InvariantCulture, @"@({0}->'{1}%(DestinationSubDirectory)%(Filename)%(Extension)')", copyLocalFilesItemName, destinationFolder));

// If any references depend on netstandard, copy netstandard.dll from the Facades folder.
// .NET Framework 4.7.1+ has netstandard 2.0 support in the Facades folder.
string netstandardFacadePath = String.Format(CultureInfo.InvariantCulture, @"$({0})Facades\netstandard.dll", targetFrameworkDirectoriesName);
string copyFacadesCondition = String.Format(CultureInfo.InvariantCulture, "'$({0})' == 'True' AND Exists('{1}')", dependsOnNetStandardPropertyName, netstandardFacadePath);
ProjectTaskInstance copyFacadesTask = target.AddTask("Copy", copyFacadesCondition, null);
copyFacadesTask.SetParameter("SourceFiles", netstandardFacadePath);
copyFacadesTask.SetParameter("SkipUnchangedFiles", "true");
copyFacadesTask.SetParameter("DestinationFolder", destinationFolder);
}

/// <summary>
Expand Down Expand Up @@ -1242,6 +1258,16 @@ private ProjectInstance CreateMetaproject(ProjectInstance traversalProject, Proj
"AspNetCompiler.UnsupportedMSBuildVersion",
project.ProjectName);
#else

if (File.Exists(Path.Combine(project.AbsolutePath, "web.config")))

@JanProvaznik JanProvaznik Jan 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this kind of project is compileable only on windows, right? and there the filenames are case insensitive, so there is no need to have them separate I suppose

@johnazule johnazule Jan 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I'll cleanup

{
metaprojectInstance.SetProperty("WebConfigFileName", Path.Combine(project.AbsolutePath, "web.config"));
}
else if (File.Exists(Path.Combine(project.AbsolutePath, "Web.config")))
{
metaprojectInstance.SetProperty("WebConfigFileName", Path.Combine(project.AbsolutePath, "Web.config"));
}

AddMetaprojectTargetForWebProject(traversalProject, metaprojectInstance, project, null);
AddMetaprojectTargetForWebProject(traversalProject, metaprojectInstance, project, "Clean");
AddMetaprojectTargetForWebProject(traversalProject, metaprojectInstance, project, "Rebuild");
Expand Down Expand Up @@ -1537,12 +1563,14 @@ private void AddTaskForAspNetCompiler(
newTask.SetParameter("TargetPath", "$(" + GenerateSafePropertyName(project, "AspNetTargetPath") + ")");
newTask.SetParameter("Force", "$(" + GenerateSafePropertyName(project, "AspNetForce") + ")");
newTask.SetParameter("Updateable", "$(" + GenerateSafePropertyName(project, "AspNetUpdateable") + ")");
newTask.SetParameter("Clean", "true");

@JanProvaznik JanProvaznik Jan 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this does not seem right, perhaps it'd be OK to make a new AspNetClean property

@johnazule johnazule Jan 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No I'm at absolutely with you here, I'm just unsure if it should be set at the solution level or as an msbuild cli argument

newTask.SetParameter("Debug", "$(" + GenerateSafePropertyName(project, "AspNetDebug") + ")");
newTask.SetParameter("KeyFile", "$(" + GenerateSafePropertyName(project, "AspNetKeyFile") + ")");
newTask.SetParameter("KeyContainer", "$(" + GenerateSafePropertyName(project, "AspNetKeyContainer") + ")");
newTask.SetParameter("DelaySign", "$(" + GenerateSafePropertyName(project, "AspNetDelaySign") + ")");
newTask.SetParameter("AllowPartiallyTrustedCallers", "$(" + GenerateSafePropertyName(project, "AspNetAPTCA") + ")");
newTask.SetParameter("FixedNames", "$(" + GenerateSafePropertyName(project, "AspNetFixedNames") + ")");
newTask.SetParameter("Clean", "true");

@JanProvaznik JanProvaznik Jan 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

set twice?

@johnazule johnazule Jan 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So it's extra clean aha (I completely didn't realise, I'll commit a cleanup in a sec)


ValidateTargetFrameworkForWebProject(project);

Expand Down
6 changes: 4 additions & 2 deletions src/Build/Instance/ProjectInstance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2398,8 +2398,10 @@ public ProjectRootElement ToProjectRootElement()
if (!_globalProperties.Contains(property.Name) || !String.Equals(_globalProperties[property.Name].EvaluatedValue, property.EvaluatedValue, StringComparison.OrdinalIgnoreCase))
{
if ((!_environmentVariableProperties.Contains(property.Name) || !String.Equals(_environmentVariableProperties[property.Name].EvaluatedValue, property.EvaluatedValue, StringComparison.OrdinalIgnoreCase))
&& _sdkResolvedEnvironmentVariableProperties is not null
&& (!_sdkResolvedEnvironmentVariableProperties.Contains(property.Name) || !String.Equals(_sdkResolvedEnvironmentVariableProperties[property.Name].EvaluatedValue, property.EvaluatedValue, StringComparison.OrdinalIgnoreCase)))
&& (!Toolset.Properties.ContainsKey(property.Name) || !String.Equals(Toolset.Properties[property.Name].EvaluatedValue, property.EvaluatedValue, StringComparison.OrdinalIgnoreCase))
&& (_sdkResolvedEnvironmentVariableProperties is null
|| !_sdkResolvedEnvironmentVariableProperties.Contains(property.Name)
|| !String.Equals(_sdkResolvedEnvironmentVariableProperties[property.Name].EvaluatedValue, property.EvaluatedValue, StringComparison.OrdinalIgnoreCase)))
Comment on lines -2401 to +2404

@JanProvaznik JanProvaznik Jan 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I got confused what happens here, but it's only a logically equivalent edit, right? (de morgan's theorem). I'd avoid changing it

@johnazule johnazule Jan 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Basically when the sdk resolved environment variable is null, we don't log out any properties to metaproject files, which I feel is confusing. This basically tries to only isolate the properties that aren't explicitly set. The problem I'm having with my implementation is that metaprojects are a copy of the solution metaproject (traversal project) which sets some properties that I'm unsure if they need to be cloned

{
property.ToProjectPropertyElement(propertyGroupElement);
}
Expand Down
1 change: 1 addition & 0 deletions src/Tasks/AspNetCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ public override bool Execute()
/// <param name="commandLine">command line builder class to add arguments to</param>
protected internal override void AddCommandLineCommands(CommandLineBuilderExtension commandLine)
{
commandLine.AppendSwitch("-errorstack");

@JanProvaznik JanProvaznik Jan 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is reasonable to add for everyone. It would be confusing to users why the task is suddenly showing a lot more information. I could get behind putting it in the if (Debug){}

commandLine.AppendSwitchIfNotNull("-m ", MetabasePath);
commandLine.AppendSwitchIfNotNull("-v ", VirtualPath);
commandLine.AppendSwitchIfNotNull("-p ", PhysicalPath);
Expand Down
Loading