Fixed `POSIXError.Code` for Linux #2113

colemancda · 2019-04-14T19:13:15Z

Right now POSIXError is broken on Linux, with Foundation defining POSIXError.Code that is valid for Darwin, not Linux.

CodaFi · 2019-04-14T19:42:12Z

What's the policy on API breakage in Corelibs at this point @parkera @millenomi? I mean, I doubt there's very much code that is e.g. switching over every single error code.

CodaFi · 2019-04-14T19:42:16Z

@swift-ci please test

colemancda · 2019-04-14T19:47:35Z

What's the policy on API breakage in Corelibs at this point @parkera @millenomi? I mean, I doubt there's very much code that is e.g. switching over every single error code.

All that code should still work, the new struct type is functionally the same as the enum only that init(rawValue: Int) is no longer synthesized by the compiler. So any existing switch statements or code that uses this type would continue to work the same was when it was an enum, the only exception would be when using Swift Reflection capabilities the type will be an struct and not enum anymore.

CodaFi · 2019-04-14T19:50:11Z

This will break:

func f(_ x : POSIXError.Code) {
  switch x {
  case .EPERM: //
  /*
  And so on
  */ 
  case .EQFULL: //
  }
}

We will now detect this as a non-exhaustive switch (all the enum patterns are now expression patterns).

colemancda · 2019-04-14T19:52:52Z

This will break:
func f(_ x : POSIXError.Code) {
  switch x {
  case .EPERM: //
  /*
  And so on
  */ 
  case .EQFULL: //
  }
}
We will now detect this as a non-exhaustive switch (all the enum patterns are now expression patterns).

That is correct, but that code was invalid in the first place. This project is only used on non-Darwin platforms, and to my knowledge a non-exhaustive switch statement would be invalid anyway because this has been broken in Linux since 2016, not to mention different Linux kernel versions and other OS like BSD and Android would have different error codes entirely.

CodaFi · 2019-04-14T20:28:27Z

Hm, looks like this patch isn't quite rebased enough to build yet.

colemancda · 2019-04-14T21:01:48Z

The error is that POSIXErrorCode is undefined on Linux, despite Glibc being imported.

/home/buildnode/jenkins/workspace/swift-corelibs-foundation-PR-Linux@2/swift-corelibs-foundation/Foundation/NSError.swift:910:33: error: use of undeclared type 'POSIXErrorCode'
15:20:51         internal let errorCode: POSIXErrorCode
15:20:51                                 ^~~~~~~~~~~~~~

I don't see how that's possible since it is defined as part of the Glibc module apple/swift@8157194#diff-c86969eead9d9f63ad0faa3e75802e66R268.

As far as I can tell, POSIXErrorCode is still present https://github.com/apple/swift/blob/master/stdlib/public/Platform/POSIXError.swift#L271

Should not use `POSIXError.Code` on Linux since it is currently broken. apple/swift-corelibs-foundation#2113

Use `POSIXErrorCode` to validate `errno` value. apple/swift-corelibs-foundation#2113

colemancda · 2019-04-14T21:51:48Z

POSIXError.Code is just a typealias on Darwin, so I changed the implementation to reflect that.

https://github.com/apple/swift/blob/577e04d35be3dba4954bd99eac5ef2af264c60b3/stdlib/public/Darwin/Foundation/NSError.swift#L2450

colemancda · 2019-04-14T22:35:53Z

Depends on apple/swift#24028 for Glibc.POSIXErrorCode to be a valid identifier

Since it is not compiled with Glibc yet https://bugs.swift.org/browse/SR-10485 apple/swift#24028 apple/swift-corelibs-foundation#2113

colemancda · 2019-04-14T22:46:50Z

@CodaFi Please test again. Fixed any usage of POSIXError.Code with switch statements.

spevans · 2019-04-14T23:07:16Z

@swift-ci test

colemancda · 2019-04-15T02:48:40Z

@spevans For some reason seems the CI was never triggered.

apple/swift#24028 apple/swift-corelibs-foundation#2113

spevans · 2019-04-15T06:58:35Z

@swift-ci test

