Bug 17543 - GetReferenceAssemblyPaths should prefer RootPath over Mono.framework/External
Summary: GetReferenceAssemblyPaths should prefer RootPath over Mono.framework/External
Status: NEW
Alias: None
Product: Class Libraries
Classification: Mono
Component: Ms.Build (show other bugs)
Version: master
Hardware: PC Mac OS
: --- normal
Target Milestone: Untriaged
Assignee: Atsushi Eno
URL:
Depends on:
Blocks:
 
Reported: 2014-01-31 17:22 UTC by Jonathan Pryor
Modified: 2014-04-30 10:28 UTC (History)
3 users (show)

See Also:
Tags:
Is this bug a regression?: ---
Last known good build:


Attachments

Description Jonathan Pryor 2014-01-31 17:22:46 UTC
It is desirable to be able to use $(TargetFrameworkRootPath) to control which assemblies are used for assembly resolution purposes, e.g. Bug #10948.

Unfortunately on OS X, the /Library/Frameworks/Mono.framework/External directory takes precedence over the GetReferenceAssemblyPaths.RootPath property:

https://github.com/mono/mono/blob/97bd727b/mcs/class/Microsoft.Build.Tasks/Microsoft.Build.Tasks/GetReferenceAssemblyPaths.cs#L87
> 			string dirs = String.Join (PathSeparatorAsString, new string [] {
> 							Environment.GetEnvironmentVariable ("XBUILD_FRAMEWORK_FOLDERS_PATH") ?? String.Empty,
> 							MSBuildUtils.RunningOnMac ? MacOSXExternalXBuildDir : String.Empty,
> 							RootPath,
> 							DefaultFrameworksBasePath });

Consequently, setting $(TargetFrameworkRootPath) is effectively ignored.

Crazy repro:

 1. Grab a Xamarin.Android install
 2. Extract it:

	mkdir mono-android-VERSION
	cd mono-android-VERSION
	xar -xf ../mono-android-VERSION.pkg
	cd xamarin.android.pkg
	gunzip -c Payload | cpio -i

 3. Try to use it!

    R=path/to/mono-andorid-VERSION/xamarin.android.pkg/Library/Frameworks/Xamarin.Android.framework/Versions/Current
    # create a Xamarin.Android project, build it:
    xbuild /t:SignAndroidPackage *.csproj \
        /p:MSBuildExtensionsPath=$R/lib/xbuild \
        /p:MonoAndroidToolsDirectory=$R/lib/mandroid \
        /p:TargetFrameworkRootPath=$R/lib/xbuild-frameworks \
        /v:diag > build.txt

 4. Search b.txt for GetReferenceAssemblyPaths.

Expected results: it should use the specified $(TargetFrameworkRootPath).

Actual results: IFF you already have Xamarin.Android installed, it uses the currently installed version, not the specified version:

> 	Task "GetReferenceAssemblyPaths"
> 		Using task GetReferenceAssemblyPaths from Microsoft.Build.Tasks.GetReferenceAssemblyPaths, Microsoft.Build.Tasks.v4.0, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
> 		Looking for framework 'MonoAndroid,Version=v1.0' in root path '/Library/Frameworks/Mono.framework/External/xbuild-frameworks'
> 		Found framework definition list '/Library/Frameworks/Mono.framework/External/xbuild-frameworks/MonoAndroid/v1.0/RedistList/FrameworkList.xml' for framework 'MonoAndroid,Version=v1.0'

I would prefer RootPath to be searched BEFORE Mono.framework/External.

Unrelated: /p:MSBuildExtensionsPath doesn't actually do anything.
Comment 1 Jonathan Pryor 2014-01-31 17:46:55 UTC
This should probably be fixed by implementing ToolLocationHelper.GetPathToReferenceAssemblies():

http://msdn.microsoft.com/en-us/library/microsoft.build.utilities.toollocationhelper.getpathtoreferenceassemblies(v=vs.100).aspx

The GetReferenceAssemblyPaths task would then be updated to use ToolLocationHelper.GetPathToReferenceAssemblies().
Comment 2 Mikayla Hutchinson [MSFT] 2014-01-31 17:54:19 UTC
I check the MS targets and it seems that TargetFrameworkRootPath has no default value. It's passed to GetReferenceAssemblyPaths.RootPath, and if it's null/empty, then the default location is computed somewhere within GetReferenceAssemblyPaths - probably in ToolLocationHelper.GetPathToReferenceAssemblies. 

Need to test MS implementation to see whether this is all handled within the overload of ToolLocationHelper.GetPathToReferenceAssemblies that accepts a root directory argument.

It's also not clear to me from the MS docs whether we should use the default directories if TargetFrameworkRootPath is non-empty but does not contain the requested framework.
Comment 3 Atsushi Eno 2014-02-18 06:12:26 UTC
There is a list of explicitly declared to be "reseved" and "well-known" properties that MSBuild users CAN depend on. In other words, user CANNOT depend on any other properties. They are internal implementation details.
Comment 4 Atsushi Eno 2014-02-18 06:12:41 UTC
Forgot to add link http://msdn.microsoft.com/en-us/library/ms164309.aspx
Comment 5 Mikayla Hutchinson [MSFT] 2014-02-18 10:58:38 UTC
Not really relevant. Those are the properties built into the MSBuild engine that are valid always.

This property is part of the "API" of Microsoft.Common.targets.
Comment 6 Atsushi Eno 2014-02-18 11:39:31 UTC
It can be a valid assumption only if it is documented.
Comment 7 Jonathan Pryor 2014-02-18 12:19:57 UTC
@Atsushi: I'm not entirely sure what your point is. There's MSBuild itself, and then there are the .targets files which are written "atop" MSBuild, and then there's overall compatibility with .NET.

The properties that MSBuild reserves for itself has nothing to do with .targets files written to run on MSBuild (just as C compiler features have little to do with C library code and programs).

In this case, ToolLocationHelper.GetPathForReferenceAssemblies() IS documented (see Comment #1), as is the <GetReferenceAssemblyPaths/> task and the GetReferenceAssemblyPaths.RootPath property:

http://msdn.microsoft.com/en-us/library/microsoft.build.tasks.getreferenceassemblypaths(v=vs.121).aspx
http://msdn.microsoft.com/en-us/library/microsoft.build.tasks.getreferenceassemblypaths.rootpath(v=vs.121).aspx

Even if none of this were documented -- which isn't the case! -- we'd still need to at least consider implementing this support for compatibility with existing MSBuild projects which DO make use of all of this, like, oh, Xamarin.Android.Common.targets.
Comment 8 Mikayla Hutchinson [MSFT] 2014-02-18 14:23:06 UTC
The convention in MSBuild is that any property/target/item that begins with an underscore is "private" to the targets file and anything else is "public".
Comment 9 Mikayla Hutchinson [MSFT] 2014-02-18 14:26:26 UTC
MS does not document many of these things since they expect advanced consumers to read the common targets. You can find many comment in the common targets that are clearly designed to serve as documentation.

Basically, our implementation of the common targets should support any property/item/target in the MS common targets that does not start with an underscore.

Note You need to log in before you can comment on or make changes to this bug.