Re: [swift-corelibs-dev] Looking at corelibs-foundation
Wanted to chirp up and say I am in the same position as David, with that said, if PR's have been rejected due to a lack of response, are there any that have not been covered elsewhere and can be picked up? James Sent from my iPhone > On 13 May 2016, at 18:01, Tony Parker via swift-corelibs-dev > wrote: > > Hi David, > >> On May 11, 2016, at 4:02 PM, David Hart via swift-corelibs-dev >> wrote: >> >> Hello people, >> >> I wanted to start giving a hand on corelibs-foundation but hit two obstacles >> I’d like to discuss: >> >> It feels difficult to know where help is needed because the ReleaseNotes, >> Status and Know Issues docs have not been updated in a very long time, as if >> abandoned. Hopefully we can update hem, but perhaps a rule should be put in >> place so that no pull request is merged without the corresponding update in >> the documentation? > > Sounds good to me. I don't want to start rejecting PRs because they miss a > bit of documentation but hopefully we can encourage it or update it after we > merge. > >> I tried downloading the master branch of corelibs-foundation and running the >> tests before starting any work, but several of them crashed or failed. I am >> on OS X, Xcode 7.3.1, up to date on the master branches of >> corelibs-foundation and corelibs-xctest and am using the latest development >> snapshot. For reference, the failing tests are: >> >> TestNSString.test_initializeWithFormat3 >> TestNSTask.test_pipe_stderr >> TestNSTask.test_pipe_stdout_and_stderr_same_pipe >> TestNSTask.test_passthrough_environment >> TestNSTask.test_no_environment >> TestNSTask.test_custom_environment >> TestNSUserDefaults.test_createUserDefaults >> TestNSUserDefaults.test_getRegisteredDefaultItem >> TestNSXMLDocument.test_xpath >> >> Any ideas? Perhaps I’m doing something wrong. > > Our CI system only builds and tests corelibs-foundation on Linux, so perhaps > some regressions have snuck in on OS X only (which is interesting if true). > > NSTask in particular has been under a lot of changes for Linux recently. > >> >> I was surprised to see fairly little tests for certain classes NSIndexPath, >> NSUserDefaults, NSScanner. Is that because the code was written before the >> Open Source project started? What are the rules on test quality and how are >> they applied? > > We’d like to see tests with all new code, for sure. Some of this was written > fairly quickly in the run up to the launch, so we probably don’t have as many > tests as we would like there. I do have a task on my plate somewhere to > figure out how we can integrate more of our internal unit tests into the open > source project to help with compatibility. > >> More generally, I feel worried at how much work is still left, especially >> with the 3.0 beta branches starting. Am I imagining things or does it not >> look very good? What can we do to rally the troops? > > I totally understand. The Foundation team itself has been focused on the > value type changes, naming changes, etc that are coming as part of Swift 3. > We haven’t had nearly as much time as I would have liked to dedicate to > bringing this project up to parity with Swift 2.2 functionality. We are still > hoping to accept as many contributions as possible. That is why I went > through and accepted a bunch of PRs last week. > > We have had a few contributions that felt like one-offs; when changes were > requested we received no response and so I had to close them, which makes me > pretty sad. I haven’t really seen any true ownership of a particular area. I > understand it’s asking a lot for someone to come in and help us implement a > pre-set API, but I believe in a bright future for this project if we can pick > up the pace a bit. > > - Tony > >> A well meaning developer, >> David. >> ___ >> 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 mailing list swift-corelibs-dev@swift.org https://lists.swift.org/mailman/listinfo/swift-corelibs-dev
[swift-corelibs-dev] NSTask and try!
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
Re: [swift-corelibs-dev] NSTask and try!
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!
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 Sw
Re: [swift-corelibs-dev] NSTask and try!
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 +0000, 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
