Skip to content
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

Jump to bottom

Remove xmlns from props/targets and UTs #7169

Merged
merged 2 commits into from
Jan 21, 2022
Merged

Remove xmlns from props/targets and UTs #7169

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
0bef5b1
Remove xmlns from props/targets and projects
dfederm Jan 13, 2022
af7c186
Remove xmlns from many unit tests
dfederm Jan 13, 2022
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
6 changes: 3 additions & 3 deletions documentation/wiki/MSBuild-Tips-&-Tricks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ See the [MSBuild Command-Line Reference](https://docs.microsoft.com/visualstudio
If MSBuild.exe is passed properties on the command line, such as `/p:Platform=AnyCPU` then this value overrides whatever assignments you have to that property inside property groups. For instance, `<Platform>x86</Platform>` will be ignored. To make sure your local assignment to properties overrides whatever they pass on the command line, add the following at the top of your MSBuild project file:

```
<Project TreatAsLocalProperty="Platform" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Project TreatAsLocalProperty="Platform" DefaultTargets="Build">
```

This will make sure that your local assignments to the `Platform` property are respected. You can specify multiple properties in `TreatAsLocalProperty` separated by semicolon.
Expand All @@ -59,11 +59,11 @@ Use this command-line to approximate what the design-time build does:
See https://www.simple-talk.com/dotnet/.net-tools/extending-msbuild, "Extending all builds" section. Also read about [MSBuildUserExtensionsPath](http://referencesource.microsoft.com/#MSBuildFiles/C/ProgramFiles(x86)/MSBuild/14.0/Microsoft.Common.props,33), [CustomBeforeMicrosoftCommonProps](http://referencesource.microsoft.com/#MSBuildFiles/C/ProgramFiles(x86)/MSBuild/14.0/Microsoft.Common.props,68), [CustomBeforeMicrosoftCommonTargets](http://referencesource.microsoft.com/#MSBuildFiles/C/ProgramFiles(x86)/MSBuild/14.0/bin_/amd64/Microsoft.Common.targets,71), and CustomAfterMicrosoftCommonProps/CustomAfterMicrosoftCommonTargets.

Example:
Create this file (Custom.props) in `C:\Users\username\AppData\Local\Microsoft\MSBuild\14.0\Microsoft.Common.targets\ImportAfter`:
Create this file (Custom.props) in `C:\Users\username\AppData\Local\Microsoft\MSBuild\Current\Microsoft.Common.targets\ImportAfter`:

```
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Project>
<PropertyGroup>
<MyCustomProperty>Value!</MyCustomProperty>
</PropertyGroup>
Expand Down
4 changes: 2 additions & 2 deletions src/Build.OM.UnitTests/Construction/ElementLocationPublic_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Microsoft.Build.UnitTests.Construction
public class ElementLocationPublic_Tests
{
/// <summary>
/// Check that we can get the file name off an element and attribute, even if
/// Check that we can get the file name off an element and attribute, even if
/// it wouldn't normally have got one because the project wasn't
/// loaded from disk, or has been edited since.
/// This is really a test of our XmlDocumentWithLocation.
Expand Down Expand Up @@ -84,7 +84,7 @@ public void XmlLocationsAreCached()
public void LocationStringsMedley()
{
string content = @"
<Project ToolsVersion=`msbuilddefaulttoolsversion` xmlns=`http://schemas.microsoft.com/developer/msbuild/2003`>
<Project ToolsVersion=`msbuilddefaulttoolsversion`>
Copy link
Member

Choose a reason for hiding this comment

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

You can also remove the ToolsVersion, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, although I figured this PR was large enough as it was, so probably will follow up with another PR.

<UsingTask TaskName='t' AssemblyName='a' Condition='true'/>
<UsingTask TaskName='t' AssemblyFile='a' Condition='true'/>
<ItemDefinitionGroup Condition='true' Label='l'>
Expand Down
28 changes: 14 additions & 14 deletions src/Build.OM.UnitTests/Construction/ProjectChooseElement_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public void ReadInvalidAttribute()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose X='Y'/>
</Project>
";
Expand All @@ -50,7 +50,7 @@ public void ReadInvalidConditionAttribute()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose Condition='true'/>
</Project>
";
Expand All @@ -68,7 +68,7 @@ public void ReadInvalidChild()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose>
<X/>
</Choose>
Expand All @@ -88,7 +88,7 @@ public void ReadInvalidWhen()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose>
<When>
<PropertyGroup><x/></PropertyGroup>
Expand All @@ -113,7 +113,7 @@ public void ReadInvalidOnlyOtherwise()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose>
<Otherwise/>
</Choose>
Expand All @@ -133,7 +133,7 @@ public void ReadInvalidTwoOtherwise()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose>
<Otherwise/>
<Otherwise/>
Expand All @@ -154,7 +154,7 @@ public void ReadInvalidOtherwiseBeforeWhen()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose>
<Otherwise/>
<When Condition='c'/>
Expand All @@ -178,7 +178,7 @@ public void ReadInvalidEmptyChoose()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose/>
</Project>
";
Expand All @@ -197,7 +197,7 @@ public void ReadInvalidEmptyChoose()
public void ReadChooseOnlyWhen()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose>
<When Condition='c'/>
</Choose>
Expand All @@ -218,7 +218,7 @@ public void ReadChooseOnlyWhen()
public void ReadChooseBothWhenOtherwise()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Choose>
<When Condition='c1'/>
<When Condition='c2'/>
Expand Down Expand Up @@ -254,7 +254,7 @@ public void ExcessivelyNestedChoose()
builder2.Append("</When></Choose>");
}

