Getting rid off sarif sdk from codebase #1011
Conversation
AllDwarf
commented
Oct 1, 2024
AllDwarf
commented
- DEP: Update dotnet version to supported versions
- NEW: Remove sarif-sdk submodule and use nuget package instead
- DEP: Update nugate packages to latest versions
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| pCallback); | ||
| } | ||
| [SupportedOSPlatform("windows")] | ||
| private void WindowsNativeLoadPdbFromPEUsingDia(string peOrPdbPath, string symbolPath, string localSymbolDirectories) |
Check notice
Code scanning / CodeQL
Local scope variable shadows member Note
There was a problem hiding this comment.
I see numerous scripts are being updated with path changes from netcoreapp3.1 to net8.0 - maybe extracting that to a variable / constant (since we have a mix of cmd, sh, pwsh) makes sense - up to you.
Something to consider - splitting 144f302 into multiple commits targeted at dependency updates for specific projects and removing sarif-sdk submodule would make it easier to review.
| @@ -121,7 +121,6 @@ public ulong ReadLength(out bool is64bit) | |||
| /// <summary> | |||
| /// Reads the string from the current position in the stream. | |||
| /// </summary> | |||
| [HandleProcessCorruptedStateExceptions] | |||
There was a problem hiding this comment.
I see this is no longer supported in .NET 8, but what are the implications of removing this attribute?
| public const string Version = AssemblyVersion + Prerelease; | ||
| } | ||
| } | ||
| namespace Microsoft.CodeAnalysis.IL |
| @@ -15,18 +15,18 @@ | |||
| <projectUrl>https://github.com/microsoft/binskim</projectUrl> | |||
| <tags>binary analysis binskim binscope security portable executable linkable format sdl devops devsecops</tags> | |||
| <iconUrl>https://go.microsoft.com/fwlink/?linkid=2008940</iconUrl> | |||
| <dependencies> | |||
| <group targetFramework=".NETStandard2.1"> | |||
| <!-- <dependencies> | |||
There was a problem hiding this comment.
Is this intentional, or just WIP?
| @@ -36,6 +36,7 @@ public async Task TestTaskAsync() | |||
| } | |||
|
|
|||
| [TestMethod, TestCategory("NightlyTest")] | |||
| [Ignore("Skipping all tests in this class temporarily.")] | |||
There was a problem hiding this comment.
I assume we're skipping these since the results are nondeterministic right now, what's our plan long term to tackle these? Additionally - can you please extract this change into a separate commit, it's rather hidden in a bunch of other changes right now.
| ``` | ||
|
|
||
| #### Help command | ||
| The following command-lines invoke the general BinSkime help message. This message will display all the built-in ModernCop commands (help, analyze, capture, et al) for which more detailed help can be requested: |
|
|
||
| **Example:** `binskim.exe analyze c:\bld\*.dll --recurse true --output MyRun.sarif` | ||
| #### Analyze Command | ||
| The primary function of BinSkim is to analyze Windows portable executables (.dlls, .exes, etc). To analyze a file, pass one or more arguments that resolve one or more portable executables. |
There was a problem hiding this comment.
Are ELF and Mach-O intentionally not mentioned here?
There was a problem hiding this comment.
We may want to make the help a bit more cross-platform (i.e. .exe suffix in examples).
AllDwarf
force-pushed
the
users/marekaldorf/Separating_sarif_sdk_from_codebase
branch
from
5923dc8 to
ecb42e4
Compare
and update dotnet to 8 Mid step for MSDIA and linux changes Changes Update project to target .NET 8.0 and remove references to .NET Core 3.1 Update README.md to simplify it with UserGuide.md usage and remove unnecessary docx file Addigng classFactory for resource release Remove ConcurrencyTests project and related files from the solution
AllDwarf
force-pushed
the
users/marekaldorf/Separating_sarif_sdk_from_codebase
branch
from
ecb42e4 to
f3d6d21
Compare
