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

In MSBuild xplat pass through path to msbuild.exe host as a property #720

Closed
brthor opened this issue Jun 22, 2016 · 11 comments
Closed

In MSBuild xplat pass through path to msbuild.exe host as a property #720

brthor opened this issue Jun 22, 2016 · 11 comments
Assignees
Labels
needs-design Requires discussion with the dev team before attempting a fix. triaged

Comments

@brthor
Copy link

brthor commented Jun 22, 2016

This will solve the issue of needing to set an environment variable in the cli when invoking msbuild.exe so the same host can be used to execute csc.exe.

@cdmihai @rainersigwald

@cdmihai
Copy link
Contributor

cdmihai commented Jun 23, 2016

Can you please detail more?

Are you suggesting that each ToolTask has a way to access the host that MSBuild is run under? I guess there's a couple of ways to do it:

  • opt in: ToolTasks choose their own host, and they may choose to use MSBuild's host via some protected field
  • opt out: ToolTasks are automatically run under the same host as MSBuild's and they have the option to opt out.
  • all the above: Opt in by default. Opt out by cmd line flag: MSBuild --RunTasksInMSBuildHost

@brthor
Copy link
Author

brthor commented Jun 29, 2016

@cdmihai

The motivating scenario here is that the Csc tooltask needs to explicitly have the host overridden by the cli when executing under dotnet.exe to prevent a file not found for corerun.exe. In general this seems like it would be common scenario for tools.

With that in mind, I think that your option 2 seems most appropriate. ToolTasks would automatically be run under the host msbuild is being run under with the override gesture being reserved for cases where that isn't appropriate or some customization needs to happen.

@Sarabeth-Jaffe-Microsoft Sarabeth-Jaffe-Microsoft added this to the Visual Studio Preview 5 milestone Aug 10, 2016
@jeffkl
Copy link
Contributor

jeffkl commented Sep 15, 2016

At some point, csc.exe could become a stand-alone app and not need to run under dotnet.exe. We might need logic to detect if the app that's about to run is a .NET Core assembly before running it under dotnet.exe as well.

@eerhardt also mentioned that Roslyn might be recommending people wrap execution in a batch script like csc.cmd which explicitly runs the app under dotnet.exe.

All that said, I still think we should at least expose the host as a property to tasks. I'm not convinced yet that there is a good way to determine whether or not the app should run under the same host though...

@Sarabeth-Jaffe-Microsoft Sarabeth-Jaffe-Microsoft modified the milestones: Visual Studio Preview 5, Visual Studio 15 RC Oct 3, 2016
@eerhardt
Copy link
Member

eerhardt commented Oct 7, 2016

This is no longer necessary for the CLI in Visual Studio 15 RC. The Roslyn team recommends to ship "wrapper" csc.cmd and csc.sh files that invoke the managed Csc assembly using the dotnet host executable. The CLI does this, and sets the CscToolExe property to tell the Csc ToolTask to use this wrapper file.

From the perspective of the CLI, this issue can be closed. (I don't have permissions to close issues in this repo.)

@rainersigwald
Copy link
Member

Is that the general "how you should run a managed tool in a build" advice, @eerhardt? csc.exe is probably the most important tool but I expect there to be many others in this situation.

@eerhardt
Copy link
Member

eerhardt commented Oct 7, 2016

I'm not sure if that is the general advice. But this is the advice for invoking csc.exe I got from conversations with @agocke.

The generalized approach being employed by the Roslyn team here is for a ToolTask to allow the caller to pass in the path and name to the executable to invoke. This seems like a fine approach in general to me. The caller can and will have more context to where the tool is in their scenario. And if the ToolTask is unable to figure it out, at least it gives the caller an "escape hatch".

@borgdylan
Copy link

borgdylan commented Oct 8, 2016

I agree on using batch files. I happen to have a compiler to only runs on .NET/Mono which cannot be run on .NET Core (even if it supports it as a target) and the batch file method would allow me to use it with the new cross-plat MSBuild.

@rainersigwald
Copy link
Member

To be clear @borgdylan, it's a hard requirement that users be able to execute a script or native binary in a ToolTask--that works today and we have no intention of breaking it.

This issue is about making it easier to run .NET Core applications. Right now that requires either modifying the project file or using a wrapper script as the tool (and that wrapper script has no way to determine "the CLI host that is currently running MSBuild" which is probably the most common scenario).

@Sarabeth-Jaffe-Microsoft Sarabeth-Jaffe-Microsoft added the needs-design Requires discussion with the dev team before attempting a fix. label Oct 11, 2016
@jeffkl
Copy link
Contributor

jeffkl commented Oct 13, 2016

So should we consider this solved by way of using batch files? Or should we expose a property that indicates the path to the CLR host?

@jeffkl
Copy link
Contributor

jeffkl commented Oct 18, 2016

The community has spoken and we're going to leave it up to task authors for now. We'll gladly revisit this if enough people feel like they need support for it.

@VSadov
Copy link
Member

VSadov commented Apr 26, 2017

Re; aspnet/KestrelHttpServer#1748

Had to propagate dotnet location to run Csc.exe from a task by forcing it to be on the PATH. And had to do that in two environments - regular run and in a docker. Took some time to figure why one fails and not another (was using different PATH env).

It could be more convenient to use a property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-design Requires discussion with the dev team before attempting a fix. triaged
Projects
None yet
Development

No branches or pull requests

9 participants