Add GetFileAbove intrinsic function

[danmoseley]

The CoreFX repo contains over 1,000 instances of this pattern:
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />

it seems to me it ought to be possible to write this more compactly like

<Import Project="$([MSBuild]::GetFileAbove(dir.props)" />

This assumes that an intrinsic function can get passed the current file's location as an implicit parameter. I haven't looked at the code but I assume it can.

Once this is added, it should be fairly easy to search and replace to the new syntax.

[danmoseley]

IElementLocation is available to Expander at the point it calls the intrinsic function
https://github.com/Microsoft/msbuild/blob/6cd2c4b3e94ef2fae38d6270a54d2948fb1d1ac2/src/XMakeBuildEngine/Evaluation/Expander.cs#L2864

[jeffkl]

I'm working on implementing this. The IElementLocation is just the location in XML where the property function call is. But my initial design on this relies on the current directory which is set to the project directory when building a project.

<Import Project="$([MSBuild]::GetPathOfFileAbove(dir.props)" />

The implementation calls Path.GetFullPath() on whatever is passed in. So you could still specify ..\dir.props or something and the function would make it an absolute path based on the current directory. You could also be more exact if necessary with:

<Import Project="$([MSBuild]::GetPathOfFileAbove('$(MSBuildProjectDirectory)\dir.props')" />

Once it has the absolute path, it would then call Path.GetDirectoryName() and pass that off to GetDirectoryNameOfFileAbove() which would do the search.

@danmosemsft Does that sound like it will meet your requirements?

@Microsoft/msbuild-maintainers thoughts?

[danmoseley]

Hi @jeffkl. In all the instances in CoreFX, $(MSBuildThisFileDirectory) is used. My assumption, although I don't know for sure, is that this is typical: the use case is to locate some file above the current file, whatever the importing project is. The situation being a hierarchy of .props files, each increasingly specialized, but extending any and all .props files above. Each of the .props files and the project itself has this tag, and the whole thing self-assembles. I can't think of a case where you would want a .props file to import something relative to the project file that's importing it.

So I don't think Path.GetFullPath() would work. That's why I was thinking of IElementLocation.File or similar.

[jeffkl]

Gotcha. So is it too much of a burden for them all to be:

<Import Project="$([MSBuild]::GetPathOfFileAbove('$(MSBuildThisFileDirectory)dir.props')" />

Another thought is to have an overload which takes a "starting directory" like GetDirectoryNameOfFileAbove(). The default would be $(MSBuildThisFileDirectory).

[danmoseley]

<Import Project="$([MSBuild]::GetPathOfFileAbove('$(MSBuildThisFileDirectory)dir.props')" /> would be an improvement, but I think the pattern is sufficiently common that it's worth making it as simple as <Import Project="$([MSBuild]::GetPathOfFileAbove(dir.props)" />.

This assumes it's reasonable to assume that GetPathOfFileAbove is relative to the current tag, but I think that's implied by the "Above" part implies that, plus my assertion is it's the common pattern.

This is not a blocking issue or anything - I simply noticed a common pattern that could potentially be simpler. I certainly don't have a strong opinion so your call ..

[jeffkl]

I really want this function as well. From what I can tell, the only real design decision here is if you pass a relative path (like just a file name), should it be made absolute to the current working directory or current file directory. Since this function is new, we can go with whatever we want.

I'll discuss it with the team and see what we come up with!

[rainersigwald]

My 2¢: error if someone puts in a relative path. It should be a filename only.

[cdmihai]

The evaluator has access to IEvaluatorData.Directory via the _data field: https://github.com/Microsoft/msbuild/blob/xplat/src/XMakeBuildEngine/Evaluation/Evaluator.cs#L105

It (hopefully) represents the directory of the project file msbuild was invoked on.

So the code that interprets GetFileAbove can use that as the starting lookup point.