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.

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

Design Time Builds failing if we have some derived properties in Implicit or leading imports #5996

Closed
Anipik opened this issue Mar 17, 2020 · 5 comments

Comments

@Anipik
Copy link

Anipik commented Mar 17, 2020

In case of multple TargetFrameworks, DesignTimeBuild uses msbuild setproperty function to set the TargetFramework property.

So this updates a property if it exists, otherwise it searches for the first unconditioned PropertyGroup in the project file to add the property to. This is always going to be after any implicit imports as well as explicitly leading imports. This appears to be the long-standing behavior of SetProperty. Contrast that to SetGlobalProperty, which will treat the property as global.

Hence any property derived from targetFramework in implicit imports and leading imports will not be evaluated correctly.

There is a detail repro here dotnet/runtime#33427 (comment)

cc @ericstj

similar issue OmniSharp/omnisharp-roslyn#1738

@davkean
Copy link
Member

davkean commented Mar 17, 2020

This is an MSBuild issue, please open it against http://github.com/microsoft/msbuild. I'd move the issue but I can't across orgs.

@ViktorHofer
Copy link
Member

@Anipik did you open an issue in msbuild as David suggested? Can't find one.

@Anipik
Copy link
Author

Anipik commented Apr 27, 2020

no i missed his comment, i am just doing it right now.

@Anipik
Copy link
Author

Anipik commented Apr 27, 2020

dotnet/msbuild#5319

@ericstj
Copy link
Member

ericstj commented Apr 28, 2020

@davkean are you sure this is an MSBuild issue? I believe MSBuild is behaving correctly. The problem is that the project system ends up setting a local property rather than a global property:

else if (!string.IsNullOrEmpty(currentTargetFramework))
{
var frameworkName = new FrameworkName(unevaluatedPropertyValue);
await defaultProperties.SetPropertyValueAsync(ConfigurationGeneral.TargetFrameworkProperty, _frameworkParser.GetShortFrameworkName(frameworkName));
}
else
{
// CPS implements IVsHierarchy.SetProperty for the TFM property to call through the multi-targeting service and change the TFM.
// This causes the project to be reloaded after changing the values.
// Since the property providers are called under a write-lock, trying to reload the project on the same context fails saying it can't load the project
// if a lock is held. We are not going to write to the file under this lock (we return null from this method) and so we fork execution here to schedule
// a lambda on the UI thread and we don't pass the lock information from this context to the new one.
_unconfiguredProjectVsServices.ThreadingService.RunAndForget(() =>
{
_unconfiguredProjectVsServices.VsHierarchy.SetProperty(HierarchyId.Root, (int)VsHierarchyPropID.TargetFrameworkMoniker, unevaluatedPropertyValue);
return System.Threading.Tasks.Task.CompletedTask;
}, options: ForkOptions.HideLocks | ForkOptions.StartOnMainThread,
unconfiguredProject: _unconfiguredProjectVsServices.Project);
}

This results in an inconsistency between what the SDK does in an actual build (global property) vs what the project system does in the design time build (local property).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants