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

No way to set /nowarn:MSB3277 from Visual Studio #1886

Closed
konste opened this issue Mar 18, 2017 · 27 comments
Closed

No way to set /nowarn:MSB3277 from Visual Studio #1886

konste opened this issue Mar 18, 2017 · 27 comments
Labels

Comments

@konste
Copy link

konste commented Mar 18, 2017

For a long time even Minimal verbosity build logs were polluted by warning MSB3277: Found conflicts between different versions of the same dependent assembly that could not be resolved, and there were no way to suppress those as they were coming directly from MSBuild.

In MSBuild 15 FINALLY we got new option /nowarn:MSB3277 which supposed to suppress that and some other MSBuild warnings. That was the primary reason for our switch to VS 15 as soon as it become available.

But now for the life of me I cannot figure out how this option could be set for Visual Studio builds! For the command line builds where we invoke MSBuild directly it is obvious, but most devs use VS and they seem out of luck - no MSBuild command line and no way to set /nowarn:MSB3277

I understand that VS most probably does not have UI to set that option and it is Ok. But I sincerely hope that there is a way to tweak the project file or machine wide configuration or SOMETHING to make this option effective for VS originated builds.

Any advice, please?

Konstantin

@jeffkl
Copy link
Contributor

jeffkl commented Mar 20, 2017

Unfortunately, we did not get enough time to update Visual Studio to support this MSBuild feature. We hope to bring it to a future release.

Visual Studio does builds via the MSBuild object model so there is no way to get it to pass the list of warnings to suppress until it is added as a feature. You can edit the "global" MSBuild.rsp located next to MSBuild.exe to affect all command-line builds for your machine.

@konste
Copy link
Author

konste commented Mar 20, 2017

Well, at least I'm not missing something obvious. We were waiting for this switch for years, guess we can wait a little more. And for command line builds we have full access to the actual command line and can just add /nowarn there explicitly.
Let's keep this issue open as a reminder that the feature must be exposed in VS to get its full usefulness.

@jeffkl
Copy link
Contributor

jeffkl commented Mar 20, 2017

I have reached out to some Visual Studio devs to try and get a concrete time frame as well.

@rwasef1830
Copy link

rwasef1830 commented Mar 20, 2017

I've been plagued by MSB3277 since manually migrating my net46 projects from xproj to csproj.

As I understand it, the warning is harmless because compiler resolves the references by itself ? But still I get "Consider adding a redirect for ..." message for each assembly and sometimes a warning with a chunk of assemblyBinding XML in it as well when I build projects using both dotnet build and vs2017.

The warning tells me to add <AutoGenerateBindingRedirects>True</AutoGenerateBindingRedirects> to my csproj file but doing so does absolutely nothing.

Is this a normal side effect but the old tooling always suppressed this warning ? I'm confused and can't find clear guidance on the situation.

Also, DotNetCli tools seem to need manual redirects, for example https://github.com/mrahhal/Migrator.EF6 (which is a replacement for powershell EF6 commands which now don't work in VS) fails with Npgsql because EntityFramework6.Npgsql depends on 3.1.2 whereas the newest is 3.2.1 (and I'm referencing both). It gives the standard "assembly version doesn't match manifest" which doesn't go away unless I manually add a dotnet-ef.exe.config with handwritten binding redirects in it in the project output dir.

My apologies if this is out of place but I am very confused and hope for any clarifications.

@jeffkl
Copy link
Contributor

jeffkl commented Mar 20, 2017

MSBuild will attempt to "unify" the references but there could be runtime exceptions. That is what the warning is saying.

Let's say you are building ProjectA and ProjectB. ProjectA depends on Newtonsoft.Json version 1.0 and ProjectB depends on Newtonsoft.Json version 2.0. ProjectA also depends on ProjectB so the output folder of ProjectA should contain:

  • ProjectA.dll
  • ProjectB.dll
  • Newtonsoft.Json.dll

So the problem here is that ProjectA or ProjectB will end up with the wrong version of Newtonsoft.Json in the output directory. The build succeeded but at run time you will probably get an exception telling you that version X of Newtonsoft.Json wasn't found. Assembly binding redirects solve this because it can upgrade/downgrade assembly versions at run time. So a redirect of Newtonsoft.Json version 1.0 to 2.0 would make the example or ProjectA and ProjectB work. When ProjectA asked for Newtonsoft.Json 1.0, it would actually receive 2.0. <AutoGenerateBindingRedirects /> automatically generates the redirects in your applications app.conffig but that only works for runtime apps like .exe and won't work for class libraries since they don't control the runtime.

From my example, what you really should do is unify the references yourself by having ProjectA and ProjectB reference the same versions of their common dependencies. The warning tries to convey this but people have rightfully complained that it isn't clear enough. We do have an open issue to address the user friendliness of the warning (#608).

The real issues can arise if you only own ProjectA and ProjectB still conflicts with what you want to use. There are certainly cases where you just can't unify the references because the actual run time app (like a Console app) aren't owned by you. This is where you'd really want to suppress the warning because you know that eventually a runnable app downstream will actually fix it with assembly binding redirects.

@danmoseley
Copy link
Member

MSBuild could read an environment variable like MSBUILDADDITIONALCOMMANDLINEFLAGS. There are of course environment variables for various handy purposes sprinkled all over MSBuild and they've not really been a support problem especially innocuous ones like this would be. They sometimes help folks out of jams like this.

@jeffkl
Copy link
Contributor

jeffkl commented Mar 22, 2017

@danmosemsft Visual Studio would need to set the BuildParameters.WarningsAsMessages property since those builds happen via the object model and not the command-line.

We are working hard to improve the feature so its available in Visual Studio but we wanted to get this first iteration out since it provides so much value.

@danmoseley
Copy link
Member

@jeffkl right, but a hypothetical env var would have allowed folks to get this behavior meantime.

@rainersigwald
Copy link
Member

I think I'd rather see a possible specific MSBUILDWARNINGSASERRORS variable than AdditionalCommandLineFlags based on implementation--it might be tough to find the right time to parse command line arguments in an API build.

Having that would be fairly unwieldy, though, since it'd affect a whole environment/VS process, and not be per-solution. That might be painful if you regularly switch between solutions with different warning states. But it'd be nice for people who mostly use a single solution. I don't feel strongly either way (except that debugging might be annoying, so we'd have to carefully log why warnings got ignored).

@danmoseley
Copy link
Member

You're right, since the command line parser isn't really involved, it would have to be more specific like that.

One can always find reasons why something global is not a good idea or would not work, but those folks can simply not use it.

@konste
Copy link
Author

konste commented Mar 22, 2017

I find global suppression of warnings (any type of warnings) dangerous as it allows for real problems to slip through. What I would like to see is the possibility to control those warnings per project by setting some MSBuild property. I only have a couple projects out of many hundreds where MSB3277 is reported, investigated and found benign. Now I want to suppress it for those two projects only. Putting aside possible VS UI for that all I need is to define MSBuild property in each project (something like MSBUILDNOWARN) which lists warning numbers to suppress. Similar how it is done for compiler warnings.

Can you make MSBuild code check for the presence and value of such property right before issuing the warning and suppress it if it is requested?

That would avoid unnecessary globality of suppression and at the same time relieve MSBuild from decision WHEN to check for it if it is not coming from the command line.

What do you say?

@jeffkl
Copy link
Contributor

jeffkl commented Mar 22, 2017

The issue with defining these for each project is that a single build episode can build multiple projects. For instance, you're building a solution with two projects. MSBuild creates one set of loggers for the entire build and one master list of warnings to suppress.

MSBuild also logs warnings during project parsing which you would not be able to suppress because we'd read them from the project after its been parsed.

In order to ensure that your defined set of all warning codes are treated correctly, this only worked as a command-line option as well as a build parameter to the object model.

To place warning codes in the project file we'd have to:

  1. Place warnings from parsing in a queue and not actually log them until after we've parsed the project fully
  2. Keep a dictionary of codes per project instead of a global list.
    a. If we wanted to keep the command-line argument, we'd need a global list as well as a per-project-list
  3. When mutating warnings, check the per-project-list as well as the global list

I wanted the feature to add as little overhead as possible and this could potentially cause more memory usage and slow the build down a little more. But if the overwhelming feedback is that users want a per-project, MSBuild property-based definition for warnings to suppress, then I'll see what I can do!

@konste
Copy link
Author

konste commented Mar 22, 2017

Now when I realized that /NOWARN is not only specified globally, but actually can only be applied globally, it looks more like a danger than convenience. Just imagine that you suppress some compiler warning across ALL projects - current and future! What a huge opportunity to hide a real problem and make it really expensive and hard to discover at runtime.
MSBuild warnings exist not for the sole reason to annoy people :-) they actually try to warn about some potentially dangerous conditions. Yes, sometimes there are false positives and it definitely helps to suppress those after investigation confirms that in each particular case it is a false positive, but it would be major overkill (and mistake) to suppress them globally and therefore allow the real problems to slip through.

Let's put aside how /NOWARN is (or may be) specified. The real point is - it MUST be applied with at least project granularity. Ideally I would even like to see it applied with "per conflict" granularity, but this indeed may be too much to ask. Per project granularity looks like a sweet middle ground.

We have solution(s) with hundreds of projects and a few MSB3277 which are confirmed to be false positives, but I rather let them spoil our server build report than suppress them globally and lose detection of potential non-trivial conflicts between hundreds of external libraries we are using.

Please consider my feedback overwhelming :-) I really think the feature as it is implemented now causes more harm than good.

jeffkl added a commit to jeffkl/msbuild that referenced this issue Mar 30, 2017
…ings.

Introduce MSBuildTreatWarningsAsErrors property that when set to true will treat all warnings as errors when building a project.
Introduce MSBuildWarningsAsErrors property which is a semicolon delmited list of warning codes to treat as errors when building a project.
Introduce MSBuildWarningsAsMessages property which is a semicolon delimited list of warning codes to treat as low importance messages.

This allows users to control warnings at the project level via standard MSBuild properties.  They can be set in a project, an import, or from the command-line.

The limitation here is that the warnings can only include ones that occur during a build.  This is because the properties are not read until after parsing so warnings generated during parse/evaluation happen too soon.  For these warnings, users will only be able to treat them as errors/messages with the /WarnAsError and /WarnAsMessage command-line arguments.

Closes dotnet#1886
jeffkl added a commit to jeffkl/msbuild that referenced this issue Apr 3, 2017
…ings.

Introduce MSBuildTreatWarningsAsErrors property that when set to true will treat all warnings as errors when building a project.
Introduce MSBuildWarningsAsErrors property which is a semicolon delmited list of warning codes to treat as errors when building a project.
Introduce MSBuildWarningsAsMessages property which is a semicolon delimited list of warning codes to treat as low importance messages.

This allows users to control warnings at the project level via standard MSBuild properties.  They can be set in a project, an import, or from the command-line.

The limitation here is that the warnings can only include ones that occur during a build.  This is because the properties are not read until after parsing so warnings generated during parse/evaluation happen too soon.  For these warnings, users will only be able to treat them as errors/messages with the /WarnAsError and /WarnAsMessage command-line arguments.

