Unique names for System.Native.* exports

[jkotas]

Number of methods exported from System.Native.* shims have short generic names, like Close or Stat. It works find so far because of the shims are build as small standalone .so/.dylib; but it will have a high chance for collisions once the shims get statically linked together with a bunch of other stuff in CoreRT (#4572).

The names for these exports should be made more unique (the common solution is to add disambiguating prefix).

[jkotas]

cc @tarekgh @AlexGhiondea

[stephentoub]

The approach thus far has been to name these functions as close as possible to the functions they're shimming but with .NET-esque capitalization (e.g. FStat instead of fstat). As you suggest, we could instead or in addition choose some standard prefix to use with all of them, maybe based on the library they're shimming, e.g. "libc_FStat" or something like that, or maybe including the shim name, e.g. "SystemNative_FStat".

cc: @eerhardt, @nguerrera, @sokket

[eerhardt]

See https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/interop-guidelines.md for the current naming guidance. If the guidance has now changed, we should update this document.

[stephentoub]

@eerhardt, this thread is to discuss if/how the guidance should change due to new requirements.

[eerhardt]

OK - I'll update my comment then. :)

See https://github.com/dotnet/corefx/blob/master/Documentation/coding-guidelines/interop-guidelines.md for the current naming guidance. If the guidance has now changed changes, we should update this document.

[stephentoub]

Agreed :)

[eerhardt]

If I got to vote on which prefix, I'd vote for "SystemNative" instead of "libc". If somehow the shim changes to no longer call "libc", or doesn't call "libc" on certain platforms/scenarios, it wouldn't really make sense. But the managed code would always be calling into SystemNative - that wouldn't change. Unless you are actually calling a different method, in which case the method name would change.

Also, just a nit question - but are underscores OK? I thought folks didn't like them in method names. https://github.com/dotnet/corefx/pull/4145/files#r43070582.

[stephentoub]

Also, just a nit question - but are underscores OK?

I don't have a strong preference, as long as we're consistent. I'd be ok with using them as a separator between two logically distinct aspects of the name, e.g. the library that contains the function and the function's actual name.

[jonmill]

I wouldn't mind the SystemNative_<function> notation...I usually dislike underscores in function names but it does make it cleaner in this respect.

[nguerrera]

That's going to get long: SystemSecurityCryptographyNative_*?

[jonmill]

Sure, but that means that we only need a long name in the EntryPoint parameter of the DllImport statement, not necessarily in the calling code. It would provide a standardized and collision-free way of handling this issue, IMO

[weshaggard]

Yes they might get long but I agree that we should make these more unique. I don't mind the underscores in these names and share @stephentoub concern that I mostly prefer consistency. I also prefer the name of the shim as the prefix as opposed to the library it is calling as it more closely maps to our namespace concept and will help limit the collisions.

[jkotas]

This will be a breaking change in the internal contract of System.Native.* libraries. It would help to fix this soon (e.g. for rc2) before we need to start worrying about how to avoid breaking changes in the internal contract of System.Native.* libraries.

[bartonjs]

FWIW, I agree with all the points :). (SystemNative_ instead of libc_, _ enhances legibility here, uniqueness is good, and that we want to do this for RC2).

I'd actually go a step further and say we probably want this merged by like January 14th, to give it time to shake out independent of everyone else trying to hit the last couple days of the milestone.

[eerhardt]

Assigning to myself. I'll get this taken care of this week.

[eerhardt]

Do we have to use the full native assembly name as a prefix? Or should we shorten it, so the names don't get so long (as @nguerrera pointed out)? Would it make more sense to use the same names as we do in the Interop.Libraries class?

        internal const string SystemNative = "System.Native";
        internal const string HttpNative = "System.Net.Http.Native";
        internal const string CryptoNative = "System.Security.Cryptography.Native";
        internal const string GlobalizationNative = "System.Globalization.Native";
        internal const string CompressionNative = "System.IO.Compression.Native";

So for System.Native, we still have "SystemNative_" prefix. But for Crypto, we use "CryptoNative_" instead of "SystemSecurityCryptographyNative_".

[jkotas]

But for Crypto, we use "CryptoNative_" instead of "SystemSecurityCryptographyNative_".

Sounds good to me.

[stephentoub]

Sounds good to me.

Me, too.

[bartonjs]

I feel like a Johnny-come-lately, but I also prefer CryptoNative_ to ThatReallyLongPrefix_.