[Shepherd] Treat Cygwin as a separate OS #10359
Conversation
|
@swift-ci please smoke test |
CodaFi
changed the title
|
Thanks to re-up os(Cygwin). I thought that my past PR is dying. Many POSIX-compliant systems return the operating system name with the POSIX function POSIX-compliant applications that run on Cygwin should recognize its OS as 'Cygwin' without any special logic. If this is merged, the macro definition used in Glibc port and Foundation port can be replaced in the cleaner way. swiftpm port (swiftlang/swift-package-manager#733) also can be reopened. |
|
The base patch is also incomplete. There are numerous places where we guard on |
|
@tinysun212 I've re-arranged the platform conditionals throughout the standard library and stdlib unit test to align with this patch. Can you please run the unit tests on a Cygwin build and see if I've accidentally regressed something? |
| @@ -157,6 +158,8 @@ std::pair<bool, bool> LangOptions::setTarget(llvm::Triple triple) { | |||
| addPlatformConditionValue(PlatformConditionKind::OS, "FreeBSD"); | |||
| else if (triple.isOSWindows()) | |||
| addPlatformConditionValue(PlatformConditionKind::OS, "Windows"); | |||
| else if (triple.isWindowsCygwinEnvironment()) | |||
| addPlatformConditionValue(PlatformConditionKind::OS, "Cygwin"); | |||
There was a problem hiding this comment.
On Cygwin, triple.isOSWindows() and triple.isWindowsCygwinEnvironment() are all true.
The condition order should be changed. After changed, os(Windows) will be false and only os(Cygwin) will be true.
stdlib/public/core/Policy.swift
Outdated
| @@ -104,7 +104,7 @@ public typealias StringLiteralType = String | |||
| // IEEE Binary64, and we need 1 bit to represent the sign. Instead of using | |||
| // 1025, we use the next round number -- 2048. | |||
| public typealias _MaxBuiltinIntegerType = Builtin.Int2048 | |||
| #if (!os(Windows) || CYGWIN) && (arch(i386) || arch(x86_64)) | |||
| #if os(Cygwin) && (arch(i386) || arch(x86_64)) | |||
There was a problem hiding this comment.
Just drop CYGWIN.
#if !os(Windows) && (arch(i386) || arch(x86_64))
Linux i386/x86_64, Cygwin i386/x86_64 and macOS x86_64 have Builtin.FPIEEE80.
Windows i386/x86_64 and other CPUs have Builtin.FPIEEE64.
stdlib/public/core/CTypes.swift
Outdated
| @@ -43,14 +43,14 @@ public typealias CShort = Int16 | |||
| public typealias CInt = Int32 | |||
|
|
|||
| /// The C 'long' type. | |||
| #if os(Windows) && !CYGWIN && arch(x86_64) | |||
| #if os(Windows) && !os(Cygwin) && arch(x86_64) | |||
There was a problem hiding this comment.
Old os(Windows) && !CYGWIN is equivalent to new os(Windows).
Just drop && !CYGWIN.
Change to #if os(Windows) && arch(x86_64)
stdlib/public/core/CTypes.swift
Outdated
| public typealias CLong = Int32 | ||
| #else | ||
| public typealias CLong = Int | ||
| #endif | ||
|
|
||
| /// The C 'long long' type. | ||
| #if os(Windows) && !CYGWIN && arch(x86_64) | ||
| #if os(Windows) && !os(Cygwin) && arch(x86_64) |
|
That's all. |
| // posix_spawn is not available on Android or Windows. | ||
| #if os(Windows) | ||
| // posix_spawn is not available on Windows. | ||
| #else |
There was a problem hiding this comment.
Better to collapse these two conditionals, I think. There's nothing particularly special about Windows or Android.
|
@swift-ci please smoke test and merge |
|
@swift-ci please smoke test |
Cygwin is considered a distinct target with a distinct ABI, environment conditions, and data types. Though the goal of the project is native Windows integration with UNIX-likes, that is not compatible with the idea that the platform can be ignored as Win-like enough to have the existing os(Windows) condition apply.
|
@swift-ci please smoke test and merge |
|
I'm very glad as if my old PR was passed. |
To re-up the discussion that terminated with Chris believing that this kind of thing could only help, not hurt, I've rebased #2351.
// cc @tinysun212