ProjectItem.SetItemMetadata changes ProjectItem.Include to be evaluated

[olgaark]

Adding metadata to an item which has macros in its Include changes the Include to be evaluated. This is causing VS bugs:
https://devdiv.visualstudio.com/DevDiv/_workitems?_a=edit&id=295640
https://devdiv.visualstudio.com/DevDiv/_workitems?_a=edit&id=376742

[olgaark]

And this is where the evaluated value is set

internal void SplitOwnItemElement()
{
ProjectItemElement oldXml = _xml;

        _xml = _xml.ContainingProject.CreateItemElement(ItemType, ((IItem)this).EvaluatedIncludeEscaped);

        oldXml.Parent.InsertBeforeChild(_xml, oldXml);

        Microsoft.Build.dll!Microsoft.Build.Construction.ProjectItemElement.Include.set(string value) Line 102    C#                Symbols loaded.

           Microsoft.Build.dll!Microsoft.Build.Evaluation.ProjectItem.SplitOwnItemElement() Line 734           C#                Symbols loaded.

Microsoft.Build.dll!Microsoft.Build.Evaluation.Project.SplitItemElementIfNecessary(Microsoft.Build.Construction.ProjectItemElement itemElement) Line 2338 C# Symbols loaded.
Microsoft.Build.dll!Microsoft.Build.Evaluation.ProjectItem.SetMetadataValue(string name, string unevaluatedValue) Line 528 C# Symbols loaded.

[cdmihai]

This appears to be more of a "feature" rather than a bug.

A ProjectItemElement (representing an item xml element) may produce multiple ProjectItem objects (an actual item). For example, the ProjectItemElement <A Include="1;2;3"/> produces three ProjectItem instances of item type A and values 1, 2, and 3. This means that one can add a ProjectItemElement via the Project.AddItem API which actually generates multiple ProjectItem objects. If any of those ProjectItems are mutated (e.g. adding new metadata), there is an ambiguity on whether to perform the mutation just to that ProjectItem, or to all the other ProjectItems originating from the same ProjectItemElement. The API chose to mutate just the current ProjectItem, without affecting the rest. To achieve this it will remove the original ProjectItemElement and generate new ProjectItemElements for each ProjectItem (the include element is taken from the evaluated value of the ProjectItem). So in our example the API will remove <A Include="1;2;3"/> and replace it with three elements <A Include="1"/><A Include="2"/><A Include="3">. The include values of the generated ProjectItemElements come from the ProjectItem values.

This exploding of the ProjectItemElement happens on the following item operations

• item gets removed, renamed, moved to new item type,
• metadata added, changed, removed

It is triggered when the unevaluated value of ProjectRootElement contains globs, semicolons, item references, or property references (properties can bring any of the other special characters).

To workaround this issue, one has to use the XML editing APIs (e.g. mutate Project*Element objects), or do more weird things like:

var items = p.AddItem("CICompile", "placeholder", new Dictionary<string, string> { { "metadata", "value" } });
items.First().Xml.Include = "$(MSBuildThisFileDirectory)Source.cpp";

[nerditation]

had looked into this problem myself. in my opinion, the split element logic seems to be alright.
for this specific issue, as cdmihai said, manually manipulate the XML element works, but I suspect there might be other cases where $(MSBuildThisFileDirectory) also got expanded unintentionally, a better fix might be change the logic in ItemElementRequiresSplitting to make $(MSBuildThisFileDirectory) a special case? if an element refers only $(MSBuildThisFileDirectory), it cannot possibly produce multiple ProjectItem, right?

[nerditation]

also, I'd like to suggest, on loading a shared items project, check and convert absolute path to be relative to $(MSBuildThisFileDirectory), which should repair the previously corrupted filters.

[cdmihai]

@nerditation

Special casing the reserved properties might improve the scenario a bit. Though it is hard to get right, as the reserved properties might get referenced from other properties ($Foo->$Bar->$MSBuildThisFileDirectory). A simplification would be to ignore the variable aliasing problem.

also, I'd like to suggest, on loading a shared items project, check and convert absolute path to be relative to $(MSBuildThisFileDirectory), which should repair the previously corrupted filters.
As far as I know, relative items are already made absolute against $(MSBuildThisFileDirectory).

[nerditation]

@cdmihai

As far as I know, relative items are already made absolute against $(MSBuildThisFileDirectory).

foo.vcxitems.filters I think not, at least not at the loading time. but I believe it's a problem in the VC project engine code, not in msbuild.

I can't find the source code for VCFilterProvider, but from some ILDASM-ed IL code, I could guess the Include attribute for the ClInclude element in the filter XML file (foo.vcxitems.filters) are compared verbatim (as keys) against the corresponding elements in the project XML file (foo.vcxitems). if the file has a expanded full path, the loaded DOM object would simply has the expanded value, no conversion at all. this is part of the reason for the reported bug[*] that unload and reload the project would discard all the filters information (unless the filters file is manually edited to workaround it).

[*] https://developercommunity.visualstudio.com/search.html?f=&type=question+OR+problem&c=&redirect=search%2Fsearch&sort=relevance&q=shared+items+filters

since the IL code is too verbose to understand, I could easily be wrong, but the loading process is roughly like so: XML files for project and filters get parsed, and the ClInclude elements are inserted to some private dictionaries. then the VCFilterProvider iterate through the items of the project file (not the filters file) and assign them to filters if they are found in the filters dictionary; if not found, then they are assigned to the root filter. I most guessed this process from the function VCFilterProvider.LoadFiltersAsync and VCFilterProvider.AssignItemToFilter.

[cdmihai]

My bad, all items are relative to $(MSBuildProjectFile), not $(MSBuildThisFileDirectory). If items coming from imports need to be relative to something else, then the full path must be explicit in the import.