colemancda · 2019-04-15T08:11:52Z

@spevans Fixed invalid error code for Linux in 7998141
Please test again

spevans · 2019-04-15T08:23:30Z

@swift-ci test

colemancda · 2019-04-15T14:43:03Z

@spevans For some reason seems the CI was never triggered, again.

spevans · 2019-04-15T14:43:51Z

@swift-ci test

colemancda · 2019-04-15T15:58:30Z

@spevans The CI seems to be broken or just not responding.
@compnerd @drodriguez Any ideas on these CI issues?

colemancda · 2019-04-15T15:59:23Z

Stuck at

drodriguez · 2019-04-15T18:08:00Z

We don’t have more visibility than you to the CI machines (except the Android/Windows ones). You can look in https://ci.swift.org/job/swift-corelibs-foundation-PR-Linux/ to see if 2113 is in the left list. The last one I see is https://ci.swift.org/job/swift-corelibs-foundation-PR-Linux/2503/. Since apple/swift#24028 haven’t been merge, I think the “please test” invocations have to say so, so the CI is started with the right set of PRs.

compnerd · 2019-04-15T18:47:46Z

@swift-ci please test

Foundation/NSError.swift

colemancda · 2019-04-16T05:51:34Z

@compnerd To provide proper Windows support, we would need POSIXErrorCode defined in ucrt , similar to how Darwin and Glibc define POSIXErrorCode. Unfortunately that needs to be done in the compiler project, not here. Any thoughts?

colemancda · 2019-04-16T07:13:03Z

@compnerd Added POSIXErrorCode for Windows to support Foundation on Windows.

apple/swift#24056

pushkarnk · 2019-04-16T08:22:16Z

@swift-ci test

colemancda · 2019-04-16T14:52:12Z

I didn't break this

04:03:10 <unknown>:0: warning: missing submodule 'CoreFoundation'
04:03:10 
/home/buildnode/jenkins/workspace/swift-corelibs-foundation-PR-Linux/swift-corelibs-xctest/Sources/XCTest/Public/XCTestMain.swift:20:23: error: missing required module 'CoreFoundation'
04:03:10     @_exported import Foundation
04:03:10

compnerd · 2019-04-16T16:12:32Z

@swift-ci please test Linux platform

colemancda · 2019-04-16T19:56:27Z

@compnerd CI passed, ready to merge.

CodaFi · 2019-04-16T20:14:16Z

I still want confirmation from somebody at Apple that this is ready to go.

Ping @parkera @millenomi

compnerd · 2019-04-16T21:20:01Z

This seems reasonable to me, but @millenomi may have opinions on this.

colemancda · 2019-04-17T13:06:55Z

For reference, POSIXError.Code was implemented as a duplicate enum previously, this PR removed that to match how the Foundation Swift overlay for Darwin is implemented, which just defines Foundation.POSIXError.Code as a typealias for Darwin.POSIXErrorCode.

https://github.com/apple/swift/blob/master/stdlib/public/Darwin/Foundation/NSError.swift#L2450

colemancda · 2019-04-17T17:44:52Z

@millenomi @parkera ?

millenomi · 2019-04-17T17:51:16Z

Thank you for the ping — lots of traffic have pushed this a lot lower than I hoped.

millenomi · 2019-04-17T18:03:52Z

It's good.

compnerd · 2019-04-17T19:13:46Z

This broke the windows build!

colemancda · 2019-04-17T19:15:42Z

I would be happy to assist you in fixing it, but it shouldn't break, please take a look at apple/swift#24056 as long as that is compiling correctly, POSIXErrorCode should be defined for Darwin, Linux and Windows.

Are you compiling the compiler again before Foundation, or in separate steps? This will only work for a compiler built after apple/swift#24056

compnerd · 2019-04-17T19:18:21Z

@colemancda - that change is merged and is available, but, Im still seeing a ton of undefined values (e.g. ETOOMANYREFS)

compnerd · 2019-04-17T19:18:55Z

In fact, it has hit the CI too: https://dev.azure.com/compnerd/windows-swift/_build/results?buildId=1431

