Skip to content

Conversation

@VSadov
Copy link
Member

@VSadov VSadov commented Dec 7, 2020

Superhost PR follow-up

Re: #44505 (comment)

For the most part a search/replace change.

@VSadov VSadov force-pushed the clrcompressionRename branch from 3ca8f5c to cd39845 Compare December 8, 2020 03:49
@VSadov VSadov force-pushed the clrcompressionRename branch from d04a11d to ea583de Compare December 8, 2020 04:40
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Unnecessary edit

Copy link
Member Author

Choose a reason for hiding this comment

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

VS did something that I cannot see or undo in the editor. I will revert the whole file in git.

@CoffeeFlux
Copy link
Contributor

Mono changes LGTM. Thanks!

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks

@VSadov
Copy link
Member Author

VSadov commented Dec 8, 2020

Thanks!!!

@VSadov VSadov merged commit 99f3f55 into dotnet:master Dec 8, 2020
@VSadov VSadov deleted the clrcompressionRename branch December 8, 2020 23:50
@ghost ghost locked as resolved and limited conversation to collaborators Jan 8, 2021
@ericstj
Copy link
Member

ericstj commented Jan 15, 2021

FYI: This name change broke an application : #47029. IMHO that app is doing a bad thing, hopefully we'll fix that app.

It's not completely unlikely that others might make a similar mistake. Any reason you chose the System.* convention here?

@VSadov
Copy link
Member Author

VSadov commented Jan 15, 2021

Any reason you chose the System.* convention here?

It was already called System.IO.Compression.Native on every platform except Windows.

@ericstj
Copy link
Member

ericstj commented Jan 15, 2021

I think we give it a "lib" prefix on those other platforms.

@VSadov
Copy link
Member Author

VSadov commented Jan 15, 2021

Yes, there is a convention about prefix (lib on Unix, none on Windows) and extensions (.dll, .so, .dylib). They were applied according to platform.

There is also prior art in the naming of System.Globalization.Native when it was a standalone dll.

Actually it still has that name for the purpose of PInvoke targets. We redirect these pinvokes to the implementation linked into coreclr, but a standalone System.Globalization.Native library can be reintroduced quite transparently.
On mac it would be libSystem.Globalization.Native.dylib, System.Globalization.Native.dll on Windows

@ericstj
Copy link
Member

ericstj commented Jan 15, 2021

System.Globalization.Native.dll on Windows

I don't think we ever shipped that. System.IO.Compression.Native.dll is the first native binary to use the convention System.*.dll on windows.

@VSadov
Copy link
Member Author

VSadov commented Jan 15, 2021

We have a lot "System.*.Native." on Unix. The question of "lib" prefix did come up in PR discussions, but it appeared more consistent to not have it on Windows: #45713 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants