Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Xcode Project Generation #174

Merged
merged 5 commits into from Mar 10, 2016
Merged

Xcode Project Generation #174

merged 5 commits into from Mar 10, 2016

Conversation

mxcl
Copy link
Contributor

@mxcl mxcl commented Mar 5, 2016

No description provided.

@paulofaria
Copy link
Contributor

The command will be swift dump?

@mxcl
Copy link
Contributor Author

mxcl commented Mar 5, 2016

The command will be swift dump?

No, I imagine it will be swift build --generate-xcodeproj or something.

@ddunbar thanks will address these.

@ddunbar
Copy link
Member

ddunbar commented Mar 5, 2016

Overall this seems like a great start and I'm sure will be immediately useful to people.

We should probably also update the README.md to explain how to start developing with it?

@mxcl
Copy link
Contributor Author

mxcl commented Mar 5, 2016

We should probably also update the README.md to explain how to start developing with it?

Indeed, will amend. Expect a few more patches before this is merged.

buildSettings["LD_RUNPATH_SEARCH_PATHS"] = "'@loader_path/../Frameworks'"
} else {
//FIXME Xcode fails to set this for some reason
buildSettings["LD_RUNPATH_SEARCH_PATHS"] = "/Users/mxcl/Library/Developer/Toolchains/swift-DEVELOPMENT-SNAPSHOT-2016-03-01-a.xctoolchain/usr/lib/swift/macosx"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incidentally @ddunbar, did this not cause you problems? And if it didn't cause you problems, did the tests still pass? I added it temporarily because it seemed to be needed. Obviously needs to at least be the local toolchain.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This caused swift-build to fail to run with error

dyld: Library not loaded: @rpath/libswiftSwiftOnoneSupport.dylib

I got this working by making the following change

if let buildToolPath = try? POSIX.popen(["xcrun", "--find", "swift-build-tool"]),
   let dylibsPath = try? Path.join(buildToolPath, "../../lib/swift/macosx").abspath() {
    buildSettings["LD_RUNPATH_SEARCH_PATHS"] = dylibsPath
} else {
    //FIXME: handle this properly
    fatalError("Unable to get path to toolChain")
}      

@mxcl
Copy link
Contributor Author

mxcl commented Mar 8, 2016

Fresh review please, no nit-picking, let's merge this.

@mxcl
Copy link
Contributor Author

mxcl commented Mar 8, 2016

@swift-ci Please test

@mxcl
Copy link
Contributor Author

mxcl commented Mar 8, 2016

Will squash history and merge later today unless there are objections.

@ankitspd
Copy link
Member

ankitspd commented Mar 9, 2016

Looks good, lets merge asap 💃

mxcl added 5 commits March 9, 2016 09:39
This global state was making everything less testable, in the end I realized mostly it was there for ManifestParser, so now the paths to that are injected.

A bonus is that Get no longer depends on ManifestParser which is a more restricted dependency diagram and makes the module more testable too.

I also removed the code that figures out the Darwin path to the executable using dlsym. Perhaps this is foolish, but `Process.arguments.first!` seems as good and doesn’t have an initialization order problem, and since XCTest exposes no way to statically initialize global state, I don’t want to deal with it.

This refactor exposed bad test boundaries for TransmuteTests and GetTests that must be fixed due to the hacks I had to add to the test dependency calculator in the Transmute module.

——————

Though I am not so happy with the injected manifestParser() function in Get. It feels dirty to pass it around to every object, but thus is the pattern.
We create dynamic libraries for modules since there is no concept in Xcode
itself of a target that has no binary product (SwiftPM compiles the modules
but the eventual binary products are declared separately, so two modules
can be linked into the same binary target).

There are things left to do, see TODO.md.
@mxcl
Copy link
Contributor Author

mxcl commented Mar 9, 2016

@swift-ci Please test


let srcroot = package.path
let nontests = modules.filter{ !($0 is TestModule) }
let tests = modules.filter{ $0 is TestModule }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just being cheeky, but aren't these supposed to have spaces before the closures? 😏

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually yes, unless you wrap the closure around params.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swift: the two year old language where apparently we already have extremely firm guidelines for whitespace.

@mxcl
Copy link
Contributor Author

mxcl commented Mar 9, 2016

tap tap tap… come on CI.

mxcl added a commit that referenced this pull request Mar 10, 2016
Xcode Project Generation
@mxcl mxcl merged commit 5040f9e into apple:master Mar 10, 2016
@mxcl mxcl deleted the xcodeproj branch March 10, 2016 00:04
@ankitspd
Copy link
Member

😍😍👍👍

@ankitspd
Copy link
Member

Hmm, looks like xcrun --find swiftc is giving the path to default toolchain instead of the development one causing swift-build to fail.

PS: I have exported the toolchain to PATH
$ export PATH=/Library/Developer/Toolchains/swift-latest.xctoolchain/usr/bin:"${PATH}"

@ankitspd
Copy link
Member

which swiftc gives correct path

@mxcl
Copy link
Contributor Author

mxcl commented Mar 10, 2016

At this point I'm not exactly sure yet what to do here.

xcrun probably is doing the right thing.

You should set TOOLCHAINS as documented in the README (since yesterday). This is really the proper way to have the active toolchain set for swift-build to use.

I think maybe the solution is to just use swiftc unless TOOLCHAINS is set, but I don't know how sensible that is for the imminent future where swift build is bundled with OS X properly.

@ankitspd
Copy link
Member

I think it needs the 7.3 beta for TOOLCHAINS to work, can't confirm right now as on slow internet can't download 4GB 😔

@mxcl
Copy link
Contributor Author

mxcl commented Mar 10, 2016

This is all a hack anyway since Xcode shouldn't need to be told which swift-* to use, it is already configured with a toolchain. I'd like to push some kind of workaround tonight. I'd hate for something that doesn't work to go into the next snapshot.

@mxcl
Copy link
Contributor Author

mxcl commented Mar 10, 2016

Actually this will work if it's in a toolchain since it looks next to itself first.

So this only affects people trying to use a self built copy of swift-build against a previously published toolchain who cannot set TOOLCHAINS, so I hate to say it, but I think we are working as intended.

@mxcl
Copy link
Contributor Author

mxcl commented Mar 10, 2016

A workaround for now is:

  1. Build the new swift-build
  2. Overwrite the latest toolchain copy with the one you built

@bhargavg
Copy link
Collaborator

As a temporary work around, I got things running by exporting SWIFT_EXEC as:

export SWIFT_EXEC=/Library/Developer/Toolchains/swift-latest.xctoolchain/usr/bin/swiftc

Best part, it is even setting LD_RUNPATH_SEARCH_PATHS correctly (#174 (comment))

I'm still on Xcode - 7.2 btw

@kostiakoval
Copy link
Collaborator

I've tried many options and I still get en error.

  1. Build it. I used swiftc and sbt from master
    Utilities/bootstrap --swiftc test /Users/kkoval/Work/apple/build/Ninja-ReleaseAssert/swift-macosx-x86_64/bin/swiftc --sbt /Users/kkoval/Work/apple/build/Ninja-ReleaseAssert/llbuild-macosx-x86_64/bin/swift-build-tool
  2. set both SWIFT_EXEC and TOOLCHAINS environment vars

I still get this error.

.build/.bootstrap/bin/swift-build
LLVM ERROR: Program used external function 
'__TIFC18PackageDescription7PackagecFT4nameGSqSS_7targetsGSaCS_6Target_12dependenciesGSaCS0_10Dependency_16testDependenciesGSaS2__7excludeGSaSS__S0_A1_' 
which could not be resolved!
error: exit(1): /Library/Developer/Toolchains/swift-latest.xctoolchain/usr/bin/swiftc --driver-mode=swift 
-I /Users/kkoval/Work/apple/swiftpm/.build/.bootstrap/lib/swift/pm 
-L /Users/kkoval/Work/apple/swiftpm/.build/.bootstrap/lib/swift/pm 
-lPackageDescription -target x86_64-apple-macosx10.10 /Users/kkoval/Work/apple/swiftpm/Package.swift -fileno 3

Here is log from the ``Utilities/bootstrapLooks like it's using newerMacOSX10.11`

Link /Users/kkoval/Work/apple/swiftpm/.build/.bootstrap/bin/swift-build
bootstrap: note: building self-hosted 'swift-build': env SWIFT_EXEC=/Users/kkoval/Work/apple/build/Ninja-ReleaseAssert/swift-macosx-x86_64/bin/swiftc
SWIFT_BUILD_TOOL=/Users/kkoval/Work/apple/build/Ninja-ReleaseAssert/llbuild-macosx-x86_64/bin/swift-build-tool 
SWIFT_BUILD_PATH=/Users/kkoval/Work/apple/swiftpm/.build
SYSROOT=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/   
MacOSX10.11.sdk /Users/kkoval/Work/apple/swiftpm/.build/.bootstrap/bin/swift-build

I use latest Xcode 7.3beta.
Setting export TOOLCHAINS=swift has no effect, when I run xcrun --find swift it always points to the Xcode one.

@mxcl
Copy link
Contributor Author

mxcl commented Mar 10, 2016

@kostiakoval you need to clean the build for swiftpm, this is a separate issue caused by the swift-3-api-guidelines merge. Though I do not know what happened there, it broke our incremental build. Which is worrying.

@mxcl
Copy link
Contributor Author

mxcl commented Mar 10, 2016

However this whole system is not robust, we should open a ticket to fix it:

  1. Do it properly, probably the main logic is close
  2. Remove the global Toolchain struct and inject all use of paths
  3. Ensure we test for the final install layout in our tests, currently we figure out the paths to swiftc and swift-build-tool but this isn't properly testing for our final install layout so potentially there's a horrible bug lurking
  4. Allow command line flags to override the toolchain prefix (consider how this combines with the supported SWIFT_EXEC)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants