Skip to content

Port System.Security.Cryptography.X509Certificates to release/6.0#9097

Merged
jonfortescue merged 2 commits intodotnet:release/6.0from
jonfortescue:Cg60
May 3, 2022
Merged

Port System.Security.Cryptography.X509Certificates to release/6.0#9097
jonfortescue merged 2 commits intodotnet:release/6.0from
jonfortescue:Cg60

Conversation

@jonfortescue
Copy link
Copy Markdown
Contributor

Port #9064 to release/6.0.

To double check:

@jonfortescue jonfortescue requested review from a user, MattGal and ulisesh April 19, 2022 20:27
@MattGal
Copy link
Copy Markdown
Member

MattGal commented Apr 19, 2022

@ulisesh has been looking at this in parallel, you should synch as what's in main may not be sufficient yet.

@ulisesh
Copy link
Copy Markdown
Contributor

ulisesh commented Apr 19, 2022

The changes for System.Security.Cryptography.X509Certificates weren't enough and we shouldn't port them

MattGal
MattGal previously approved these changes Apr 21, 2022
* Force import of the "right" version so we always restore the version we ask for.

* Need to ignore for .proj since these don't have TFMs/restore correctly.
@jonfortescue
Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@jonfortescue
Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@ulisesh
Copy link
Copy Markdown
Contributor

ulisesh commented May 2, 2022

What's next for this PR @jonfortescue?

@jonfortescue
Copy link
Copy Markdown
Contributor Author

@ulisesh I think this can be merged as is since I am going off FR and we know it resolves the issue it's supposed to.

@ulisesh
Copy link
Copy Markdown
Contributor

ulisesh commented May 2, 2022

@jonfortescue can you please hand it off to someone in the FR rotation?

@jonfortescue
Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@AlitzelMendez
Copy link
Copy Markdown
Member

AlitzelMendez commented May 3, 2022

@ulisesh is something missing on this pr? or is ready to be approved/merged?
or could you please let us know your findings about why this work is not sufficient?

Copy link
Copy Markdown
Contributor

@ulisesh ulisesh left a comment

Choose a reason for hiding this comment

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

LGTM

@jonfortescue jonfortescue reopened this May 3, 2022
@jonfortescue jonfortescue enabled auto-merge (squash) May 3, 2022 18:54
@jonfortescue jonfortescue merged commit d464309 into dotnet:release/6.0 May 3, 2022
@jonfortescue jonfortescue deleted the Cg60 branch May 10, 2022 17:38
@ericstj
Copy link
Copy Markdown
Member

ericstj commented May 16, 2022

This change should be undone. It's causing issues downstream as it's introducing package dependencies in places where they aren't compatible.

See https://dev.azure.com/dnceng/public/_build/results?buildId=1770224&view=logs&j=108d2c4a-8a62-5a58-8dad-8e1042acc93c&t=f1671469-0c48-5b47-9674-85d49bdb5829&l=59

The fix for CG flagging this package is to address the reference higher in the stack, and only in the projects that cause the problem.

@MattGal
Copy link
Copy Markdown
Member

MattGal commented May 16, 2022

This change should be undone. It's causing issues downstream as it's introducing package dependencies in places where they aren't compatible.

See https://dev.azure.com/dnceng/public/_build/results?buildId=1770224&view=logs&j=108d2c4a-8a62-5a58-8dad-8e1042acc93c&t=f1671469-0c48-5b47-9674-85d49bdb5829&l=59

The fix for CG flagging this package is to address the reference higher in the stack, and only in the projects that cause the problem.

I'll get this taken care of today and make sure you're on the PR.

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.

5 participants