Implement the Visual Studio build log converter #1533

ghost · 2019-06-13T16:21:19Z

Compare the test file SomeErrors.txt (the input) to SomeErrors.sarif (the output) to see the implementation choices I made. In particular, I did not segregate those lines that mentioned a build tool (such as "LINK") into separate run objects. IMO the entire log represents a single "run" -- the build.

It is not at all clear what to put in run.tool.name. It's not even necessarily true that the log was produced by Visual Studio or even MSBuild. Any script that ran the underlying tools (csc, link, rc, ...) would produce error messages in the same format. Suggestions welcome.

Also: - Fix #1524: ResultLogJsonWriter emits language "en-US" - Fix #1525: FileDiffingUnitTests doesn't notice spurious default-valued properties

ghost · 2019-06-13T16:23:19Z

src/Sarif/Writers/ResultLogJsonWriter.cs

@@ -294,7 +294,7 @@ public void CompleteRun()

            SerializeIfNotNull(_run.Tags, "tags");
            SerializeIfNotNull(_run.Conversion, "conversion");
-            SerializeIfNotNull(_run.Language, "language");
+            SerializeIfNotDefault(_run.Language, "language", "en-US");


SerializeIfNotDefault [](start = 12, length = 21)

This is the fix for #1524, "ResultLogJsonWriter emits language 'en-US'". Note that I hard-coded the default -- this string appears in several places and I chose not to make today the day I DRY it out. #Resolved

ghost · 2019-06-13T16:24:35Z

src/Test.Utilities.Sarif/FileDiffingUnitTests.cs

@@ -188,24 +188,26 @@ protected static string GenerateDiffCommand(string suiteName, string expected, s

        public static bool AreEquivalent<T>(string actualSarif, string expectedSarif, IContractResolver contractResolver = null)
        {
-            if (actualSarif.Equals(expectedSarif)) { return true; }
+            expectedSarif = expectedSarif ?? "{}";


expectedSarif [](start = 12, length = 13)

The changes in this method are the fix for #1525, "FileDiffingUnitTests don't notice spurious default-valued properties".

ghost · 2019-06-13T16:24:54Z

src/Test.Utilities.Sarif/DeletesOutputsDirectoryOnClassInitializationFixture.cs

@@ -16,12 +16,12 @@ namespace Microsoft.CodeAnalysis.Sarif
    /// See https://xunit.github.io/docs/shared-context for more information about xUnit class fixtures.
    ///
    /// This particular fixture deletes any existing test output files that may have been produced
-    /// by a previous run. It is designed for use on test classes that derive from FileDiffingTests.


FileDiffingTests [](start = 83, length = 16)

Wrong class name. #Resolved

michaelcfanning · 2019-06-13T22:48:47Z

src/ReleaseHistory.md

+* BUGFIX: Add `Result.Level` (and remove `Result.Rank`) for Fortify Converter based on MicroFocus feedback.
+* API NON-BREAKING: Provider a SARIF converter for Visual Studio log files.


Provider [](start = 20, length = 8)

s/be 'Provide' #Resolved

Fixed.

In reply to: 293605399 [](ancestors = 293605399)

michaelcfanning · 2019-06-13T22:49:32Z

have you tested your converter in all three viewers? vs. vs code, and ado results explorer?
have you tried open vey large build logs from actual product builds? #Pending

ghost · 2019-06-13T23:28:21Z

Will send you results in email.

In reply to: 501906158 [](ancestors = 501906158)

ghost · 2019-06-17T18:53:36Z

@harleenkohli is added to the review. #Closed

harleenkohli · 2019-06-17T21:01:55Z

The files in the Inputs directory have the extension .txt rather than the more natural .log because the repo's .gitignore file excludes .log files.

very NIT: could a better place for this readme be inside the "Inputs" folder? #ByDesign

Refers to: src/Test.UnitTests.Sarif.Converters/TestData/VisualStudioBuildLogConverter/README.md:1 in 2caebcf. [](commit_id = 2caebcf, deletion_comment = False)

harleenkohli · 2019-06-17T21:02:50Z

The files in the Inputs directory have the extension .txt rather than the more natural .log because the repo's .gitignore file excludes .log files.

(cool trick for getting the gitignore issue out of the way)

In reply to: 502849560 [](ancestors = 502849560)

Refers to: src/Test.UnitTests.Sarif.Converters/TestData/VisualStudioBuildLogConverter/README.md:1 in 2caebcf. [](commit_id = 2caebcf, deletion_comment = False)

harleenkohli · 2019-06-17T21:05:25Z

src/ReleaseHistory.md

@@ -382,7 +382,7 @@
 * BUGFIX: Multitool crashes on launch: Can't find CommandLine.dll. https://github.com/microsoft/sarif-sdk/issues/1487

 ## **v2.1.2** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.1.2) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.1.2) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.1.2) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.1.2)
-* API BREAKING: Change location.logicalLocation to logicalLocations array. https://github.com/oasis-tcs/sarif-spec/issues/414
+* API BREAKING: Change result.logicalLocation to logicalLocations array. https://github.com/oasis-tcs/sarif-spec/issues/414

 ## **v2.1.3** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.1.3) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.1.3) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.1.3) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.1.3)


/Sarif.Sdk/2.1.3) [](start = 50, length = 17)

Should we call out that its MSBuild log file ? (VS logs could mean something more generic?) #Pending

sigh I did struggle with the naming. I supposed MSBuild is better. Even that's not perfect because any tool or script that produces error lines in this standard format will work. I'll merge this but will ask for a call from Michael.

In reply to: 294517116 [](ancestors = 294517116)

harleenkohli

ghost · 2019-06-17T21:19:48Z

The files in the Inputs directory have the extension .txt rather than the more natural .log because the repo's .gitignore file excludes .log files.

I put the file here because this is the folder that contains the affected files. My usual "scope reduction" approach :)

