Fix merge bug in Linux when only pass file name without path #2408

shaopeng-gh · 2021-11-11T04:57:43Z

Description:

merge bug in Linux when only pass file name, like this command:
merge --force --merge-runs --output-directory=result/ --output-file=MergeResult.sarif FileNameOnly.sarif

Fixes:

The issue is our code use windows only file path, fixed the code.
added Linux and macOS build and test pipeline.
fix all build and test script for Linux issues encountered.
fixed some tests not working in Linux btw since the new test I am adding is in the same place.
also went though all failed tests and fix those easily fixable like path issue.
for those have like .exe usage and win32 calls and need more time to see how to fix, added a attribute to skip them for now and fix in separate changesets.

shaopeng-gh · 2021-11-11T18:26:43Z

src/Sarif.Multitool.Library/MergeCommand.cs

@@ -273,7 +273,7 @@ private async Task<bool> FindFilesAsync()

                if (directory.Length == 0)
                {
-                    directory = @".\";
+                    directory = Directory.GetCurrentDirectory();
                }


just a quick update that
will update the azure pipeline and add ubuntu-latest for this pr to see how the tests run in Linux. #Closed

able to add azure pipeline to also run test in Linux
but I found it does not even build before run unit test,
I think the Powershell script we have have the syntax for window only, I will check and fix issues.

fixed, also fixed the build in macos so tests can see how the test run in macos as well

…com/microsoft/sarif-sdk into users/shaopeng-gh/mergeissueinlinux

yongyan-gh · 2021-11-17T18:37:32Z

ado-build.yml

 jobs:
 - job: NET_pipeline
+  strategy:


consider adding "maxParallel" to reduce overall build time #Closed

…RelativeWithTrailingSlash

shaopeng-gh · 2022-02-14T08:36:09Z

src/Test.UnitTests.Sarif.Multitool.Library/Test.UnitTests.Sarif.Multitool.Library.csproj

-    <None Remove="TestData\ValidateCommand\Configuration.xml" />
-    <None Remove="TestData\ValidateCommand\ValidateSarifOneZeroZero.sarif" />
-    <None Remove="xunit.runner.json" />
-  </ItemGroup>


Michael: This is junk that VS insists on adding. You can see it's not in sync w/what's below, let's just delete it. #Closed

michaelcfanning · 2022-02-16T22:22:31Z

BUGFIX: Fix 'InvalidOperationException' with message Collection was modified; enumeration operation may not execute in MultithreadedAnalyzeCommandBase, which is raised when analyzing with the --hashes switch. #2447

Why did this release note get deleted? Let's restore it.

In reply to: 1042368013

Refers to: src/ReleaseHistory.md:10 in 6f84064. [](commit_id = 6f84064, deletion_comment = True)

michaelcfanning · 2022-02-16T22:25:49Z

src/Test.UnitTests.Sarif/ExtensionsTests.cs

+                (@"\\hostname/path\file.ext", expectedResult),
+                (@"file:///C:\path/file.ext", expectedResult),
+                (@"\\hostname/c:\path\file.ext", expectedResult),
+                (@"\home\username/path/file.ext", expectedResult),


home

Should we test the ~\path convention? #Resolved

shaopeng-gh · 2022-02-17T23:42:22Z

BUGFIX: Fix 'InvalidOperationException' with message Collection was modified; enumeration operation may not execute in MultithreadedAnalyzeCommandBase, which is raised when analyzing with the --hashes switch. #2447

not an issue was looking at old version

In reply to: 1042368013

Refers to: src/ReleaseHistory.md:10 in 6f84064. [](commit_id = 6f84064, deletion_comment = True)

shaopeng-gh and others added 3 commits November 10, 2021 18:33

fix current test

ef3453c

Actual code fix

186ec49

Add Test

d17fdf8

shaopeng-gh requested review from eddynaka and michaelcfanning as code owners November 11, 2021 04:57

shaopeng-gh requested a review from yongyan-gh November 11, 2021 04:57

shaopeng-gh added 2 commits November 10, 2021 21:02

Fix format

7b6dcd0

fix the test with space

249a059

shaopeng-gh commented Nov 11, 2021

View reviewed changes

shaopeng-gh marked this pull request as draft November 11, 2021 21:02

shaopeng-gh and others added 8 commits November 11, 2021 13:06

try build strategy

a6af4b1

try build strategy

ed19a6b

Merge branch 'users/shaopeng-gh/mergeissueinlinux' of https://github.…

6ea1d33

…com/microsoft/sarif-sdk into users/shaopeng-gh/mergeissueinlinux

Merge branch 'users/shaopeng-gh/mergeissueinlinux' of https://github.…

9b4ed87

…com/microsoft/sarif-sdk into users/shaopeng-gh/mergeissueinlinux

Merge branch 'users/shaopeng-gh/mergeissueinlinux' of https://github.…

11efbad

…com/microsoft/sarif-sdk into users/shaopeng-gh/mergeissueinlinux

fix space

9005e12

Try Linux build

18239f3

Test with old code should fail

94d4dfc

yongyan-gh reviewed Nov 17, 2021

View reviewed changes

shaopeng-gh added 11 commits November 17, 2021 19:05

try build

42f61ee

test Resolve-Path

