Skip to content

Dev/erarndt/project root element #12318

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Dev/erarndt/project root element #12318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a66f14
Remove some fields from ProjectRootElement
Erarndt Aug 8, 2025
9de072e
Avoid storing dirtyReason
Erarndt Aug 8, 2025
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
64 changes: 12 additions & 52 deletions src/Build/Construction/ProjectRootElement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -115,11 +114,6 @@ public partial class ProjectRootElement : ProjectElementContainer
/// </summary>
private ElementLocation _projectFileLocation;

/// <summary>
/// The project file's full path, escaped.
/// </summary>
private string _escapedFullPath;

/// <summary>
/// The directory that the project is in.
/// Essential for evaluating relative paths.
Expand All @@ -142,16 +136,6 @@ public partial class ProjectRootElement : ProjectElementContainer
/// </summary>
private DateTime _lastWriteTimeWhenReadUtc;

/// <summary>
/// Reason it was last marked dirty; unlocalized, for debugging
/// </summary>
private string _dirtyReason = "first created project {0}";

/// <summary>
/// Parameter to be formatted into the dirty reason
/// </summary>
private string _dirtyParameter = String.Empty;

internal ProjectRootElementLink RootLink => (ProjectRootElementLink)Link;

/// <summary>
Expand Down Expand Up @@ -184,19 +168,11 @@ internal ProjectRootElement(XmlReader xmlReader, ProjectRootElementCacheBase pro
ProjectParser.Parse(document, this);
}

private readonly bool _isEphemeral = false;

private ProjectRootElement(ProjectRootElementCacheBase projectRootElementCache, NewProjectFileOptions projectFileOptions, bool isEphemeral)
: this(projectRootElementCache, projectFileOptions)
{
_isEphemeral = isEphemeral;
}

/// <summary>
/// Initialize an in-memory, empty ProjectRootElement instance that can be saved later.
/// Leaves the project dirty, indicating there are unsaved changes.
/// </summary>
private ProjectRootElement(ProjectRootElementCacheBase projectRootElementCache, NewProjectFileOptions projectFileOptions)
internal ProjectRootElement(ProjectRootElementCacheBase projectRootElementCache, NewProjectFileOptions projectFileOptions)
{
ErrorUtilities.VerifyThrowArgumentNull(projectRootElementCache);

Expand Down Expand Up @@ -337,8 +313,6 @@ public override string Condition
/// </summary>
public ICollection<ProjectImportElement> Imports => new ReadOnlyCollection<ProjectImportElement>(GetAllChildrenOfType<ProjectImportElement>());

internal bool IsEphemeral => _isEphemeral;

/// <summary>
/// Get a read-only collection of the child property groups, if any.
/// Does not include any that may not be at the root, i.e. inside Choose elements.
Expand Down Expand Up @@ -398,7 +372,7 @@ public string DirectoryPath
// Used during solution load to ensure solutions which were created from a file have a location.
}

public string EscapedFullPath => _escapedFullPath ?? (_escapedFullPath = ProjectCollection.Escape(FullPath));
public string EscapedFullPath => ProjectCollection.Escape(FullPath);

/// <summary>
/// Full path to the project file.
Expand Down Expand Up @@ -434,7 +408,6 @@ public string FullPath
}

_projectFileLocation = ElementLocation.Create(newFullPath);
_escapedFullPath = null;
_directory = Path.GetDirectoryName(newFullPath);

if (XmlDocument != null)
Expand Down Expand Up @@ -704,15 +677,6 @@ public override ElementLocation ConditionLocation
/// </summary>
internal bool IsMemberOfProjectCollection => _projectFileLocation != null;

/// <summary>
/// Indicates whether there are any targets in this project
/// that use the "Returns" attribute. If so, then this project file
/// is automatically assumed to be "Returns-enabled", and the default behavior
/// for targets without Returns attributes changes from using the Outputs to
/// returning nothing by default.
/// </summary>
internal bool ContainsTargetsWithReturnsAttribute { get; set; }

/// <summary>
/// Gets the ProjectExtensions child, if any, otherwise null.
/// </summary>
Expand All @@ -722,14 +686,6 @@ public override ElementLocation ConditionLocation
internal ProjectExtensionsElement ProjectExtensions
=> GetChildrenReversedOfType<ProjectExtensionsElement>().FirstOrDefault();

/// <summary>
/// Returns an unlocalized indication of how this file was last dirtied.
/// This is for debugging purposes only.
/// String formatting only occurs when retrieved.
/// </summary>
internal string LastDirtyReason
=> _dirtyReason == null ? null : String.Format(CultureInfo.InvariantCulture, _dirtyReason, _dirtyParameter);

/// <summary>
/// Initialize an in-memory empty ProjectRootElement instance that CANNOT be saved later.
/// The ProjectRootElement will not be marked dirty.
Expand All @@ -739,7 +695,7 @@ internal static ProjectRootElement CreateEphemeral(ProjectRootElementCacheBase p
{
ErrorUtilities.VerifyThrowArgumentNull(projectRootElementCache);

return new ProjectRootElement(projectRootElementCache, Project.DefaultNewProjectTemplateOptions, isEphemeral: true);
return new EphemeralProjectRootElement(projectRootElementCache, Project.DefaultNewProjectTemplateOptions);
}

/// <summary>
Expand Down Expand Up @@ -1847,7 +1803,7 @@ internal override void VerifyThrowInvalidOperationAcceptableLocation(ProjectElem
/// </remarks>
internal sealed override void MarkDirty(string reason, string param)
{
if (_isEphemeral)
if (this is EphemeralProjectRootElement)
{
return;
}
Expand All @@ -1860,9 +1816,6 @@ internal sealed override void MarkDirty(string reason, string param)

IncrementVersion();

_dirtyReason = reason;
_dirtyParameter = param;

_timeLastChangedUtc = DateTime.UtcNow;

var changedEventArgs = new ProjectXmlChangedEventArgs(this, reason, param);
Expand Down Expand Up @@ -2105,7 +2058,6 @@ private XmlDocumentWithLocation LoadDocument(string fullPath, bool preserveForma
}

_projectFileLocation = ElementLocation.Create(fullPath);
_escapedFullPath = null;
_directory = Path.GetDirectoryName(fullPath);

if (XmlDocument != null)
Expand Down Expand Up @@ -2172,4 +2124,12 @@ private void ThrowIfUnsavedChanges(bool throwIfUnsavedChanges)
}
}
}

internal class EphemeralProjectRootElement : ProjectRootElement
{
internal EphemeralProjectRootElement(ProjectRootElementCacheBase projectRootElementCache, NewProjectFileOptions projectFileOptions)
: base(projectRootElementCache, projectFileOptions)
{
}
}
}
10 changes: 0 additions & 10 deletions src/Build/Construction/ProjectTargetElement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,16 +273,6 @@ public string Returns
value,
true); /* only remove the element if the value is null -- setting to empty string is OK */

// if this target's Returns attribute is non-null, then there is at least one target in the
// parent project that has the returns attribute.
// NOTE: As things are currently, if a project is created that has targets with Returns, but then
// all of those targets are set to not have Returns anymore, the PRE will still claim that it
// contains targets with the Returns attribute. Do we care?
if (returnsAttribute != null)
{
((ProjectRootElement)Parent).ContainsTargetsWithReturnsAttribute = true;
}

