Skip to content

Remove references to System.Private.Uri#7822

Merged
Forgind merged 2 commits into
dotnet:mainfrom
lbussell:sb-system.private.uri
Jul 20, 2022
Merged

Remove references to System.Private.Uri#7822
Forgind merged 2 commits into
dotnet:mainfrom
lbussell:sb-system.private.uri

Conversation

@lbussell

Copy link
Copy Markdown
Member

System.Private.Uri 6.0.0 is reported as a prebuilt in source-build due to these references. This fix is similar to dotnet/sdk#26448.

I tested this change in a source-build tarball context and verified the prebuilt package is no longer getting pulled. I also made sure msbuild builds with ./build.sh.

I noticed that these were introduced due to some CG concerns (20cdf6f), so if this change causes issues for the MSBuild build, then these System.Private.Uri and System.Runtime references can be conditioned out for source-build instead.

@Forgind

Forgind commented Jul 14, 2022

Copy link
Copy Markdown
Contributor

CG issues only show up when you try to insert MSBuild into VS, correct? Would you mind merging these changes into a branch (on dotnet/msbuild) prefixed with exp/? Then it'll automatically make a test insertion, and we can see how it does before we consider merging it into main then possibly reverting it.

@rainersigwald

Copy link
Copy Markdown
Member

Yes, let's condition. The references are useless but required, a fun position to be in!

@Forgind, CG alerts are generated in official builds on certain branches only because of configuration we don't control so there's no good way to test; an exp branch doesn't do it as I learned to my chagrin a while ago.

@Forgind Forgind left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You don't need System.Runtime either? I'm surprised, but the source build leg seems to have passed...

@lbussell

lbussell commented Jul 18, 2022

Copy link
Copy Markdown
Member Author

You don't need System.Runtime either? I'm surprised, but the source build leg seems to have passed...

The source-build leg might not have caught if this reference was needed, but I verified locally that removing both references doesn't cause any problems in a full end-to-end build. As well as gets rid of the System.Private.Uri prebuilt package.

@lbussell

Copy link
Copy Markdown
Member Author

As before, please merge when appropriate, I don't have permissions.

@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jul 20, 2022
@Forgind Forgind merged commit fe59c22 into dotnet:main Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants