[Compiler] Cygwin is identified by os(Cygwin) instead of os(Windows)

[tinysun212]

Since MinGW, Cygwin and MSVC are definitely vary greatly in ABI and C language
level behavior, it is neccessary to distinguish each other in Swift. (In target
name, MSVC is x86_64-*-windows-msvc, Cygwin is x86_64-*-windows-cygnus)

Before change, os(Windows) was for *-*-windows-*, and now,
os(Cygwin) is for *-*-windows-cygnus, and os(Windows) is for the rest.

[gribozavr]

@swift-ci Please test and merge

[jakepetroules]

Please don't pollute the meaning of "os". Cygwin is not an operating system.

Perhaps compiler() or even triple() as originally suggested in this comment?

[tinysun212]

There was a discussion about using os(Cygwin) or introducing new condition env(). I agree that Cygwin is not OS nor OS emulator, but I think using os() is practically not bad for the present.

https://lists.swift.org/pipermail/swift-dev/Week-of-Mon-20160425/001853.html
https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20160425/015852.html

[jakepetroules]

The problem with doing that "for the present" is that you break user applications if you fix it later, and then you never fix it to avoid breaking user applications, and you're stuck with it.

[tinysun212]

@jakepetroules, You have a point there. Is it will be OK if we introduce the new env() and that exactly defined by the EnvironmentType of the LLVM tripple? Or, do you have any other name and definition to suggest?

I think we need more opinions. (CC: @jckarter, @rjmccall)

[jakepetroules]

I think env() would make more sense.

On the other hand, LLVM does identify Android as an "environment" rather than an "OS" but I can understand that from a compiler/ABI perspective.

At least for Cygwin, using env() rather than os() is a lot more sensible. Otherwise you'd see #if os(Windows) || os(Cygwin) appearing all over the place unnecessarily.

[tinysun212]

I don't know how to move to swift-evolution. Is it sufficient simply sending a mail to swift-evolution@swift.org including all participants as CC?

[rjmccall]

Yes, I would recommend sending an email to swift-evolution with a subject like "#if os(Windows) and MSVC / Cygwin Compatibility". It should link to this thread and then briefly summarize it. CC'ing everybody here is a good idea, too.

For anybody to comes to this PR from swift-evolution: please keep the conceptual discussion on the mailing list.

[tinysun212]

I found that github does not expose the email address.
@jakepetroules, @ephemer, Could you give me your address to continue this discussion in swift-evolution?

[jakepetroules]

I've just put my email address on my profile. Spam filters are good enough these days. :)

[ephemer]

Mine is an @gmail.com address starting with geojay

[tinysun212]

Could anybody tell me what this test is?

ERROR: test_lldbmi_break_insert_function_pending (tools/lldb-mi/breakpoint/TestMiBreak.py)

[jckarter]

cc @egranata for the lldb error.

[egranata]

Actually, cc @tfiala - I don't maintain the build system

[tfiala]

Actually, cc @tfiala - I don't maintain the build system

@egranata, I think you mean the test system? He was asking about a test failing.

From the console output for the failed build, we see this:

======================================================================
ERROR: test_lldbmi_break_insert_function_pending (TestMiBreak.MiBreakTestCase)
   Test that 'lldb-mi --interpreter' works for pending function breakpoints.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/buildnode/jenkins/workspace/swift-PR-Linux/lldb/packages/Python/lldbsuite/test/decorators.py", line 329, in wrapper
    func(*args, **kwargs)
  File "/home/buildnode/jenkins/workspace/swift-PR-Linux/lldb/packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py", line 39, in test_lldbmi_break_insert_function_pending
    self.expect("=breakpoint-modified,bkpt={number=\"1\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"(?!0xffffffffffffffff)0x[0-9a-f]+\",func=\".+?\",file=\".+?\",fullname=\".+?\",line=\"(-1|\d+)\",pending=\[\"printf\"\],times=\"0\",original-location=\"printf\"}")
  File "/home/buildnode/jenkins/workspace/swift-PR-Linux/lldb/packages/Python/lldbsuite/test/tools/lldb-mi/lldbmi_testcase.py", line 54, in expect
    return self.child.expect(pattern, *args, **kwargs)
  File "/home/buildnode/jenkins/workspace/swift-PR-Linux/lldb/third_party/Python/module/pexpect-2.4/pexpect.py", line 1316, in expect
    return self.expect_list(compiled_pattern_list, timeout, searchwindowsize)
  File "/home/buildnode/jenkins/workspace/swift-PR-Linux/lldb/third_party/Python/module/pexpect-2.4/pexpect.py", line 1330, in expect_list
    return self.expect_loop(searcher_re(pattern_list), timeout, searchwindowsize)
  File "/home/buildnode/jenkins/workspace/swift-PR-Linux/lldb/third_party/Python/module/pexpect-2.4/pexpect.py", line 1414, in expect_loop
    raise TIMEOUT (str(e) + '\n' + str(self))