2038816

test ExecCommand

adb748f

ExecCommand

b765474

fix Join-Path

b11283b

PowerShell 7

7903e0d

PowerShell 3

92d483f

$ENV:OS

bd7f76a

$ENV:OS Only

5bc844a

Join-Path fix

1d2cc1d

no Otherwise

daf795d

shaopeng-gh linked an issue Feb 9, 2022 that may be closed by this pull request

merge produces empty SARIF file regardless of the inputs #2400

Closed

shaopeng-gh marked this pull request as draft February 9, 2022 19:56

shaopeng-gh added 2 commits February 9, 2022 23:13

fix comments part 1

59fda0b

fix test FortifyFprConverter_GetOriginalUriBaseIdsDictionary_sourceIs…

5df976f

…RelativeWithTrailingSlash

shaopeng-gh mentioned this pull request Feb 13, 2022

Fix GitHelper.Default.GetTopLevel in Linux and revert FileDiffingUnitTests.GetProductDirectory #2450

Open

shaopeng-gh added 9 commits February 13, 2022 00:17

Fix comments part 2

2dc14cb

Merge branch 'main' into users/shaopeng-gh/mergeissueinlinux

ce7d09f

update test AnalyzeCommandBase_GetFileNameFromUriWorks

3f45add

use PathSeparator

7ab58a5

fix $TestFilter

feb2aca

try to use $NonWindowsOptions

b4f6cb3

revert Regex change in Stack.Create()

3c98a71

revert not needed change in StackFrame.cs

8061eb8

change FileNameOnlyWithoutPath.sarif to have only 2 result instead of 8

0c7e2f4

shaopeng-gh mentioned this pull request Feb 14, 2022

Merge command split by rule while Option.SplittingStrategy is None #2453

Open

microsoft deleted a comment from yongyan-gh Feb 14, 2022

shaopeng-gh commented Feb 14, 2022

View reviewed changes

microsoft deleted a comment from michaelcfanning Feb 14, 2022

shaopeng-gh marked this pull request as ready for review February 14, 2022 08:41

shaopeng-gh requested a review from michaelcfanning February 14, 2022 08:42

shaopeng-gh added 4 commits February 15, 2022 16:04

Merge branch 'main' into users/shaopeng-gh/mergeissueinlinux

7b88cc5

Fix release history order

599b054

try fix new test for Linux

ea39610

fix Linux uri issue

169fdc6

michaelcfanning reviewed Feb 16, 2022

View reviewed changes

update powershell & use simple syntax & add ~ test

e58eaba

michaelcfanning merged commit 92cb8e0 into main Feb 18, 2022

michaelcfanning deleted the users/shaopeng-gh/mergeissueinlinux branch February 18, 2022 00:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix merge bug in Linux when only pass file name without path #2408

Fix merge bug in Linux when only pass file name without path #2408

shaopeng-gh commented Nov 11, 2021 •

edited

Loading

shaopeng-gh Nov 11, 2021 •

edited

Loading

shaopeng-gh Nov 12, 2021

shaopeng-gh Nov 18, 2021

yongyan-gh Nov 17, 2021 •

edited by eddynaka

Loading

shaopeng-gh Nov 18, 2021

shaopeng-gh Feb 14, 2022 •

edited

Loading

shaopeng-gh Feb 14, 2022

michaelcfanning commented Feb 16, 2022 •

edited by shaopeng-gh

Loading

michaelcfanning Feb 16, 2022 •

edited by shaopeng-gh

Loading

shaopeng-gh Feb 17, 2022

shaopeng-gh commented Feb 17, 2022

Fix merge bug in Linux when only pass file name without path #2408

Fix merge bug in Linux when only pass file name without path #2408

Conversation

shaopeng-gh commented Nov 11, 2021 • edited Loading

Description:

Fixes:

shaopeng-gh Nov 11, 2021 • edited Loading

Choose a reason for hiding this comment

shaopeng-gh Nov 12, 2021

Choose a reason for hiding this comment

shaopeng-gh Nov 18, 2021

Choose a reason for hiding this comment

yongyan-gh Nov 17, 2021 • edited by eddynaka Loading

Choose a reason for hiding this comment

shaopeng-gh Nov 18, 2021

Choose a reason for hiding this comment

shaopeng-gh Feb 14, 2022 • edited Loading

Choose a reason for hiding this comment

shaopeng-gh Feb 14, 2022

Choose a reason for hiding this comment

michaelcfanning commented Feb 16, 2022 • edited by shaopeng-gh Loading

michaelcfanning Feb 16, 2022 • edited by shaopeng-gh Loading

Choose a reason for hiding this comment

shaopeng-gh Feb 17, 2022

Choose a reason for hiding this comment

shaopeng-gh commented Feb 17, 2022

shaopeng-gh commented Nov 11, 2021 •

edited

Loading

shaopeng-gh Nov 11, 2021 •

edited

Loading

yongyan-gh Nov 17, 2021 •

edited by eddynaka

Loading

shaopeng-gh Feb 14, 2022 •

edited

Loading

michaelcfanning commented Feb 16, 2022 •

edited by shaopeng-gh

Loading

michaelcfanning Feb 16, 2022 •

edited by shaopeng-gh

Loading