Closes dotnet#1886
jeffkl added a commit that referenced this issue Apr 4, 2017
…ings (#1928)

* Implement ability for project properties to suppress and elevate warnings.

Introduce MSBuildTreatWarningsAsErrors property that when set to true will treat all warnings as errors when building a project.
Introduce MSBuildWarningsAsErrors property which is a semicolon delmited list of warning codes to treat as errors when building a project.
Introduce MSBuildWarningsAsMessages property which is a semicolon delimited list of warning codes to treat as low importance messages.

This allows users to control warnings at the project level via standard MSBuild properties.  They can be set in a project, an import, or from the command-line.

The limitation here is that the warnings can only include ones that occur during a build.  This is because the properties are not read until after parsing so warnings generated during parse/evaluation happen too soon.  For these warnings, users will only be able to treat them as errors/messages with the /WarnAsError and /WarnAsMessage command-line arguments.

Closes #1886
@konste
Copy link
Author

konste commented Apr 4, 2017

I got confused - what people were asking for and what command line option /nowarn was [over]-delivering is suppression of the false positive warnings. Now I can see they may be demoted from warnings to messages per project, but I don't see full suppression option! Am i missing anything? I was hoping to clean up build log from noise...

@jeffkl
Copy link
Contributor

jeffkl commented Apr 4, 2017

We decided to not allow you to just suppress everything and instead you must specify the individual warning codes to suppress. Generally this list isn't too long.

@konste
Copy link
Author

konste commented Apr 4, 2017

Understand that and agree, so how for example shall I suppress MSB3277 for particular project? I understand MSBuildWarningsAsMessages only demotes it from warning level to message level, but does not really suppress it. Hm-m?

@jeffkl
Copy link
Contributor

jeffkl commented Apr 5, 2017

@konste Warnings are mutated to messages but they have low importance which means they won't show up unless you set your Verbosity to Diagnostic. This effectively suppresses them.

@konste
Copy link
Author

konste commented Apr 5, 2017

@jeffkl I understand now. This is perfectly acceptable.
Can you make a guess when we see it as VS update? Or at least Release from master?

@jeffkl
Copy link
Contributor

jeffkl commented Apr 5, 2017

This made the cut for the next preview which I believe is shipping in the near future.

@konste
Copy link
Author

konste commented Apr 5, 2017

Super! Thank you, Jeff!

@jeffkl
Copy link
Contributor

jeffkl commented Apr 5, 2017

No problem, let me know if you have any problems with the new feature. It works pretty well in all of my testing.

@konste
Copy link
Author

konste commented Jun 11, 2017

@jeffkl I have related question which looks deceptively simple, but I could not find decent solution for years. I need some Targets executed for each and every project in Solution. I figured that pointing CustomAfterMicrosoftCommonTargets property to my common targets file does it, but where to set that property? When building with MSBuild I pass it as a command line parameter. But what to do for VS build? So far the best I could do was to define CustomAfterMicrosoftCommonTargets as machine wide environment variable and this way MSBuild sees it "through VS", but now it affects all projects built on the machine. Ugly.
"before..sln.targets" is another way to define solution wide global property, but VS ignores it!
How in the world can I execute something before solution starts to build?
I even tried to write VS add-in for that but it still was too global for the purpose and affected way more than it should.
Any other ideas?

@jeffkl
Copy link
Contributor

jeffkl commented Jun 12, 2017

If you're using MSBuild 15.0 or above, I added the import of Directory.Build.props at the top and Directory.Build.targets at the bottom of your project for a project's directory tree. You'll usually want to place the file at the root of your source code repository or a folder that is common to your projects.

http://blog.seravy.com/directory-build-targets-solution-wide-msbuild-target-part-2/

#751

@konste
Copy link
Author

konste commented Jun 12, 2017

@jeffkl I'm so glad I asked the right person and also so glad you actually went ahead and implemented this feature! It is a holy grail indeed - I was desperately looking for it for five years at least! Now if it would only be a little more discoverable, because I was looking for it recently again and could not find it. If I would not ask you directly I would still think it does not exist.

@rainersigwald
Copy link
Member

@konste I have some draft documentation in my email inbox for review right now . . . should be up on docs.microsoft.com before too long.

@rainersigwald
Copy link
Member

Directory.Build.props is now documented at https://docs.microsoft.com/en-us/visualstudio/msbuild/customize-your-build. Feedback is welcome!

@akrieger
Copy link

I can't find documentation on this anywhere, and this is the closest place I could find which seems reasonable to ask - is it expected that building projects from Visual Studio will ignore the global MSBuild.rsp file? I want to disable MSB4011 '"foo.props" cannot be imported again. It was already imported at "bar.vcxproj"', which must be specified as a command line item because the warning needs to be disabled at parse time, so it cannot be disabled by the very files causing the warning in the first place. Unfortunately, while I confirmed that commandline builds process MSBuild.rsp as expected, VS builds seem to ignore the file and I can't find a way to force them to not.

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

No branches or pull requests

7 participants