TIMEOUT: Timeout exceeded in read_nonblocking().
<pexpect.spawn object at 0x7f40c4f9f1d0>
version: 2.4 ($Revision: 516 $)
command: /home/buildnode/jenkins/workspace/swift-PR-Linux/buildbot_linux/lldb-linux-x86_64/bin/lldb-mi
args: ['/home/buildnode/jenkins/workspace/swift-PR-Linux/buildbot_linux/lldb-linux-x86_64/bin/lldb-mi', '--interpreter']
searcher: searcher_re:
    0: re.compile("=breakpoint-modified,bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="(?!0xffffffffffffffff)0x[0-9a-f]+",func=".+?",file=".+?",fullname=".+?",line="(-1|\d+)",pending=\["printf"\],times="0",original-location="printf"}")
buffer (last 100 chars): "_IO_printf",args=[],file="??",fullname="??",line="-1"},thread-id="1",stopped-threads="all"
(gdb)

before (last 100 chars): "_IO_printf",args=[],file="??",fullname="??",line="-1"},thread-id="1",stopped-threads="all"
(gdb)

after: <class 'pexpect.TIMEOUT'>
match: None
match_index: None
exitstatus: None
flag_eof: False
pid: 16528
child_fd: 5
closed: False
timeout: 30
delimiter: <class 'pexpect.EOF'>
logfile: None
logfile_read: <open file 'child.log', mode 'w' at 0x7f40c4f8e300>
logfile_send: None
maxread: 2000
ignorecase: False
searchwindowsize: None
delaybeforesend: 0.05
delayafterclose: 0.1
delayafterterminate: 0.1
Config=x86_64-/home/buildnode/jenkins/workspace/swift-PR-Linux/buildbot_linux/llvm-linux-x86_64/bin/clang-3.9
----------------------------------------------------------------------

It failed twice in the same spot, so this test is behaving as if test expectations are no longer being met.

The file lives in LLDB's repo (swift-lldb) at:
packages/Python/lldbsuite/test/tools/lldb-mi/breakpoint/TestMiBreak.py

The lldb-mi tests are actually all written up at LLVM.org LLDB. I'm not sure who is maintaining those these days, but pinging lldb-dev@llvm.org is probably a good place to start.

You can run the test by hand after doing a build that includes lldb, you'd want to do this:

{path-to-lldb-source}/test/dotest.py --executable /path/to/your/just-built/lldb --no-multiprocess -v -p TestMiBreak.py

(You can add 'python' before that if needed, and use python 2.x rather than python 3.x).

[tinysun212]

I rebased this PR. Many part was duplicated with #3886, so I removed the stdlib part.
See the hiden conversation for stdlib/public/Platform/Platform.swift. In fact, it is related to the lib/Basic/LangOptions.cpp.
I know the discussion in swift-evolution was not concluded to use this macro. But I think it is worthwhile to keep this patch.

[slavapestov]

@CodaFi What was the outcome of the swift-evolution discussion?

[CodaFi]

My impression was the opposite from @tinysun212's back in August. @lattner made an excellent point about their distinctness, and Jordan's (modulo concerns about overlap) that this wouldn't really hurt things and could always be reclassified if we discover a better taxonomy for these things in the future.

We could bring this up one more time to get fresh opinions.

[CodaFi]

Also, to answer the question on prior art: DLang considers this (and MinGW and Windows) as separate platform conditionals (environments, technically, but their static condition system doesn't make a syntactic distinction).

[CodaFi]

Subsumed by the merge of #10359

Thank you for holding out so long @tinysun212