Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

Use MSBuild project extensions instead of importing the users project #243

Merged
merged 1 commit into from
Dec 8, 2016

Conversation

natemcmaster
Copy link
Contributor

Implicit imports prevents using on a project file that has the Sdk attribute. This change instead generates a file in the MSBuildProjectExtensionsPath to inject targets require to find the UserSecretsId property in a project. (This is the approached used by dotnet-watch and dotnet-ef).

Resolves #242

cc @mlorbetske @muratg
cc @bricelam can you review? This is similar to the approach you've implemented in dotnet-ef to extract project metadata.

@natemcmaster
Copy link
Contributor Author

cc @rainersigwald

Copy link

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

No strong objections for now, though the hardcoding of ./obj/ will have to be changed before RTM. And it seems like changing the extension of your wrapper project would be a smaller, easier change to make for this prerelease.

// into the target project
var projectExtensionsPath = Path.Combine(
Path.GetDirectoryName(projectFile),
"obj",

Choose a reason for hiding this comment

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

This embeds some assumptions about the path to IntermediateObjectDirectory that won't always hold--but are probably ok for most new (or converted) .NET Core projects.

Copy link

@bricelam bricelam left a comment

Choose a reason for hiding this comment

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

:shipit:

@natemcmaster
Copy link
Contributor Author

the hardcoding of ./obj/ will have to be changed before RTM.

Good to know. Where will MSBuildProjectExtensionsPath default to by RTM?

@rainersigwald
Copy link

@natemcmaster I expect that to stay the default (it's been like that for years), but the location of the obj directory is a user-specifiable option. Many teams prefer, for example, to have a single {repoRoot}\out directory that's easy to delete. Some folks even go so far as to put all build outputs on a different drive that they can format to get back to a clean state.

@bricelam
Copy link

bricelam commented Dec 7, 2016

We allow overriding it in dotnet-ef, but it does feel like we're abusing this feature a bit... Is there a way outside of MSBuild to get the value of the MSBuildProjectExtensionsPath property?

@natemcmaster
Copy link
Contributor Author

Ok. Do you have any ideas how a tool like this could evaluate the value of MSBuildProjectExtensionsPath?

@bricelam
Copy link

bricelam commented Dec 7, 2016

Maybe, in general, we just need a built-in target to get the value of properties. E.g. msbuild /t:GetPropertyValue /p:Property=UserSecretsId /verbosity:minimal

(This would satisfy EF's requirements too.)

@rainersigwald
Copy link

You could use the OM to extract the property, but then you could just get the property you wanted instead (which is what I want to talk to you about after resolving the immediate issue).

For this limited case, you could probably force-override MSBuildProjectExtensionsPath with a command-line property to a folder you control (and clean up after execution). That might hork the project build in other ways, but since you're only calling one target that you control it might be ok?

@natemcmaster
Copy link
Contributor Author

OM? Sorry, not familiar with that abbreviation.

Overriding the MSBuildProjectExtensionsPath prop would probably work for dotnet-user-secrets, but quickly falls over for anything that needs properties that depend on other project extensions such as those generated by NuGet restore.

Let's continue this conversation on #244.

@rainersigwald
Copy link

OM is "Object model"--I'm saying you could just evaluate the project programmatically and ask MSBuild what the property you want is.

@natemcmaster
Copy link
Contributor Author

Gotcha. Unfortunately, we went down that path and got burned too many times by MSBuild API. At the strong recommendation of David Kean and others, we have been pursuing project-evaluation-as-a-target instead.

Implicit imports prevents using <Import> on a project file that has the Sdk attribute. This change instead generates a file in the MSBuildProjectExtensionsPath to inject targets require to find the UserSecretsId property in a project.

Resolves #242
@natemcmaster natemcmaster force-pushed the namc/gen-targets-file branch from 3d8d2fc to d48f2ab Compare December 8, 2016 17:55
@natemcmaster natemcmaster changed the base branch from rel/1.0.0-msbuild2 to feature/msbuild December 8, 2016 17:55
@natemcmaster
Copy link
Contributor Author

Retargeted to feature/msbuild. This fix won't make it into RC.2, but will be in the next update.

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

Successfully merging this pull request may close these issues.

4 participants