MSB4132 is emitted when a task logs errors in custom format

[sae42]

Steps to reproduce

This is a new issue in VS2019 16.6 Preview 1 with our custom build tasks. The issue looks to have been introduced by #4940

The issue occurs because we use our own custom log format when building within Visual Studio. We use the standard error format during command line builds so there is no problem there.
Our custom format allows us to give a richer experience navigating errors and populating the Visual Studio Error List (which allows custom columns etc). Essentially we have our own set of RegEx parse routines (similar to CanonicalError in MSBuild). Since we hook the output in IDE builds we can create the appropriate errors in the Visual Studio Error list. This is similar to the default functionality offered by VS ParseOutputStringForTaskItem However, the build tasks (which use TaskLoggingHelper.LogMessagesFromStream() ) will fail to match the format and incorrectly assume no errors were output. The MSB4132 error is then emitted.

One easy workaround/fix would have been to manually set the TaskLoggingHelper's HasLoggedErrors property, but that is unfortunately is only gettable. The only other simple fix I have is to parse the output for this new MSB4132 error and throw it away.

It would be useful if there was a way to opt out of this change in behavior. The code change itself seems to have various exemptions and I see other issues raised concerning this, so it would be good to have a simple way for any task to bypass this.

Expected behavior

Same as pre 16.6. Don't show: "...error MSB4132: The "[TaskName]" task returned false but did not log an error."

Actual behavior

"...error MSB4132: The "[TaskName]" task returned false but did not log an error."

Environment data

OS info:

Visual Studio 16.6 Preview 1

[rainersigwald]

@benvillalobos can you take a look at this?

[Forgind]

Hi @sae42,

We came up with a few ways we could add this:

1. Normal escape hatch with environment variable
2. Add a new parameter via IBuildEngine that you can set in each task
3. Look for a property and only turn on the error message when that property is not set to false

I'm leaning towards #2. #1 is inconvenient from VS, and #3 would change behavior in all projects that have that property set, so if your package comes in from a NuGet package, it would affect your users' error logging, too. #2 feels like overkill, but I think it's the most practical.

What do you think?

[sae42]

Hi @Forgind . The trouble I foresee with 2) is that I assume it would require the build tasks to be compiled against 16.6 making it the pre-req for the extension. That's not necessarily a deal-breaker, but something to bear in mind. If there was a way to do that for existing (i.e pre-16.6) build tasks, so that they could be kept to pre 16.6 behavior, that would be great. But I'm not sure that's possible.
Option 3) sounds like it would resolve the compatibility problem; from my perspective that property exists right now ('BuildingInsideVisualStudio'), but setting anything as say an MSBuild global property would work. Would it be possible to provide both solutions?

[Forgind]

If it comes in a BuildEngine, the BuildEngine would be there either way; the only question would be how new it is, so you can say something like if (BuildEngine is IBuildEngine7)... and it will work for any MSBuild.

[sae42]

Yes, that approach should work. The pre-16.6 built code would have to have its own copy of the interface though as it won't exist in the version it's compiled against. I'd be happy to try this out if you could get this into a preview build.

[Forgind]

Great! I'll link to this issue when the PR is ready. If you need help pulling it into your changes, I can try to help with that.

[Forgind]

I put a change out in #5207. I haven't rigorously tested it yet, but if you want to test it on your project a little early, feel free.

This will create a merge conflict with #5191, but it should be easy to straighten out.

[sae42]

Thanks. I tried building it myself and it seems to work. A few places were returning IBuildEngine6 rather than IBuildEngine7.

[Forgind]

Good catch—I fixed that.

I thought about it more, and other than ensuring the older MSBuild has access to the new interface, you could write a task using reflection similar to the one I provided in the (updated) test. It's probably the easiest solution.

[sae42]

Thanks, using reflection is probably a better option than my current workaround of filtering out the error in the logger. Just out of curiosity, when I made that change I noticed that MSB4132 is not unique - it is already defined as 'UnrecognizedToolsVersion'. So, my change had to be more than just filtering on the error code.

[rainersigwald]

@sae42 Nice catch, we should change the new error's code so that it's unique. @Forgind, while you're in here?

[Forgind]

Fixed