Skip to content

Conversation

@brettfo
Copy link
Member

@brettfo brettfo commented Mar 7, 2018

Different build targets copy their outputs at different times during the build which means $(OutputPath) will only really be honored if it's set in the top-level FSharp.Directory.Build.props, but we need to consume the $(TargetFramework) variable to properly set this, and $(TargetFramework) is set in the individual project files, which means we can only have a meaningful value of $(OutputPath) in FSharp.Directory.Build.targets, but that doesn't work for certain VS projects (notably those consuming the VsSDK) because they've already captured the value of $(OutputPath) before we have a chance to properly set it.

The fix is to manually copy the build artifacts after the AfterBuild target has executed.

This is a hack and is temporary until I've upgraded the rest of our project files to the SDK, at which point it's trivial to move our output directories away from $(Configuration)\net40\bin and to a cleaner, and more importantly, supported by the dotnet CI system value of artifacts\$(Configuration)\$(AssemblyName)\$(TargetFramework)\bin.

@realvictorprm
Copy link
Contributor

This fixes #4424?

@brettfo
Copy link
Member Author

brettfo commented Mar 7, 2018

Possibly, I'm checking this locally now.

This does, however, fix a problem where $(OutputPath) was honored by machines with Dev15.5, but not always with Dev15.6.

@brettfo
Copy link
Member Author

brettfo commented Mar 7, 2018

@realvictorprm This isn't the fix for #4424, but I need to merge this anyways to fix other issues, but I have ideas on why F5 from VS isn't working and I'm back on that problem now.

@brettfo brettfo merged commit 2c9030d into dotnet:master Mar 7, 2018
@brettfo brettfo deleted the outputpath branch March 7, 2018 22:04
@realvictorprm
Copy link
Contributor

realvictorprm commented Mar 7, 2018 via email

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

Successfully merging this pull request may close these issues.

2 participants