Re: [swift-corelibs-dev] NSTask and try!

[James Lee via swift-corelibs-dev]

Cheers for the clarification. I will spend some time working through some of 
the other failures, should help me get to grips with it all :)

James

Sent from my iPhone

> On 14 May 2016, at 15:02, Bouke Haarsma via swift-corelibs-dev 
>  wrote:
> 
> On 2016-05-14 09:05:08 +, James Lee via swift-corelibs-dev said:
> 
> 
> 
> Please excuse my ignorance, I have looked into the POSIX calls, but am I 
> right in assuming that the EBADF is due to the test calling to a file that 
> doesn't exist and that is just how OSX handles this case?
> 
> 
> 
> The problem on OSX was that closing the same FD in the child process would 
> result in the EBADF. The change in the error handling posted before made the 
> error more visible. The actual fix for OSX was to make sure a single FD was 
> closed only once. See also PR #363 
> https://github.com/apple/swift-corelibs-foundation/pull/362, especially 
> https://github.com/apple/swift-corelibs-foundation/pull/363/commits/3fd0df28107847b7121f6cb2f823fbad9e2ddcff.
> 
> 
> 
> - Bouke
> 
> ___
> swift-corelibs-dev mailing list
> swift-corelibs-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-corelibs-dev
___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

Re: [swift-corelibs-dev] NSTask and try!

[Bouke Haarsma via swift-corelibs-dev]

On 2016-05-14 09:05:08 +, James Lee via swift-corelibs-dev said:

Please excuse my ignorance, I have looked into the POSIX calls, but am 
I right in assuming that the EBADF is due to the test calling to a file 
that doesn't exist and that is just how OSX handles this case?


The problem on OSX was that closing the same FD in the child process 
would result in the EBADF. The change in the error handling posted 
before made the error more visible. The actual fix for OSX was to make 
sure a single FD was closed only once. See also PR #363 
https://github.com/apple/swift-corelibs-foundation/pull/362, especially 
https://github.com/apple/swift-corelibs-foundation/pull/363/commits/3fd0df28107847b7121f6cb2f823fbad9e2ddcff. 



- Bouke___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

Re: [swift-corelibs-dev] NSTask and try!

[James Lee via swift-corelibs-dev]

This does seem to keep more inline with the current Darwin implementation. 

Please excuse my ignorance, I have looked into the POSIX calls, but am I right 
in assuming that the EBADF is due to the test calling to a file that doesn't 
exist and that is just how OSX handles this case?

Cheers for the clarification

James

Sent from my iPhone

