-
Notifications
You must be signed in to change notification settings - Fork 510
Add diagnostic messages to the build targets #5171
Conversation
Two things: * Running ILC takes a while but we don't print any information about it. Print a message. * Invoking the publish from a clean command promt is something that will happen very often I assume. Make sure the failure message is more prescriptive. We'll want to make these messages localizable at some point...
<Exec Command="where /Q $(CppLinker)" IgnoreExitCode="true"> | ||
<Output TaskParameter="ExitCode" PropertyName="_WhereLinker"/> | ||
</Exec> | ||
<Error Condition="'$(_WhereLinker)' != '0'" Text="Platform linker not found. Make sure to publish from a Visual Studio Developer Command prompt with C++ tools installed." /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be x64 Native Tools Command Prompt for VS 2017
? It is what all our other docs say.
The requirement to run in VS command problem is not pretty. I was thinking that we may want to auto-detect the VS location if you do not have linker available on your path (call |
@@ -77,5 +77,10 @@ See the LICENSE file in the project root for more information. | |||
<LinkerArg Include="/OPT:REF" /> | |||
<LinkerArg Include="/OPT:ICF" /> | |||
</ItemGroup> | |||
|
|||
<Exec Command="where /Q $(CppLinker)" IgnoreExitCode="true"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add similar message for Unix as well?
(Another alternative is to get the linker and tools using https://www.nuget.org/packages/VisualCppTools.Community.VS2017Layout/ nuget package. This package is huge and does not seem to be used much, so it is probably not a good idea.) |
@dotnet-bot test this please |
Yeah, I looked at that package in the past. If the C++ team wanted it to be usable, they would probably need to split it up into something along the lines of a compiler package, a header package(?), and platform SDK packages (with the lib files) for each architecture. If it's a 200 MB dependency, it might be more palatable than the 650 MB that it is now. |
<Exec Command="command -v $(CppLinker)" IgnoreExitCode="true"> | ||
<Output TaskParameter="ExitCode" PropertyName="_WhereLinker"/> | ||
</Exec> | ||
<Error Condition="'$(_WhereLinker)' != '0'" Text="Platform linker ('$(CppLinker)') not found. On macOS, platform linker gets installed with Xcode. On Linux, it comes with clang." /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: We can customize the message per platform:
<Error Condition="'$(_WhereLinker)' != '0' and '$(TargetOS)' == 'OSX'" ...
<Error Condition="'$(_WhereLinker)' != '0' and '$(TargetOS)' != 'OSX'" ...
<Exec Command="command -v $(CppLinker)" IgnoreExitCode="true"> | ||
<Output TaskParameter="ExitCode" PropertyName="_WhereLinker"/> | ||
</Exec> | ||
<Error Condition="'$(_WhereLinker)' != '0'" Text="Platform linker ('$(CppLinker)') not found. On macOS, platform linker gets installed with Xcode. On Linux, it comes with clang." /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be better if the message tells you what to do to fix the problem, something like: Try installing clang-3.9 or the appropriate packages for your platform.
or Try installing XCode.
.
Two things:
We'll want to make these messages localizable at some point...