[README] Encourage using the Swift build script #77

modocache · 2016-03-21T06:22:20Z

New contributors to swift-corelibs-xctest are having some difficulty knowing where to start:

The problem is that over time this project has begun to take on more dependencies:

To build the project, one must use a modern Swift 3 compiler (often one more recent than the latest https://swift.org snapshot).
To build the project, one must have swift-corelibs-foundation checked out at a particular path.
To run the test suite, one must have the LLVM lit test runner installed at a particular path.

In addition, the build steps differ based on whether the contributor is using an OS X or Linux development environment.

This commit revamps the README and the Linux build_script.py to require fewer steps for new contributors to get started. Specifically, it encourages contributors to use the Swift utils/build-script. There are many reasons to do so:

The XCTest CI uses that script, so contributors will need to make sure it passes to get their contributions merged.
That script ensures that the correct build of swiftc and swift-corelibs-foundation are used.
That script works across all platforms, and is invoked in the same way on each platform.

In addition, more detailed information on the project has been moved to a Documentation directory. This resembles other projects in the Swift family of repositories.

This should probably be merged after apple/swift#1748. That pull request puts the final touches on building XCTest via the Swift build script on OS X.

shahmishal · 2016-03-22T03:06:14Z

@swift-ci Please test

shahmishal · 2016-03-23T08:40:14Z

@swift-ci Please test

shahmishal · 2016-03-23T17:01:24Z

@modocache Looks like we have a new failure, so the changes you made might have helped resolve swiftc issues.

/Users/buildnode/jenkins/workspace/swift-corelibs-xctest-PR-osx/swift-corelibs-foundation/Foundation/NSXMLParser.swift:31:11: error: use of undeclared type '_CFXMLInterface'
extension _CFXMLInterface {
          ^~~~~~~~~~~~~~~
/Users/buildnode/jenkins/workspace/swift-corelibs-xctest-PR-osx/swift-corelibs-foundation/Foundation/NSXMLNode.swift:774:24: error: use of undeclared type '_CFXMLNodePtr'
    internal init(ptr: _CFXMLNodePtr) {

modocache · 2016-03-23T18:53:53Z

Yeah, I took a look at that this morning but can't tell what's wrong. The same build failure doesn't reproduce for me locally, either...

@parkera, any ideas? I could probably figure this out eventually, but help would be greatly appreciated!

parkera · 2016-03-23T19:35:09Z

Hm, I'm not sure. @shahmishal is the Foundation build passing without this change?

modocache · 2016-03-23T19:46:22Z

@parkera Just to be clear, this pull request doesn't really change anything besides documentation. The XCTest CI job for OS X has been failing since its introduction. 😅 I've been submitting pull requests to apple/swift to attempt to fix it, then running tests on this pull request to check whether it's been fixed.

I believe Foundation is built and tested via its own mechanism--it doesn't use xcodebuild like the XCTest CI job does. Maybe XCTest should do the same...?

parkera · 2016-03-23T20:02:29Z

Yah I think we should probably merge this; the failure seems unrelated.

modocache · 2016-03-23T20:10:15Z

@parkera It's true that the failure is unrelated to this change. But the documentation changes in this pull request are to encourage people to use utils/build-script --preset buildbot_incremental,tools=RA,stdlib=RA,XCTest to build the project locally. I don't want to encourage that unless the CI works with that preset. Since it's currently failing, I don't want to merge this yet.

Instead, I'd like to get to the bottom of the failure first. Once we have a single command all contributors can use to build and test the project, I'll update this pull request to recommend using that.

modocache · 2016-04-08T02:18:58Z

@briancroom @mike-ferris-apple The changes to the Swift build script have landed, and this can now be merged. I think these README instructions are much simpler for newcomers--what do you think?

briancroom · 2016-04-08T17:38:31Z

build_script.py

    build_parser.set_defaults(func=_build)
    build_parser.add_argument(
        "--swiftc",
        help="Path to the 'swiftc' compiler that will be used to build "
             "XCTest.so, XCTest.swiftmodule, and XCTest.swiftdoc. This will "
             "also be used to build the tests for those built products if the "
             "--test option is specified.",
-        metavar="PATH",


I'm curious why you chose to remove the metavar values here?

I didn't think they added any value, and I figured I'd just include them in this pull request.

This is the help text diff:

diff --git a/build_script_help_text.txt b/build_script_help_text.txt index 194df0d..e28cd3b 100755 --- a/build_script_help_text.txt +++ b/build_script_help_text.txt $ build_script.py build -h # ... optional arguments: -h, --help show this help message and exit - --swiftc PATH Path to the 'swiftc' compiler that will be used to + --swiftc SWIFTC Path to the 'swiftc' compiler that will be used to build XCTest.so, XCTest.swiftmodule, and XCTest.swiftdoc. This will also be used to build the tests for those built products if the --test option is specified. - --build-dir PATH + --build-dir BUILD_DIR Path to the output build directory. If not specified, a temporary directory is used - --foundation-build-dir PATH + --foundation-build-dir FOUNDATION_BUILD_DIR Path to swift-corelibs-foundation build products, which the built XCTest.so will be linked against. - --swift-build-dir PATH + --swift-build-dir SWIFT_BUILD_DIR deprecated, do not use --arch ARCH deprecated, do not use - --module-install-path PATH + --module-install-path MODULE_PATH Location at which to install XCTest.swiftmodule and XCTest.swiftdoc. This directory will be created if it doesn't already exist. - --library-install-path PATH + --library-install-path LIB_PATH Location at which to install XCTest.so. This directory will be created if it doesn't already exist. --release builds for release --debug builds for debug (the default) --test Whether to run tests after building. Note that you must have cloned https://github.com/apple/swift-llvm at /Users/bgesiak/GitHub/apple/llvm in order to run this command.

Personally I think it's less code without them, and removing them provides clearer help text. What do you think?

Thanks for the explanation! Looks great.

I'm pretty sure I'd trust your judgement over mine on things Python 😆 Always looking to learn.

New contributors to swift-corelibs-xctest are having some difficulty knowing where to start: - https://bugs.swift.org/browse/SR-981 - https://bugs.swift.org/browse/SR-907 - https://bugs.swift.org/browse/SR-906 The problem is that over time this project has begun to take on more dependencies: - To build the project, one must use a modern Swift 3 compiler (often one more recent than the latest https://swift.org snapshot). - To build the project, one must have swift-corelibs-foundation checked out at a particular path. - To run the test suite, one must have the LLVM `lit` test runner installed at a particular path. In addition, the build steps differ based on whether the contributor is using an OS X or Linux development environment. This commit revamps the README and the Linux `build_script.py` to require fewer steps for new contributors to get started. Specifically, it encourages contributors to use the Swift `utils/build-script`. There are many reasons to do so: - The XCTest CI uses that script, so contributors will need to make sure it passes to get their contributions merged. - That script ensures that the correct build of `swiftc` and swift-corelibs-foundation are used. - That script works across all platforms, and is invoked in the same way on each platform. In addition, more detailed information on the project has been moved to a Documentation directory. This resembles other projects in the Swift family of repositories.

briancroom · 2016-04-08T21:55:58Z

Hey @modocache, overall, I think this is a good direction for the project as we attempt to lower the barrier to entry for contributors. The writeup itself in the README is very well done 💯

The only thing that makes me hesitate is that I've personally had some rather unpleasant experiences with running the build script on my dev machine, ranging from "whelp, better find something to do for the next 2 hours" to "wait why is clang failing to build. Hmm maybe if I symlink these headers here... 😕" etc. Having said that, I realize that these issues are probably mostly tied to some quirk on my particular system, and I know that the script has a fully-supported incremental build mode which, especially in conjunction with the new build preset that you got merged (yay!), should make it reasonable to use for development.

Assuming that other folks are seeing successful builds by following these instructions, I would consider this good to merge!

(I'm hoping to get a chance to wipe my Swift dev environment sometime soon and start afresh which should theoretically straighten out my issues, and also help me understand the experience of new contributors.)

modocache · 2016-04-09T18:00:50Z

Hmm maybe if I symlink these headers here...

Yeah, that does sound strange... I think one benefit here is that there's a wider body of people running the Swift build script. I consider that important; personally I had a lot of trouble building LLVM from source a few years ago, but kept at it because I knew it couldn't possibly be broken for everyone--"it must be a problem on my end", I thought, and I tried to figure out what was wrong. I also consider the Swift build script much more user-friendly. So all-in-all, I don't think building Swift is an unreasonable prerequisite.

I think if we can get a passing OS X and Linux CI build on this pull request, we should go ahead and merge it. The script works for me locally. @mike-ferris-apple?

mike-ferris · 2016-04-10T14:41:57Z

@swift-ci please test

briancroom · 2016-04-10T16:17:06Z

Looks like more strange Foundation build issues on the OS X CI box:

=== BUILD TARGET SwiftFoundation OF PROJECT Foundation WITH CONFIGURATION Debug ===

Check dependencies
warning: using 'ALWAYS_SEARCH_USER_PATHS = YES' while building targets which define modules ('DEFINES_MODULE = YES') may fail. Please migrate to using 'ALWAYS_SEARCH_USER_PATHS = NO'.
warning: using 'ALWAYS_SEARCH_USER_PATHS = YES' while building targets which define modules ('DEFINES_MODULE = YES') may fail. Please migrate to using 'ALWAYS_SEARCH_USER_PATHS = NO'.
warning: no umbrella header found for target 'SwiftFoundation', module map will not be generated

2016-04-10 08:42:29.294 xcodebuild[22177:33024]  DVTAssertions: Warning in /Library/Caches/com.apple.xbs/Sources/DVTFrameworks/DVTFrameworks-10174/DVTFoundation/Utilities/DVTFileSystemUtilities.mm:142
Details:  lstat('/Users/buildnode/Library/Developer/Xcode/DerivedData/XCTest-dvkzigzfgybeztatzsyerimgjubd/Build/Products/Debug/SwiftFoundation.framework/Versions/A/Modules') failed with errno 2 (No such file or directory)
Function: DVTRecursiveMkdirResult dvt_recursiveMkdir_returningErrorString(NSString *__strong, NSString *__autoreleasing *)
Thread:   <NSThread: 0x7fbfcb41b820>{number = 5, name = (null)}
Please file a bug at http://bugreport.apple.com with this warning message and any useful information you can provide.
Create product structure
/bin/mkdir -p /Users/buildnode/Library/Developer/Xcode/DerivedData/XCTest-dvkzigzfgybeztatzsyerimgjubd/Build/Products/Debug/SwiftFoundation.framework/Versions/A/Modules
/bin/mkdir -p /Users/buildnode/Library/Developer/Xcode/DerivedData/XCTest-dvkzigzfgybeztatzsyerimgjubd/Build/Products/Debug/SwiftFoundation.framework
error: File is a broken symbolic link: /Users/buildnode/Library/Developer/Xcode/DerivedData/XCTest-dvkzigzfgybeztatzsyerimgjubd/Build/Products/Debug/SwiftFoundation.framework

** BUILD FAILED **


The following build commands failed:

@parkera @phausler have you seen this sort of thing before?

phausler · 2016-04-10T16:26:22Z

Looks like the failure is a build config race condition. It is expecting a product directory to exist but it has not been created yet (and the command is not recursive) I have not seen this specific failure before but I have seen similar ones in other projects.

briancroom · 2016-04-11T22:31:49Z

Now that apple/swift#2137 is in, could someone fire off CI again on this and apple/swift#2137? I have high hopes of clean builds at last! @mike-ferris-apple @shahmishal

parkera · 2016-04-11T22:35:00Z

@swift-ci please test

briancroom · 2016-04-12T00:40:21Z

Bingo! I think we've achieved our goal. Thanks for pushing us onwards @modocache.

Despite recent discussion about a minor strategy change and bringing Darwin into the build script after all, this PR is still unquestionably a step forward.

modocache · 2016-04-12T01:41:29Z

I... I can't believe the day has finally come that we have a passing OS X and Linux CI!! Thanks, @briancroom! Couldn't have done it without apple/swift#2137!

modocache mentioned this pull request Mar 22, 2016

[build-script] Link Objective-C runtime for XCTest apple/swift#1748

Merged

1 task

modocache mentioned this pull request Mar 22, 2016

[build-presets] Foundation built based on platform apple/swift#1764

Merged

1 task

This was referenced Mar 27, 2016

[XCTestObservation] Replace sharedTestObservationCenter with shared #82

Merged

[XCTestExpectation] Adding expectationForNotification #85

Merged

[build-script] XCTest CI builds on OS X apple/swift#2036

Merged

modocache force-pushed the readme-getting-started branch from 9346a64 to 3dde03e Compare April 2, 2016 18:53

briancroom reviewed Apr 8, 2016
View reviewed changes

modocache force-pushed the readme-getting-started branch from 3dde03e to db60942 Compare April 8, 2016 20:18

briancroom mentioned this pull request Apr 9, 2016

[xctest_checker] Inline expectations #94

Merged

This was referenced Apr 10, 2016

Disable implicit dependency finding and parallelized builds for Xcode schemes #95

Merged

[build-script] Use the correct build directory when building Darwin XCTest apple/swift#2137

Merged

briancroom merged commit ab3a2d9 into apple:master Apr 12, 2016

modocache deleted the readme-getting-started branch April 12, 2016 01:40

modocache mannequin mentioned this pull request Mar 23, 2016

[SR-981] Can't build swift-corelibs-xctest on OS X #401

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[README] Encourage using the Swift build script #77

[README] Encourage using the Swift build script #77

modocache commented Mar 21, 2016

shahmishal commented Mar 22, 2016

shahmishal commented Mar 23, 2016

shahmishal commented Mar 23, 2016

modocache commented Mar 23, 2016

parkera commented Mar 23, 2016

modocache commented Mar 23, 2016

parkera commented Mar 23, 2016

modocache commented Mar 23, 2016

modocache commented Apr 8, 2016

briancroom Apr 8, 2016

modocache Apr 8, 2016

briancroom Apr 11, 2016

briancroom commented Apr 8, 2016

modocache commented Apr 9, 2016

mike-ferris commented Apr 10, 2016

briancroom commented Apr 10, 2016

phausler commented Apr 10, 2016

briancroom commented Apr 11, 2016

parkera commented Apr 11, 2016

briancroom commented Apr 12, 2016

modocache commented Apr 12, 2016

[README] Encourage using the Swift build script #77

[README] Encourage using the Swift build script #77

Conversation

modocache commented Mar 21, 2016

shahmishal commented Mar 22, 2016

shahmishal commented Mar 23, 2016

shahmishal commented Mar 23, 2016

modocache commented Mar 23, 2016

parkera commented Mar 23, 2016

modocache commented Mar 23, 2016

parkera commented Mar 23, 2016

modocache commented Mar 23, 2016

modocache commented Apr 8, 2016

briancroom Apr 8, 2016

Choose a reason for hiding this comment

modocache Apr 8, 2016

Choose a reason for hiding this comment

briancroom Apr 11, 2016

Choose a reason for hiding this comment

briancroom commented Apr 8, 2016

modocache commented Apr 9, 2016

mike-ferris commented Apr 10, 2016

briancroom commented Apr 10, 2016

phausler commented Apr 10, 2016

briancroom commented Apr 11, 2016

parkera commented Apr 11, 2016

briancroom commented Apr 12, 2016

modocache commented Apr 12, 2016