string content = "<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>";
string content = "<Project>";
content += builder1.ToString();
content += builder2.ToString();
content += @"</Project>";
Expand All @@ -270,13 +270,13 @@ public void ExcessivelyNestedChoose()
public void SettingWhenConditionDirties()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
Copy link
Member

Choose a reason for hiding this comment

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

💡 Make sure to verify there is still adequate coverage for cases where xmlns is included, since both approaches are supported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, I left it in a bunch of UTs, mostly ones which dealt with evaluation. It probably could be cleaned from even more UTs (many use xmlns="msbuildnamespace" instead of xmlns="http://schemas.microsoft.com/developer/msbuild/2003">), but whatever this change is large enough as-is.

Comparing repo-wide searches of "xmlns=" (which admittedly covers other namespaces eg in loc xml files):
Before: 2641 results in 377 files
After: 1693 results in 277 files

Copy link
Member

Choose a reason for hiding this comment

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

This also had me worried but by inspection it looks ok (this concern is why I split out the "remove from unit tests" part of the change).

<Choose>
<When Condition='true'>
<PropertyGroup>
<p>v1</p>
</PropertyGroup>
</When>
</PropertyGroup>
</When>
</Choose>
</Project>
";
Expand Down
24 changes: 12 additions & 12 deletions src/Build.OM.UnitTests/Construction/ProjectExtensionsElement_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class ProjectExtensionsElement_Tests
public void Read()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
<Project>
<ProjectExtensions>
<a/>
</ProjectExtensions>
Expand All @@ -36,7 +36,7 @@ public void Read()
ProjectRootElement project = ProjectRootElement.Create(XmlReader.Create(new StringReader(content)));
ProjectExtensionsElement extensions = (ProjectExtensionsElement)Helpers.GetFirst(project.Children);

Assert.Equal(@"<a xmlns=""http://schemas.microsoft.com/developer/msbuild/2003"" />", extensions.Content);
Assert.Equal(@"<a />", extensions.Content);
}

/// <summary>
Expand All @@ -48,7 +48,7 @@ public void ReadInvalidCondition()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
<Project>
<ProjectExtensions Condition='c'/>
</Project>
";
Expand All @@ -66,7 +66,7 @@ public void ReadInvalidDuplicate()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
<Project>
<ProjectExtensions/>
<Target Name='t'/>
<ProjectExtensions />
Expand All @@ -88,7 +88,7 @@ public void SetValid()

extensions.Content = "a<b/>c";

Assert.Equal(@"a<b xmlns=""http://schemas.microsoft.com/developer/msbuild/2003"" />c", extensions.Content);
Assert.Equal(@"a<b />c", extensions.Content);
Assert.True(extensions.ContainingProject.HasUnsavedChanges);
}

Expand All @@ -107,13 +107,13 @@ public void SetInvalidNull()
);
}
/// <summary>
/// Delete by ID
/// Delete by ID
/// </summary>
[Fact]
public void DeleteById()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
<Project>
<ProjectExtensions>
<a>x</a>
<b>y</b>
Expand All @@ -132,13 +132,13 @@ public void DeleteById()
}

/// <summary>
/// Get by ID
/// Get by ID
/// </summary>
[Fact]
public void GetById()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
<Project>
<ProjectExtensions>
<a>x</a>
<b>y</b>
Expand All @@ -163,7 +163,7 @@ public void GetById()
public void SetById()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
<Project>
<ProjectExtensions>
<a>x</a>
<b>y</b>
Expand All @@ -185,7 +185,7 @@ public void SetById()
public void SetByIdWhereItAlreadyExists()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
<Project>
<ProjectExtensions>
<a>x</a>
<b>y</b>
Expand All @@ -206,7 +206,7 @@ public void SetByIdWhereItAlreadyExists()
private static ProjectExtensionsElement GetEmptyProjectExtensions()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
<Project>
<ProjectExtensions/>
</Project>
";
Expand Down
20 changes: 10 additions & 10 deletions src/Build.OM.UnitTests/Construction/ProjectImportElement_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void ReadInvalidMissingProject()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Import/>
</Project>
";
Expand All @@ -60,7 +60,7 @@ public void ReadInvalidEmptyProject()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Import Project=''/>
</Project>
";
Expand All @@ -78,7 +78,7 @@ public void ReadInvalidAttribute()
Assert.Throws<InvalidProjectFileException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Import Project='p' X='Y'/>
</Project>
";
Expand All @@ -94,7 +94,7 @@ public void ReadInvalidAttribute()
public void ReadBasic()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Import Project='i1.proj' />
<Import Project='i2.proj' Condition='c'/>
</Project>
Expand All @@ -117,7 +117,7 @@ public void ReadBasic()
public void SetProjectValid()
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Import Project='i1.proj' />
</Project>
";
Expand All @@ -139,7 +139,7 @@ public void SetProjectInvalidEmpty()
Assert.Throws<ArgumentException>(() =>
{
string content = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Import Project='i1.proj' />
</Project>
";
Expand Down Expand Up @@ -175,7 +175,7 @@ public void SettingProjectDirties()

string content = String.Format
(
@"<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
@"<Project>
<Import Project='{0}'/>
</Project>",
file1
Expand Down Expand Up @@ -215,7 +215,7 @@ public void SettingConditionDirties()

string content = String.Format
(
@"<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
@"<Project>
<Import Project='{0}'/>
</Project>",
file
Expand Down Expand Up @@ -250,14 +250,14 @@ public void ImportWithRelativePath()
string projectfileContent = String.Format
(
@"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
<Import Project='{0}'/>
</Project>
",
testTempPath + "\\..\\x.targets"
);
string targetsfileContent = @"
<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003' >
<Project>
</Project>
";
try
Expand Down
Loading