In reply to: 502849825 [](ancestors = 502849825,502849560)

Refers to: src/Test.UnitTests.Sarif.Converters/TestData/VisualStudioBuildLogConverter/README.md:1 in 2caebcf. [](commit_id = 2caebcf, deletion_comment = False)

Larry Golding added 28 commits June 7, 2019 13:55

Introduce VisualStudioBuildLogConverter class and unit test class.

58f3288

Validate arguments to Convert method.

199d5b8

Empty input produces run with empty results.

9da17c6

Introduce GetResults method.

8cd5f05

Introduce GetResults(reader)

84d7e5d

Introduce GetResultFromLine(string line).

d9e66af

First real test: Convert log with no errors.

879d24f

Also: - Fix #1524: ResultLogJsonWriter emits language "en-US" - Fix #1525: FileDiffingUnitTests doesn't notice spurious default-valued properties

Emit one result per distinct error line.

419ec8e

Parse entire error line.

104d47f

Fix regex to not include leading colon in message.

ba1df94

Populate result.message.

65ddff6

Populate ruleId.

72aaff4

Populate level.

ce7a9fa

Populate location.physicalLocation.uri.

12e707a

Populate region for (m,n) case.

47f94ee

Parse startLine-endLine region format.

cc7c3a6

Parse startLine-only region format.

abfe823

Parse startLine,startColumn-endColumn region format.

2a0ec16

Parse startLine,startColumn,endLine,endColumn region format.

4db63cf

Handle error line with tool name and no location.

2a443c4

Modify regexes to prepare for text in front of error|warning.

b20d74d

Move some comments around.

47bf0fd

Handle "level qualification" (text before error|warning).

c7477bb

DRY out line patterns.

67c06e5

Line hash list can't be static.

aaee455

DRY out with helper SetPropertyIfPresent.

b79aff4

Add entire error line to property bag.

bf9848c

Rename property "line" to "fullMessage".

051065c

ghost requested review from michaelcfanning and harleenkohli June 13, 2019 16:21

ghost commented Jun 13, 2019

View reviewed changes

Larry Golding added 2 commits June 13, 2019 09:47

Update release history.

83d5c3a

Merge branch 'master' into users/lgolding/build-log-converter

8612b1f

michaelcfanning reviewed Jun 13, 2019

View reviewed changes

Fix typo in release history.

fcab66a

Fix a couple of IDE warnings.

2caebcf

Merge branch 'master' into users/lgolding/build-log-converter

e22995e

harleenkohli reviewed Jun 17, 2019

View reviewed changes

harleenkohli approved these changes Jun 17, 2019

View reviewed changes

ghost merged commit 44f0e46 into master Jun 17, 2019

ghost deleted the users/lgolding/build-log-converter branch June 17, 2019 21:23

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the Visual Studio build log converter #1533

Implement the Visual Studio build log converter #1533

ghost commented Jun 13, 2019

ghost Jun 13, 2019 •

edited by michaelcfanning

Loading

ghost Jun 13, 2019 •

edited by ghost

Loading

ghost Jun 13, 2019 •

edited by ghost

Loading

michaelcfanning Jun 13, 2019 •

edited by ghost

Loading

ghost Jun 13, 2019

michaelcfanning commented Jun 13, 2019 •

edited by ghost

Loading

ghost commented Jun 13, 2019

ghost commented Jun 17, 2019

harleenkohli commented Jun 17, 2019 •

edited by ghost

Loading

harleenkohli commented Jun 17, 2019

harleenkohli Jun 17, 2019 •

edited by ghost

Loading

ghost Jun 17, 2019

harleenkohli left a comment

ghost commented Jun 17, 2019

		* BUGFIX: Add `Result.Level` (and remove `Result.Rank`) for Fortify Converter based on MicroFocus feedback.
		* API NON-BREAKING: Provider a SARIF converter for Visual Studio log files.

Implement the Visual Studio build log converter #1533

Implement the Visual Studio build log converter #1533

Conversation

ghost commented Jun 13, 2019

ghost Jun 13, 2019 • edited by michaelcfanning Loading

Choose a reason for hiding this comment

ghost Jun 13, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

ghost Jun 13, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

michaelcfanning Jun 13, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

ghost Jun 13, 2019

Choose a reason for hiding this comment

michaelcfanning commented Jun 13, 2019 • edited by ghost Loading

ghost commented Jun 13, 2019

ghost commented Jun 17, 2019

harleenkohli commented Jun 17, 2019 • edited by ghost Loading

harleenkohli commented Jun 17, 2019

harleenkohli Jun 17, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

ghost Jun 17, 2019

Choose a reason for hiding this comment

harleenkohli left a comment

Choose a reason for hiding this comment

ghost commented Jun 17, 2019

ghost Jun 13, 2019 •

edited by michaelcfanning

Loading

ghost Jun 13, 2019 •

edited by ghost

Loading

ghost Jun 13, 2019 •

edited by ghost

Loading

michaelcfanning Jun 13, 2019 •

edited by ghost

Loading

michaelcfanning commented Jun 13, 2019 •

edited by ghost

Loading

harleenkohli commented Jun 17, 2019 •

edited by ghost

Loading

harleenkohli Jun 17, 2019 •

edited by ghost

Loading