Lowering System.Collections.Immutable to 1.5.0 #2533

eddynaka · 2022-08-15T21:47:19Z

This change will drop to a previous version of System.Collections.Immutable that we used to use (1.5.0).
We did this update nearly two years ago: #2146 but we didn't have any reason to do it.

Approach:

Removed the dependency from the test projects since the tests can use the binaries from the actual product
Lowered the dependency from the product
Ran tests

Tests:

All tests are passing
BinSkim tests are passing
Sarif.PatternMatcher tests are passing

eddynaka · 2022-08-15T22:54:30Z

src/ReleaseHistory.md

 ## **v3.0.0** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/3.0.0) | [Driver](https://www.nuget.org/packages/Sarif.Driver/3.0.0) | [Converters](https://www.nuget.org/packages/Sarif.Converters/3.0.0) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/3.0.0) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/3.0.0)
 * BUGFIX: Loosen Newtonsoft.JSON minimum version requirement to 6.0.8 (for .NET framework) or 9.0.1 (for all other compilations) for Sarif.Sdk. Sarif.Converts requires 8.0.1, minimally, for .NET framework compilations.
 * BUGFIX: Broaden set of supported .NET frameworks for compatibility reasons. Sarif.Sdk, Sarif.Driver and Sarif.WorkItems requires net461.
-* BUGFIX: Set default stack limit in Newtonsoft.JSON utilization (if `JsonConvert.Defaults` is not already configured) to address GitHub advisory [GHSA-5crp-9r3c-p9vr](https://github.com/advisories/GHSA-5crp-9r3c-p9vr).


BUGFIX: Set default stack limit in Newtonsoft.JSON utilization

Removing this because we are not setting any limit anymore. #Closed

We're no longer setting a limit? Which PR was this removed in and why?

This got removed in this:
#2527

We discussed today in the IcMs meeting about this. The point is: the consume will need to fix and the sarif.sdk, since its the lower level, it cannot enforce the fix on anyone.

Got it. Thanks for the context.

marmegh · 2022-08-15T23:35:50Z

src/ReleaseHistory.md

@@ -1,9 +1,12 @@
 # SARIF Package Release History (SDK, Driver, Converters, and Multitool)

+## **v3.1.0-beta1** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/3.1.0-beta1) | [Driver](https://www.nuget.org/packages/Sarif.Driver/3.1.0-beta1) | [Converters](https://www.nuget.org/packages/Sarif.Converters/3.1.0-beta1) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/3.1.0-beta1) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/3.1.0-beta1)
+
+* DEPENDENCY BREAKING: SARIF.SDK now requires `System.Collections.Immutable` 1.5.0. [#2504](https://github.com/microsoft/sarif-sdk/pull/2533)


Is there a compatibility issue between 1.5.0 and 5.0.0? If 5.0.0 was previously required, is this really a breaking change or are we actually loosening restrictions for the sarif sdk? #Resolved

we have other systems that uses sarif-sdk which is causing major breaking changes in other projects.

So, with that in mind, I want to reduce to the previous version that we were using.

If it is considered breaking, we need to increment the major version (4.0.0).

I choose 3.1.0 because its a minor change, with backwards compatibility. No API changes and everything should work as is.

I would only choose for a major bump if: we are changing the targetframework or if we are breaking existing APIs.

marmegh · 2022-08-15T23:37:36Z

Can you explain in the description the reason for downgrading this dependency? Is it for VS compatibility like the newtonsoft downgrade? Is there an external motivation for this? Or are we simply applying a principle of lowest required version for all dependencies?

eddynaka · 2022-08-16T00:03:33Z

src/Test.FunctionalTests.Sarif/Test.FunctionalTests.Sarif.csproj

@@ -49,7 +49,6 @@
    <PackageReference Include="FluentAssertions" Version="5.10.2" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.4.0-preview-20220707-01" />
    <PackageReference Include="Moq" Version="4.13.1" />
-    <PackageReference Include="System.Collections.Immutable" Version="5.0.0" />


System.Collections.Immutable

I'm removing the dependency from the tests, since it will use the same version of the product. This will facilitate new updates in the future if required.

marmegh

EasyRhinoMSFT · 2022-08-15T23:54:43Z

src/ReleaseHistory.md

@@ -1,9 +1,12 @@
 # SARIF Package Release History (SDK, Driver, Converters, and Multitool)

+## **v3.1.0-beta1** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/3.1.0-beta1) | [Driver](https://www.nuget.org/packages/Sarif.Driver/3.1.0-beta1) | [Converters](https://www.nuget.org/packages/Sarif.Converters/3.1.0-beta1) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/3.1.0-beta1) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/3.1.0-beta1)
+
+* DEPENDENCY BREAKING: SARIF.SDK now requires `System.Collections.Immutable` 1.5.0. [#2504](https://github.com/microsoft/sarif-sdk/pull/2533)


If it is considered breaking, we need to increment the major version (4.0.0).

eddynaka added 2 commits August 15, 2022 14:45

Lowering System.Collections.Immutable to 1.5.0

f375f73

Updating build.props and ReleaseHistory

b7cc0cd

eddynaka commented Aug 15, 2022

View reviewed changes

eddynaka marked this pull request as ready for review August 15, 2022 23:10

eddynaka requested review from michaelcfanning, EasyRhinoMSFT, marmegh and cfaucon as code owners August 15, 2022 23:10

releasing a beta1 version

f237cfa

marmegh reviewed Aug 15, 2022

View reviewed changes

eddynaka commented Aug 16, 2022

View reviewed changes

marmegh approved these changes Aug 16, 2022

View reviewed changes

EasyRhinoMSFT approved these changes Aug 16, 2022

View reviewed changes

michaelcfanning merged commit 5988503 into main Aug 16, 2022

michaelcfanning deleted the users/ednakamu/lowering-immutable-package-dependency branch August 16, 2022 16:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lowering System.Collections.Immutable to 1.5.0 #2533

Lowering System.Collections.Immutable to 1.5.0 #2533

eddynaka commented Aug 15, 2022 •

edited

Loading

eddynaka Aug 15, 2022 •

edited by marmegh

Loading

marmegh Aug 15, 2022

eddynaka Aug 15, 2022

eddynaka Aug 16, 2022

marmegh Aug 16, 2022

marmegh Aug 15, 2022 •

edited by EasyRhinoMSFT

Loading

eddynaka Aug 15, 2022

EasyRhinoMSFT Aug 15, 2022

eddynaka Aug 16, 2022

marmegh commented Aug 15, 2022

eddynaka Aug 16, 2022

marmegh left a comment

EasyRhinoMSFT Aug 15, 2022

Lowering System.Collections.Immutable to 1.5.0 #2533

Lowering System.Collections.Immutable to 1.5.0 #2533

Conversation

eddynaka commented Aug 15, 2022 • edited Loading

eddynaka Aug 15, 2022 • edited by marmegh Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marmegh Aug 15, 2022 • edited by EasyRhinoMSFT Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marmegh commented Aug 15, 2022

Choose a reason for hiding this comment

marmegh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddynaka commented Aug 15, 2022 •

edited

Loading

eddynaka Aug 15, 2022 •

edited by marmegh

Loading

marmegh Aug 15, 2022 •

edited by EasyRhinoMSFT

Loading