MarkDirty("Set target Returns {0}", value);
}
}
Expand Down
15 changes: 5 additions & 10 deletions src/Build/Definition/Project.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1819,7 +1819,7 @@ internal void VerifyThrowInvalidOperationNotImported(ProjectRootElement otherXml
/// Internal project evaluation implementation.
/// </summary>
[DebuggerDisplay("#GlobalProperties={_data.GlobalPropertiesDictionary.Count} #Properties={_data.Properties.Count} #ItemTypes={_data.ItemTypes.Count} #ItemDefinitions={_data.ItemDefinitions.Count} #Items={_data.Items.Count} #Targets={_data.Targets.Count}")]
private class ProjectImpl : ProjectLink, IProjectLinkInternal
internal class ProjectImpl : ProjectLink, IProjectLinkInternal
{
/// <summary>
/// Backing data; stored in a nested class so it can be passed to the Evaluator to fill
Expand Down Expand Up @@ -2065,7 +2065,7 @@ public override bool IsDirty
{
if (Xml.Count > 0) // don't log empty projects, evaluation is not interesting
{
Trace.WriteLine(String.Format(CultureInfo.InvariantCulture, "MSBUILD: Is dirty because {0} [{1}] [PC Hash {2}]", Xml.LastDirtyReason, FullPath, ProjectCollection.GetHashCode()));
Trace.WriteLine(String.Format(CultureInfo.InvariantCulture, "MSBUILD: Is dirty because of version mismatch [{0}] [PC Hash {1}]", FullPath, ProjectCollection.GetHashCode()));
}
}

Expand All @@ -2088,12 +2088,7 @@ public override bool IsDirty
{
if (s_debugEvaluation)
{
string reason = import.ImportedProject.LastDirtyReason;

if (reason != null)
{
Trace.WriteLine(String.Format(CultureInfo.InvariantCulture, "MSBUILD: Is dirty because {0} [{1} - {2}] [PC Hash {3}]", reason, FullPath, import.ImportedProject.FullPath == FullPath ? String.Empty : import.ImportedProject.FullPath, ProjectCollection.GetHashCode()));
Copy link
Member

Choose a reason for hiding this comment

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

Hello, according to my search in code, there are several possible dirty reasons.
Are we reasonably sure that they are not in use please?

	MarkDirty("Set project FullPath to '{0}'", FullPath);	D:\bld\src\Build\Construction\ProjectRootElement.cs	456	17	Microsoft.Build	FullPath	ProjectRootElement	Read	
	MarkDirty("Project reloaded", null);	D:\bld\src\Build\Construction\ProjectRootElement.cs	1732	13	Microsoft.Build	ReloadFrom	ProjectRootElement	Read	
	MarkDirty("Set express as attribute: {0}", value.ToString());	D:\bld\src\Build\Construction\ProjectElement.cs	92	21	Microsoft.Build	ExpressedAsAttribute	ProjectElement	Read	
	MarkDirty("CopyFrom", null);	D:\bld\src\Build\Construction\ProjectElement.cs	415	13	Microsoft.Build	CopyFrom	ProjectElement	Read	
	MarkDirty("Replace element {0}", newElement.Name);	D:\bld\src\Build\Construction\ProjectElement.cs	467	13	Microsoft.Build	ReplaceElement	ProjectElement	Read

Or maybe (to be on a safer side) since there is a limited pool of the reasons, we could shift to an enum to keep the information while getting rid of the string.

}
Trace.WriteLine(String.Format(CultureInfo.InvariantCulture, "MSBUILD: Is dirty because version numbers are mismatched [{0} - {1}] [PC Hash {2}]", FullPath, import.ImportedProject.FullPath == FullPath ? String.Empty : import.ImportedProject.FullPath, ProjectCollection.GetHashCode()));
}

return true;
Expand Down Expand Up @@ -2263,7 +2258,7 @@ public override IList<ResolvedImport> Imports

foreach (ResolvedImport import in _data.ImportClosure)
{
if (import.ImportingElement != null && !import.ImportedProject.IsEphemeral) // Exclude outer project itself and SDK-resolver synthesized imports
if (import.ImportingElement != null && import.ImportedProject is not EphemeralProjectRootElement) // Exclude outer project itself and SDK-resolver synthesized imports
{
imports.Add(import);
}
Expand All @@ -2286,7 +2281,7 @@ public override IList<ResolvedImport> ImportsIncludingDuplicates

foreach (var import in _data.ImportClosureWithDuplicates)
{
if (import.ImportingElement != null && !import.ImportedProject.IsEphemeral) // Exclude outer project itself and SDK-resolver synthesized imports
if (import.ImportingElement != null && import.ImportedProject is not EphemeralProjectRootElement) // Exclude outer project itself and SDK-resolver synthesized imports
{
imports.Add(import);
}
Expand Down