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

Handle non-existent directories the same for Multi/Single-threaded analysis #2461

Merged
merged 6 commits into from
Apr 26, 2022

Conversation

marmegh
Copy link
Contributor

@marmegh marmegh commented Feb 25, 2022

Updated multithreaded analysis to incorporate a directory validation as already implemented for single-threaded analysis. Also, added a test to prevent regressions for either. This is to account for any customers using sarif that may be dependent on the single threaded implementation. This is to address issue #2457.

Fixes #2457

@eddynaka
Copy link
Collaborator

@marmegh , what is the status of this PR?

@@ -9,6 +9,7 @@
* BUGFIX: Fix `ArgumentException` when `--recurse` is enabled and two file target specifiers generates the same file path. [#2438](https://github.com/microsoft/sarif-sdk/pull/2438)
* 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](https://github.com/microsoft/sarif-sdk/pull/2447)
* BUGFIX: Fix `Merge` command produces empty SARIF file in Linux when providing file name only without path. [#2408](https://github.com/microsoft/sarif-sdk/pull/2408)
* BUGFIX: Fix `UnhandledEngineException` when target path does not exist for multithreaded application by validating directories as is done for singlethreaded analysis. []()
Copy link
Member

@michaelcfanning michaelcfanning Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnhandledEngineException

This change looks good but you'll need to update your release notes, since we shipped v.2.14 #Resolved

@marmegh marmegh marked this pull request as ready for review April 26, 2022 00:53
@marmegh marmegh requested a review from eddynaka as a code owner April 26, 2022 00:53
Copy link
Collaborator

@eddynaka eddynaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@michaelcfanning michaelcfanning merged commit 503fe29 into main Apr 26, 2022
@michaelcfanning michaelcfanning deleted the users/marmegh/fixMultithreadPathBug branch April 26, 2022 17:08
@eddynaka eddynaka mentioned this pull request Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MultithreadAnalyzeCommandBase is throwing exception when path provided in target specifiers does not exist
3 participants