colemancda · 2019-04-17T19:21:16Z

Ok, I am going to introduce a patch to fix that, I know where the issue is.

colemancda · 2019-04-17T19:31:35Z

Windows build fixed in #2130

shahmishal · 2019-04-17T21:27:27Z

We are seeing a failure on swift-5.1-branch due to this change:

https://ci.swift.org/view/swift-5.1-branch/job/oss-swift-5.1-incremental-RA-linux-ubuntu-18_04/475/

/home/buildnode/jenkins/workspace/oss-swift-5.1-incremental-RA-linux-ubuntu-18_04/swift-corelibs-foundation/Foundation/NSError.swift:919:29: error: use of undeclared type 'POSIXErrorCode'
01:25:06.442     public typealias Code = POSIXErrorCode
01:25:06.442                             ^~~~~~~~~~~~~~
01:25:06.442 /home/buildnode/jenkins/workspace/oss-swift-5.1-incremental-RA-linux-ubuntu-18_04/swift-corelibs-foundation/Foundation/NSError.swift:922:11: error: use of undeclared type 'POSIXErrorCode'
01:25:06.442 extension POSIXErrorCode: _ErrorCodeProtocol {
01:25:06.442           ^~~~~~~~~~~~~~
01:25:06.442 /home/buildnode/jenkins/workspace/oss-swift-5.1-incremental-RA-linux-ubuntu-18_04/swift-corelibs-foundation/Foundation/NSError.swift:908:15: error: type 'POSIXError' does not conform to protocol '_BridgedStoredNSError'
01:25:06.442 public struct POSIXError : _BridgedStoredNSError {
01:25:06.442               ^
01:25:06.442 /home/buildnode/jenkins/workspace/oss-swift-5.1-incremental-RA-linux-ubuntu-18_04/swift-corelibs-foundation/Foundation/NSError.swift:486:20: note: protocol requires nested type 'Code'; do you want to add it?
01:25:06.442     associatedtype Code: _ErrorCodeProtocol
01:25:06.442                    ^
01:25:06.442 /home/buildnode/jenkins/workspace/oss-swift-5.1-incremental-RA-linux-ubuntu-18_04/swift-corelibs-foundation/Foundation/NSError.swift:352:31: warning: conditional cast from 'Self' to 'NSError' always succeeds
01:25:06.442         if let nsError = self as? NSError {
01:25:06.442                               ^
01:25:06.442 ninja: build stopped: subcommand failed.

cc/ @colemancda @millenomi @compnerd

shahmishal · 2019-04-17T21:27:57Z

This is blocking PR testing on swift-5.1-branch of apple/swift

colemancda · 2019-04-17T21:29:23Z

Incremental builds are not going to work, this PR requires the Swift compiler to be built with apple/swift#24028 included. It seems the failure is due to building with a version of Swift that does not include that fix.

colemancda · 2019-04-17T21:30:02Z

@shahmishal Please see apple/swift#24028

shahmishal · 2019-04-17T21:34:12Z

@colemancda has apple/swift#24028 already been cherry-picked into swift-5.1-branch?

colemancda · 2019-04-17T21:34:23Z

@shahmishal So either the compiler's swift-5.1-branch need to be updated with latest master commits, or just that single commit is needs to be merged apple/swift@787bc36

shahmishal · 2019-04-17T21:34:48Z

if not, please create a PR for swift-5.1-branch.

colemancda · 2019-04-17T21:43:43Z

Ok

colemancda mentioned this pull request Apr 14, 2019

Make POSIXError use POSIXErrorCode from Swift Stdlib #933

Closed

colemancda added a commit to PureSwift/Bluetooth that referenced this pull request Apr 14, 2019

Fixed POSIXError for Linux

7a9dacd

Should not use `POSIXError.Code` on Linux since it is currently broken. apple/swift-corelibs-foundation#2113

colemancda added a commit to PureSwift/Bluetooth that referenced this pull request Apr 14, 2019

Fixed POSIXError for Linux

4aeba6d

Use `POSIXErrorCode` to validate `errno` value. apple/swift-corelibs-foundation#2113

colemancda mentioned this pull request Apr 14, 2019

Fixed POSIXError for Linux PureSwift/Bluetooth#122

Merged

colemancda added a commit to PureSwift/Bluetooth that referenced this pull request Apr 14, 2019

Added missing POSIXErrorCode

bb89fb1

Since it is not compiled with Glibc yet https://bugs.swift.org/browse/SR-10485 apple/swift#24028 apple/swift-corelibs-foundation#2113

colemancda added a commit to PureSwift/BluetoothLinux that referenced this pull request Apr 15, 2019

Fixed POSIXError for Linux

66466d7

apple/swift#24028 apple/swift-corelibs-foundation#2113

compnerd reviewed Apr 16, 2019

View reviewed changes

Foundation/NSError.swift Outdated Show resolved Hide resolved

CodaFi assigned millenomi and parkera Apr 16, 2019

millenomi merged commit d840648 into apple:master Apr 17, 2019

colemancda deleted the posixErrorFix branch April 17, 2019 18:06

colemancda mentioned this pull request Apr 17, 2019

[5.1] Cherry-pick: [stdlib] Include POSIXError as part of Glibc apple/swift#24108

Merged

colemancda mentioned this pull request Apr 14, 2019

[SR-10485] Use of unresolved identifier 'POSIXErrorCode' on Linux apple/swift#52885

Closed

Fixed POSIXError.Code for Linux #2113

Fixed POSIXError.Code for Linux #2113

Conversation

colemancda commented Apr 14, 2019 • edited

CodaFi commented Apr 14, 2019

CodaFi commented Apr 14, 2019

colemancda commented Apr 14, 2019

CodaFi commented Apr 14, 2019 • edited

colemancda commented Apr 14, 2019 • edited

CodaFi commented Apr 14, 2019

colemancda commented Apr 14, 2019 • edited

colemancda commented Apr 14, 2019

colemancda commented Apr 14, 2019

colemancda commented Apr 14, 2019

spevans commented Apr 14, 2019

colemancda commented Apr 15, 2019

spevans commented Apr 15, 2019

colemancda commented Apr 15, 2019

spevans commented Apr 15, 2019

colemancda commented Apr 15, 2019

spevans commented Apr 15, 2019

colemancda commented Apr 15, 2019

colemancda commented Apr 15, 2019

drodriguez commented Apr 15, 2019

compnerd commented Apr 15, 2019

colemancda commented Apr 16, 2019 • edited

colemancda commented Apr 16, 2019

pushkarnk commented Apr 16, 2019

colemancda commented Apr 16, 2019

compnerd commented Apr 16, 2019

colemancda commented Apr 16, 2019

CodaFi commented Apr 16, 2019

compnerd commented Apr 16, 2019

colemancda commented Apr 17, 2019 • edited

colemancda commented Apr 17, 2019

millenomi commented Apr 17, 2019

millenomi commented Apr 17, 2019

compnerd commented Apr 17, 2019

colemancda commented Apr 17, 2019 • edited

compnerd commented Apr 17, 2019

compnerd commented Apr 17, 2019

colemancda commented Apr 17, 2019

colemancda commented Apr 17, 2019

shahmishal commented Apr 17, 2019

shahmishal commented Apr 17, 2019

colemancda commented Apr 17, 2019 • edited

colemancda commented Apr 17, 2019

shahmishal commented Apr 17, 2019

colemancda commented Apr 17, 2019

shahmishal commented Apr 17, 2019

colemancda commented Apr 17, 2019

Fixed `POSIXError.Code` for Linux #2113

Fixed `POSIXError.Code` for Linux #2113

colemancda commented Apr 14, 2019 •

edited

CodaFi commented Apr 14, 2019 •

edited

colemancda commented Apr 14, 2019 •

edited

colemancda commented Apr 14, 2019 •

edited

colemancda commented Apr 16, 2019 •

edited

colemancda commented Apr 17, 2019 •

edited

colemancda commented Apr 17, 2019 •

edited

colemancda commented Apr 17, 2019 •

edited