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

nologo should be the default #6614

Open
terrajobst opened this issue Jun 22, 2021 · 14 comments
Open

nologo should be the default #6614

terrajobst opened this issue Jun 22, 2021 · 14 comments

Comments

@terrajobst
Copy link
Member

Issue Description

Our goal is to make the command line experience great for developers. This includes dotnet and msbuild. People often invoke build and want a clean output.

Related: PowerShell/PowerShell#15644

Steps to Reproduce

Run msbuild /v:m.

Expected Behavior

❯ msbuild /v:m
ConsoleApp1 -> T:\ConsoleApp1\ConsoleApp1\bin\Debug\net5.0\ConsoleApp1.dll

Actual Behavior

❯ msbuild /v:m
Microsoft (R) Build Engine version 16.11.0-preview-21302-05+5e37cc992 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

  ConsoleApp1 -> T:\ConsoleApp1\ConsoleApp1\bin\Debug\net5.0\ConsoleApp1.dll
@terrajobst terrajobst added bug needs-triage Have yet to determine what bucket this goes in. labels Jun 22, 2021
@Nirmal4G
Copy link
Contributor

This is especially true in scripts where we want to get output from MSBuild target output into the build process.

The nologo doesn't suppress the response file header and there is no noRspLogo option to suppress it manually. Since, MSBuild also adds an extra indentation to the output (because of the headers), we have to remove the headers and the extra indent everytime from the output. This affects build perf as well.

So, can we also remove response file header and the 1° indentation applied to the MSBuild output (as indicated in the example above).

@dotMorten
Copy link

Interesting proposal but I'm not a fan. I've discovered numerous build issues on the build server from realizing which version was installed on it, merely by looking at the output. This is especially true for cloud builds where we don't control the server upgrades

@benvillalobos benvillalobos added Partner request Feature Request and removed needs-triage Have yet to determine what bucket this goes in. bug labels Jun 23, 2021
@KathleenDollard
Copy link
Contributor

It's too late in the cycle for us to adopt this without time for user feedback. It is not a universally desired change and it is potentially breaking.

We will consider this for a future SDK version, so are not closing it. Further comments for future implementation are welcome.

I think this is part of a broader problem that user's can't customize their environment with a set of configurations, or a set of options that are applied to all relevant commands, or a response-like file that is always applied, or similar.

@KathleenDollard
Copy link
Contributor

@Nirmal4G feel free to open a separate issue if you would think the response file logo should be handled as a separate issue. Sounds like that could be separately solved.

@terrajobst
Copy link
Member Author

terrajobst commented Jun 23, 2021

I think this is part of a broader problem that user's can't customize their environment with a set of configurations

I don't think it is. To me, it's part of the default experience that should feel polished and blend with user's expectation. The default output of MSBuild on the CLI is close to being unusable. Yes, it's colored, but it's a lot of noise and if you have enough projects scrolling won't help you because the output exceeds the buffer size of the terminal. And unless you're an expert the vast majority of the output is meaningless. And if you're an experienced user you're unlikely to benefit either because a .binlog or a preprocessed project that I can open in an editor is more helpful anyways.

If anything, this is part of the broader problem of how we can make the CLI nicer to use by default. But I dislike gating small issues like this behind these cloudy initiatives because in my experience they are rarely funded as a big item and thus end up blocking small incremental changes that in aggregate could make a big difference.

I think it's OK to say that this change is too late for .NET 6, but personally I don't believe issues like these need a ton of customer validation. It's an aesthetic issue. We should have an opinion on this and we should be the ones deciding the overall look & feel of our CLI experiences.

With respect to @dotMorten's argument that this taking information away. I have to admit that I'm not buying this as a counter argument. Any reasonable CI should dump version information of the .NET SDK being used anyway -- we don't need to see this output every time I'm invoking it from the terminal. Also, in my experience the logo itself wasn't sufficient information to trouble shoot version issues -- dotnet --info on the other hand has all the information necessary and is the more appropriate avenue for this.

@rainersigwald
Copy link
Contributor

It's an aesthetic issue.

I would be shocked if making this change did not break some company build scripts. Our console output is, sadly, part of the ill-defined "public interface" of MSBuild.

Any reasonable CI should dump version information of the .NET SDK being used anyway

Can you give an example of a CI system that does this?

Also, in my experience the logo itself wasn't sufficient information to trouble shoot version issues -- dotnet --info on the other hand has all the information necessary and is the more appropriate avenue for this.

This is reasonable, though please remember our non-.NET SDK scenarios.

@terrajobst
Copy link
Member Author

It's an aesthetic issue.

I would be shocked if making this change did not break some company build scripts. Our console output is, sadly, part of the ill-defined "public interface" of MSBuild.

That's fair. My comment was more talking about the motivation of making this change in order to assess what sort of customer validation is needed.

Any reasonable CI should dump version information of the .NET SDK being used anyway

Can you give an example of a CI system that does this?

The default template for GitHub Actions does this:

Run actions/setup-dotnet@v1
with:
dotnet-version: 5.0.x
/home/runner/work/_actions/actions/setup-dotnet/v1/externals/install-dotnet.sh --version 5.0.301

I'm targeting .NET 6 so this fails with this:

Error: /home/runner/.dotnet/sdk/5.0.301/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.TargetFrameworkInference.targets(141,5): error NETSDK1045: The current .NET SDK does not support targeting .NET 6.0. Either target .NET 5.0 or lower, or use a version of the .NET SDK that supports .NET 6.0.

The information returned here is sufficient to see what the problem is (using .NET SDK 5.0.301 trying to build a .NET 6 app).

@omajid
Copy link
Member

omajid commented Sep 9, 2021

Maybe an incremental step to solving this is removing this line?

Copyright (C) Microsoft Corporation. All rights reserved.

IMO, that gets rid of 50% of the noise in the output. I can only think of one other program that prints copyright on default invocation, and that's gdb. No other program I use does that.

Also, the copyright doesn't even match the LICENSE file in this repo, which says:

Copyright (c) .NET Foundation and contributors

@danmoseley
Copy link
Member

danmoseley commented Apr 27, 2022

The reason for emitting these lines originally was to do the same thing that Csc did back then. Which was not a good reason.

@ViktorHofer
Copy link
Member

While the UX was improved since the issue was filed, msbuild still emits its informational message:

> dotnet msbuild /v:m
MSBuild version 17.5.0-preview-23054-02+762ae6c6b for .NET
  msbuildverbtest -> C:\temp\msbuildverbtest\bin\Debug\net8.0\msbuildverbtest.dll

@danmoseley
Copy link
Member

+1 for suppressing the banner. It was done that way to copy tools like csc.exe, but that reason isn't a useful one. It's not "necessary"

@terrajobst
Copy link
Member Author

+1 for suppressing the banner. It was done that way to copy tools like csc.exe, but that reason isn't a useful one. It's not "necessary"

Does this mean you think that this output

> dotnet msbuild /v:m
MSBuild version 17.5.0-preview-23054-02+762ae6c6b for .NET
  msbuildverbtest -> C:\temp\msbuildverbtest\bin\Debug\net8.0\msbuildverbtest.dll

should just be

> dotnet msbuild /v:m
  msbuildverbtest -> C:\temp\msbuildverbtest\bin\Debug\net8.0\msbuildverbtest.dll

@danmoseley
Copy link
Member

Yes I do. Essentially something like -- /v:quiet and /v:minimal (and probably /v:normal) imply /nologo.
Or just make /nologo the default, and add /nologo- or /logo if you want it.

rainersigwald added a commit to rainersigwald/msbuild that referenced this issue Apr 13, 2023
Address dotnet#6614 by logging the logo as a deferred normal-priority build message rather than emitting it to stdout. This allows logger verbosity to control whether it actually makes it to the console, but puts it in text logs.
@rainersigwald
Copy link
Contributor

One very easy way to get a similar effect would be to log it as a deferred message (473937a).

That would produce:

image

text
\msbuild\.dotnet\dotnet build
  Determining projects to restore...
  All projects are up-to-date for restore.
  templateLibrary -> S:\play\templateLibrary\bin\Debug\netstandard2.0\templateLibrary.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.01

image

text
\msbuild\.dotnet\dotnet build -v:n
Build started 4/13/2023 10:49:00 AM.
MSBuild version 17.7.0-dev-23213-01+cb5e76064 for .NET
     1>Project "S:\play\templateLibrary\templateLibrary.csproj" on node 1 (Restore target(s)).
     1>_GetAllRestoreProjectPathItems:
         Determining projects to restore...
       Restore:
         X.509 certificate chain validation will use the default trust store selected by .NET.
         X.509 certificate chain validation will use the default trust store selected by .NET.
         Assets file has not changed. Skipping assets file writing. Path: S:\play\templateLibrary\obj\project.assets.json
         Restored S:\play\templateLibrary\templateLibrary.csproj (in 24 ms).

         NuGet Config files used:
             C:\Users\raines\AppData\Roaming\NuGet\NuGet.Config
             C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.FallbackLocation.config
             C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.Offline.config

         Feeds used:
             https://api.nuget.org/v3/index.json
             C:\Program Files (x86)\Microsoft SDKs\NuGetPackages\
         All projects are up-to-date for restore.
     1>Done Building Project "S:\play\templateLibrary\templateLibrary.csproj" (Restore target(s)).
   1:7>Project "S:\play\templateLibrary\templateLibrary.csproj" on node 1 (default targets).
     1>GenerateTargetFrameworkMonikerAttribute:
       Skipping target "GenerateTargetFrameworkMonikerAttribute" because all output files are up-to-date with respect to the input files.
       CoreGenerateAssemblyInfo:
       Skipping target "CoreGenerateAssemblyInfo" because all output files are up-to-date with respect to the input files.
       CoreCompile:
       Skipping target "CoreCompile" because all output files are up-to-date with respect to the input files.
       GenerateBuildDependencyFile:
       Skipping target "GenerateBuildDependencyFile" because all output files are up-to-date with respect to the input files.
       CopyFilesToOutputDirectory:
         templateLibrary -> S:\play\templateLibrary\bin\Debug\netstandard2.0\templateLibrary.dll
     1>Done Building Project "S:\play\templateLibrary\templateLibrary.csproj" (default targets).

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.03

Biggest downside I see of this is that the logo is no longer guaranteed to be the first line.

Would folks be happy with that?

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

No branches or pull requests

9 participants