> On 14 May 2016, at 09:33, Bouke Haarsma via swift-corelibs-dev 
>  wrote:
> 
> The failing testcase is TestNSTask.test_pipe_stdout_and_stderr_same_pipe. The 
> call to posix_spawn returns an error code 9 (EBADF). 
> 
> 
> 
> In order to avoid code repetition I've wrapped all posix calls with a 
> throwing status code check;
> 
> 
> 
> private func posix(_ code: Int32) throws {
> 
> switch code {
> 
> case 0: return
> 
> default: throw NSError(domain: NSPOSIXErrorDomain, code: Int(code), 
> userInfo: nil)
> 
> }
> 
> }
> 
> 
> 
> However this produces the not-so-helpful error dump on OSX:
> 
> 
> 
> Test Case 'TestNSTask.test_pipe_stdout_and_stderr_same_pipe' started at 
> 10:20:59.741
> 
> fatal error: 'try!' expression unexpectedly raised an error:  0x60067c40>: file 
> /Users/buildnode/jenkins/workspace/oss-swift-package-osx/swift/stdlib/public/core/ErrorType.swift,
>  line 53
> 
> 
> 
> 
> 
> 
> 
> On 2016-05-13 21:07:59 +, Tony Parker via swift-corelibs-dev said:
> 
> 
> 
> 
> 
> On May 13, 2016, at 1:05 PM, James Lee  wrote:
> 
> 
> 
> Cheers for the clarification. I started assuming there may be a reason when 
> changing the guard let on the launch args to use the InvalidArgumentException.
> 
> 
> 
> Could this be a position where we may need os checking to cover the 
> regression for the moment. It seems odd that the test would pass in CI when 
> an error is thrown with a try! but fail on OSX
> 
> Task is certainly one of the cases where the underlying stuff that we’re 
> abstracting is significantly different, so I’m not too surprised.
> 
> 
> 
> We should try to get something in place so we’re not failing on OS X in the 
> short term for sure.
> 
> 
> 
> - Tony
> 
> 
> 
> Sent from my iPhone
> 
> 
> 
> On 13 May 2016, at 20:48, Tony Parker  wrote:
> 
> 
> 
> Hi James,
> 
> 
> 
> On May 13, 2016, at 12:25 PM, James Lee via swift-corelibs-dev 
>  wrote:
> 
> 
> 
> Following on from a previous discussion with Tests failing on OSX. I have 
> been looking into the failures. It seems that one of the earliest failures is 
> due to an error from a try! within NSTask.launch(). This came in with this 
> commit: 
> https://github.com/apple/swift-corelibs-foundation/commit/4c6f04cfcad3d4b06688558021595d06751fc66a
> 
> 
> 
> Going by the docs for Foundation - The launch function apparently "Raises an 
> NSInvalidArgumentException if the launch path has not been set or is invalid 
> or if it fails to create a process."
> 
> 
> 
> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSTask_Class/#//apple_ref/occ/instm/NSTask/launch
> 
> 
> 
> My question is, should this be built into the Swift Foundation API? The 
> documentation for Swift doesn't state that the launch function throws.
> 
> 
> 
> With the test that is failing expecting an error, it feels more Swift-y to 
> have any errors throw explicitly, rather than looking at what the lower level 
> fills the data with.
> 
> 
> 
> But before jumping into doing this, I would rather put it out there and see 
> what the community feels about this?
> 
> 
> 
> Unfortunately the ‘throws’ syntax in Swift often causes a mixup between two 
> different things, because it flipped the terminology from what all of our 
> documentation and header comments use.
> 
> 
> 
> 1. Cocoa uses exceptions (@throw in ObjC) to indicate programmer errors and 
> they are generally not intended to be recoverable.  Example: passing nil 
> where not expected, passing an invalid argument, failing to meet a 
> precondition of an API.
> 
> 2. Cocoa uses NSError ** to indicate runtime errors that are recoverable or 
> at least presentable to user. Example: out of disk space, name of file 
> already exists.
> 
> 
> 
> The ‘throws’ syntax in Swift is actually for case #2, not #1. In Swift, #1 is 
> fatalError or preconditionFailure. #2 is ‘throw Error’.
> 
> 
> 
> While API compatibility should be the fore-most goal here, I feel like 
> there's room for improvement here for the API overlays. While in ObjC one has 
> the ability to recover from NSInvalidArgumentException, on Swift this would 
> trap.
> 
> 
> 
> 
> 
> In the case of NSTask, when the documentation says “raises an 
> NSInvalidArgumentException” (#1) then in Swift, that should translate to 
> fatalError or preconditionFailure.
> 
> 
> 
> As a resort; I propose to change the error wrapper (see 
> https://github.com/apple/swift-corelibs-foundation/pull/362):
> 
> 
> 
> private func posix(_ code: Int32) {
> 
> switch code {
> 

Re: [swift-corelibs-dev] NSTask and try!

[Bouke Haarsma via swift-corelibs-dev]

The failing testcase is 
TestNSTask.test_pipe_stdout_and_stderr_same_pipe. The call to 
posix_spawn returns an error code 9 (EBADF). 

In order to avoid code repetition I've wrapped all posix calls with a 
throwing status code check;


private func posix(_ code: Int32) throws {
   switch code {
   case 0: return
   default: throw NSError(domain: NSPOSIXErrorDomain, code: Int(code), 
userInfo: nil)

   }
}

However this produces the not-so-helpful error dump on OSX:

Test Case 'TestNSTask.test_pipe_stdout_and_stderr_same_pipe' started at 
10:20:59.741
fatal error: 'try!' expression unexpectedly raised an error: 0x60067c40>: file 
/Users/buildnode/jenkins/workspace/oss-swift-package-osx/swift/stdlib/public/core/ErrorType.swift, 
line 53




On 2016-05-13 21:07:59 +, Tony Parker via swift-corelibs-dev said:




On May 13, 2016, at 1:05 PM, James Lee  wrote:

Cheers for the clarification. I started assuming there may be a reason 
when changing the guard let on the launch args to use the 
InvalidArgumentException.


Could this be a position where we may need os checking to cover the 
regression for the moment. It seems odd that the test would pass in CI 
when an error is thrown with a try! but fail on OSX
Task is certainly one of the cases where the underlying stuff that 
we’re abstracting is significantly different, so I’m not too surprised.


We should try to get something in place so we’re not failing on OS X in 
the short term for sure.


- Tony


Sent from my iPhone


On 13 May 2016, at 20:48, Tony Parker  wrote:

Hi James,

On May 13, 2016, at 12:25 PM, James Lee via swift-corelibs-dev 
 wrote:


Following on from a previous discussion with Tests failing on OSX. I 
have been looking into the failures. It seems that one of the earliest 
failures is due to an error from a try! within NSTask.launch(). This 
came in with this commit: 
https://github.com/apple/swift-corelibs-foundation/commit/4c6f04cfcad3d4b06688558021595d06751fc66a 



Going by the docs for Foundation - The launch function apparently 
"Raises an NSInvalidArgumentException if the launch path has not been 
set or is invalid or if it fails to create a process."


https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSTask_Class/#//apple_ref/occ/instm/NSTask/launch 



My question is, should this be built into the Swift Foundation API? The 
documentation for Swift doesn't state that the launch function throws.


With the test that is failing expecting an error, it feels more Swift-y 
to have any errors throw explicitly, rather than looking at what the 
lower level fills the data with.


But before jumping into doing this, I would rather put it out there and 
see what the community feels about this?


Unfortunately the ‘throws’ syntax in Swift often causes a mixup between 
two different things, because it flipped the terminology from what all 
of our documentation and header comments use.


1. Cocoa uses exceptions (@throw in ObjC) to indicate programmer errors 
and they are generally not intended to be recoverable.  Example: 
passing nil where not expected, passing an invalid argument, failing to 
meet a precondition of an API.
2. Cocoa uses NSError ** to indicate runtime errors that are 
recoverable or at least presentable to user. Example: out of disk 
space, name of file already exists.


The ‘throws’ syntax in Swift is actually for case #2, not #1. In Swift, 
#1 is fatalError or preconditionFailure. #2 is ‘throw Error’.


While API compatibility should be the fore-most goal here, I feel like 
there's room for improvement here for the API overlays. While in ObjC 
one has the ability to recover from NSInvalidArgumentException, on 
Swift this would trap.




In the case of NSTask, when the documentation says “raises an 
NSInvalidArgumentException” (#1) then in Swift, that should translate 
to fatalError or preconditionFailure.


As a resort; I propose to change the error wrapper (see 
https://github.com/apple/swift-corelibs-foundation/pull/362):


private func posix(_ code: Int32) {
   switch code {
   case 0: return
   case EBADF: fatalError("POSIX command failed with error: \(code) -- EBADF")
   default: fatalError("POSIX command failed with error: \(code)")
   }
}

Which would produce the following –more helpful– error on OSX:

Test Case 'TestNSTask.test_pipe_stdout_and_stderr_same_pipe' started at 
10:13:55.584
fatal error: POSIX command failed with error: 9 -- EBADF: file 
/swift-corelibs-foundation/Foundation/NSTask.swift, line 424





Hope this helps,
- Tony


Cheers

James
___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev






___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

Re: [swift-corelibs-dev] NSTask and try!

[Tony Parker via swift-corelibs-dev]

> On May 13, 2016, at 1:05 PM, James Lee  wrote:
> 
> Cheers for the clarification. I started assuming there may be a reason when 
> changing the guard let on the launch args to use the InvalidArgumentException.
> 
> Could this be a position where we may need os checking to cover the 
> regression for the moment. It seems odd that the test would pass in CI when 
> an error is thrown with a try! but fail on OSX
> 

Task is certainly one of the cases where the underlying stuff that we’re 
abstracting is significantly different, so I’m not too surprised.

We should try to get something in place so we’re not failing on OS X in the 
short term for sure.

- Tony

> Sent from my iPhone
> 
>> On 13 May 2016, at 20:48, Tony Parker  wrote:
>> 
>> Hi James,
>> 
>>> On May 13, 2016, at 12:25 PM, James Lee via swift-corelibs-dev 
>>>  wrote:
>>> 
>>> Following on from a previous discussion with Tests failing on OSX. I have 
>>> been looking into the failures. It seems that one of the earliest failures 
>>> is due to an error from a try! within NSTask.launch(). This came in with 
>>> this commit: 
>>> https://github.com/apple/swift-corelibs-foundation/commit/4c6f04cfcad3d4b06688558021595d06751fc66a
>>> 
>>> Going by the docs for Foundation - The launch function apparently "Raises 
>>> an NSInvalidArgumentException if the launch path has not been set or is 
>>> invalid or if it fails to create a process."
>>> 
>>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSTask_Class/#//apple_ref/occ/instm/NSTask/launch
>>> 
>>> My question is, should this be built into the Swift Foundation API? The 
>>> documentation for Swift doesn't state that the launch function throws.
>>> 
>>> With the test that is failing expecting an error, it feels more Swift-y to 
>>> have any errors throw explicitly, rather than looking at what the lower 
>>> level fills the data with.
>>> 
>>> But before jumping into doing this, I would rather put it out there and see 
>>> what the community feels about this?
>> 
>> Unfortunately the ‘throws’ syntax in Swift often causes a mixup between two 
>> different things, because it flipped the terminology from what all of our 
>> documentation and header comments use.
>> 
>> 1. Cocoa uses exceptions (@throw in ObjC) to indicate programmer errors and 
>> they are generally not intended to be recoverable.  Example: passing nil 
>> where not expected, passing an invalid argument, failing to meet a 
>> precondition of an API.
>> 2. Cocoa uses NSError ** to indicate runtime errors that are recoverable or 
>> at least presentable to user. Example: out of disk space, name of file 
>> already exists.
>> 
>> The ‘throws’ syntax in Swift is actually for case #2, not #1. In Swift, #1 
>> is fatalError or preconditionFailure. #2 is ‘throw Error’.
>> 
>> In the case of NSTask, when the documentation says “raises an 
>> NSInvalidArgumentException” (#1) then in Swift, that should translate to 
>> fatalError or preconditionFailure.
>> 
>> Hope this helps,
>> - Tony
>> 
>>> Cheers
>>> 
>>> James
>>> ___
>>> swift-corelibs-dev mailing list
>>> swift-corelibs-dev@swift.org
>>> https://lists.swift.org/mailman/listinfo/swift-corelibs-dev
>> 
> 

___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

Re: [swift-corelibs-dev] NSTask and try!

[James Lee via swift-corelibs-dev]

Cheers for the clarification. I started assuming there may be a reason when 
changing the guard let on the launch args to use the InvalidArgumentException.

Could this be a position where we may need os checking to cover the regression 
for the moment. It seems odd that the test would pass in CI when an error is 
thrown with a try! but fail on OSX

Sent from my iPhone

> On 13 May 2016, at 20:48, Tony Parker  wrote:
> 
> Hi James,
> 
>> On May 13, 2016, at 12:25 PM, James Lee via swift-corelibs-dev 
>>  wrote:
>> 
>> Following on from a previous discussion with Tests failing on OSX. I have 
>> been looking into the failures. It seems that one of the earliest failures 
>> is due to an error from a try! within NSTask.launch(). This came in with 
>> this commit: 
>> https://github.com/apple/swift-corelibs-foundation/commit/4c6f04cfcad3d4b06688558021595d06751fc66a
>> 
>> Going by the docs for Foundation - The launch function apparently "Raises an 
>> NSInvalidArgumentException if the launch path has not been set or is invalid 
>> or if it fails to create a process."
>> 
>> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSTask_Class/#//apple_ref/occ/instm/NSTask/launch
>> 
>> My question is, should this be built into the Swift Foundation API? The 
>> documentation for Swift doesn't state that the launch function throws.
>> 
>> With the test that is failing expecting an error, it feels more Swift-y to 
>> have any errors throw explicitly, rather than looking at what the lower 
>> level fills the data with.
>> 
>> But before jumping into doing this, I would rather put it out there and see 
>> what the community feels about this?
> 
> Unfortunately the ‘throws’ syntax in Swift often causes a mixup between two 
> different things, because it flipped the terminology from what all of our 
> documentation and header comments use.
> 
> 1. Cocoa uses exceptions (@throw in ObjC) to indicate programmer errors and 
> they are generally not intended to be recoverable.  Example: passing nil 
> where not expected, passing an invalid argument, failing to meet a 
> precondition of an API.
> 2. Cocoa uses NSError ** to indicate runtime errors that are recoverable or 
> at least presentable to user. Example: out of disk space, name of file 
> already exists.
> 
> The ‘throws’ syntax in Swift is actually for case #2, not #1. In Swift, #1 is 
> fatalError or preconditionFailure. #2 is ‘throw Error’.
> 
> In the case of NSTask, when the documentation says “raises an 
> NSInvalidArgumentException” (#1) then in Swift, that should translate to 
> fatalError or preconditionFailure.
> 
> Hope this helps,
> - Tony
> 
>> Cheers
>> 
>> James
>> ___
>> swift-corelibs-dev mailing list
>> swift-corelibs-dev@swift.org
>> https://lists.swift.org/mailman/listinfo/swift-corelibs-dev
> 

___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

Re: [swift-corelibs-dev] NSTask and try!

[Tony Parker via swift-corelibs-dev]

Hi James,

> On May 13, 2016, at 12:25 PM, James Lee via swift-corelibs-dev 
>  wrote:
> 
> Following on from a previous discussion with Tests failing on OSX. I have 
> been looking into the failures. It seems that one of the earliest failures is 
> due to an error from a try! within NSTask.launch(). This came in with this 
> commit: 
> https://github.com/apple/swift-corelibs-foundation/commit/4c6f04cfcad3d4b06688558021595d06751fc66a
> 
> Going by the docs for Foundation - The launch function apparently "Raises an 
> NSInvalidArgumentException if the launch path has not been set or is invalid 
> or if it fails to create a process."
> 
> https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSTask_Class/#//apple_ref/occ/instm/NSTask/launch
> 
> My question is, should this be built into the Swift Foundation API? The 
> documentation for Swift doesn't state that the launch function throws.
> 
> With the test that is failing expecting an error, it feels more Swift-y to 
> have any errors throw explicitly, rather than looking at what the lower level 
> fills the data with.
> 
> But before jumping into doing this, I would rather put it out there and see 
> what the community feels about this?
> 

Unfortunately the ‘throws’ syntax in Swift often causes a mixup between two 
different things, because it flipped the terminology from what all of our 
documentation and header comments use.

1. Cocoa uses exceptions (@throw in ObjC) to indicate programmer errors and 
they are generally not intended to be recoverable.  Example: passing nil where 
not expected, passing an invalid argument, failing to meet a precondition of an 
API.
2. Cocoa uses NSError ** to indicate runtime errors that are recoverable or at 
least presentable to user. Example: out of disk space, name of file already 
exists.

The ‘throws’ syntax in Swift is actually for case #2, not #1. In Swift, #1 is 
fatalError or preconditionFailure. #2 is ‘throw Error’.

In the case of NSTask, when the documentation says “raises an 
NSInvalidArgumentException” (#1) then in Swift, that should translate to 
fatalError or preconditionFailure.

Hope this helps,
- Tony

> Cheers
> 
> James
> ___
> swift-corelibs-dev mailing list
> swift-corelibs-dev@swift.org
> https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev

[swift-corelibs-dev] NSTask and try!

[James Lee via swift-corelibs-dev]

Following on from a previous discussion with Tests failing on OSX. I 
have been looking into the failures. It seems that one of the earliest 
failures is due to an error from a try! within NSTask.launch(). This 
came in with this commit: 
https://github.com/apple/swift-corelibs-foundation/commit/4c6f04cfcad3d4b06688558021595d06751fc66a


Going by the docs for Foundation - The launch function apparently 
"Raises an NSInvalidArgumentException if the launch path has not been 
set or is invalid or if it fails to create a process."


https://developer.apple.com/library/mac/documentation/Cocoa/Reference/Foundation/Classes/NSTask_Class/#//apple_ref/occ/instm/NSTask/launch

My question is, should this be built into the Swift Foundation API? The 
documentation for Swift doesn't state that the launch function throws.


With the test that is failing expecting an error, it feels more Swift-y 
to have any errors throw explicitly, rather than looking at what the 
lower level fills the data with.


But before jumping into doing this, I would rather put it out there and 
see what the community feels about this?


Cheers

James
___
swift-corelibs-dev mailing list
swift-corelibs-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-corelibs-dev