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

Conversation

Projects
None yet
10 participants
@mxcl
Contributor

mxcl commented Mar 5, 2016

No description provided.

@paulofaria

This comment has been minimized.

Show comment
Hide comment
@paulofaria

paulofaria Mar 5, 2016

Contributor

The command will be swift dump?

Contributor

paulofaria commented Mar 5, 2016

The command will be swift dump?

@mxcl

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 5, 2016

Contributor

The command will be swift dump?

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

@ddunbar thanks will address these.

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@ddunbar

ddunbar Mar 5, 2016

Member

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?

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

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 5, 2016

Contributor

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.

Contributor

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.

@@ -17,9 +17,9 @@ import Utility
- Throws: Error.InvalidDependencyGraph
- Returns: The modules that this manifest requires building
*/
public func get(manifest: Manifest) throws -> [Package] {
public func get(manifest: Manifest, manifestParser: (path: String, url: String) throws -> Manifest) throws -> [Package] {

This comment has been minimized.

@aciidb0mb3r

aciidb0mb3r Mar 5, 2016

Member

I think (path: String, url: String) throws -> Manifest can be typealiased as its being repeated a few times

@aciidb0mb3r

aciidb0mb3r Mar 5, 2016

Member

I think (path: String, url: String) throws -> Manifest can be typealiased as its being repeated a few times

This comment has been minimized.

@czechboy0

czechboy0 Mar 6, 2016

Contributor

typealias ManifestParser = (path: String, url: String) throws -> Manifest

@czechboy0

czechboy0 Mar 6, 2016

Contributor

typealias ManifestParser = (path: String, url: String) throws -> Manifest

This comment has been minimized.

@mxcl

mxcl Mar 7, 2016

Contributor

I prefer not to typealias closures. It just means you have to hunt out the definition rather than have the information in front of you.

There is no potential badness that can happen from not typealiasing it that I can see, if I accidentally type the wrong signature the compiler will point it out.

@mxcl

mxcl Mar 7, 2016

Contributor

I prefer not to typealias closures. It just means you have to hunt out the definition rather than have the information in front of you.

There is no potential badness that can happen from not typealiasing it that I can see, if I accidentally type the wrong signature the compiler will point it out.

This comment has been minimized.

@kostiakoval

kostiakoval Mar 7, 2016

Collaborator

Agree with Max. I've used typealias for function types in production app, didn't like it. I think it's worse also because autocompletion just five you ManifestParser instead of (path: String, url: String) throws -> Manifest

@kostiakoval

kostiakoval Mar 7, 2016

Collaborator

Agree with Max. I've used typealias for function types in production app, didn't like it. I think it's worse also because autocompletion just five you ManifestParser instead of (path: String, url: String) throws -> Manifest

env["SWIFT_EXEC"] = Path.join(bindir, "swiftc")
env["SWIFT_BUILD_TOOL"] = Path.join(bindir, "swift-build-tool")
} else {
fatalError("HURRAY! This is fixed")

This comment has been minimized.

@modocache

modocache Mar 5, 2016

Collaborator

I've never seen a fatal error exclaim "hurray" before! 😝

@modocache

modocache Mar 5, 2016

Collaborator

I've never seen a fatal error exclaim "hurray" before! 😝

This comment has been minimized.

@kostiakoval

kostiakoval Mar 5, 2016

Collaborator

😂👏

@kostiakoval

kostiakoval Mar 5, 2016

Collaborator

😂👏

This comment has been minimized.

@aciidb0mb3r

aciidb0mb3r Mar 5, 2016

Member

This is correct. 🚁

@aciidb0mb3r

aciidb0mb3r Mar 5, 2016

Member

This is correct. 🚁

This comment has been minimized.

@mxcl

mxcl Mar 7, 2016

Contributor

It seems my comment got deleted (by me, bad me). There's a bug in Xcode currently where SWIFT_EXEC is not set for tests, I have a radar open for it.

@mxcl

mxcl Mar 7, 2016

Contributor

It seems my comment got deleted (by me, bad me). There's a bug in Xcode currently where SWIFT_EXEC is not set for tests, I have a radar open for it.

}
/// the location of swiftc relative to our installation
public static let swiftc = getenv("SWIFT_EXEC") ?? Toolchain.which("swiftc")

This comment has been minimized.

@bhargavg

bhargavg Mar 8, 2016

Collaborator

This gives me wrong path for swiftc

➜ xcrun --find swiftc
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swiftc
➜ xcrun --find swift-build-tool
/Library/Developer/Toolchains/swift-latest.xctoolchain/usr/bin/swift-build-tool

Causing the error

/Users/bhargavg/Documents/workspaces/xcode/github/swift-package-manager/Package.swift:11:8: error: module file was created by a newer version of the compiler: /Users/bhargavg/Documents/workspaces/xcode/github/swift-package-manager/.build/lib/swift/pm/PackageDescription.swiftmodule

when I try to run .build/debug/swift-build --generate-xcodeproj

@bhargavg

bhargavg Mar 8, 2016

Collaborator

This gives me wrong path for swiftc

➜ xcrun --find swiftc
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swiftc
➜ xcrun --find swift-build-tool
/Library/Developer/Toolchains/swift-latest.xctoolchain/usr/bin/swift-build-tool

Causing the error

/Users/bhargavg/Documents/workspaces/xcode/github/swift-package-manager/Package.swift:11:8: error: module file was created by a newer version of the compiler: /Users/bhargavg/Documents/workspaces/xcode/github/swift-package-manager/.build/lib/swift/pm/PackageDescription.swiftmodule

when I try to run .build/debug/swift-build --generate-xcodeproj

This comment has been minimized.

@mxcl

mxcl Mar 8, 2016

Contributor

Seems like your environment is not set up correctly if xcrun is giving you the wrong swiftc. /cc @ddunbar

@mxcl

mxcl Mar 8, 2016

Contributor

Seems like your environment is not set up correctly if xcrun is giving you the wrong swiftc. /cc @ddunbar

@mxcl

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 8, 2016

Contributor

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

Contributor

mxcl commented Mar 8, 2016

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

@mxcl

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 8, 2016

Contributor

@swift-ci Please test

Contributor

mxcl commented Mar 8, 2016

@swift-ci Please test

@mxcl

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 8, 2016

Contributor

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

Contributor

mxcl commented Mar 8, 2016

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

@aciidb0mb3r

This comment has been minimized.

Show comment
Hide comment
@aciidb0mb3r

aciidb0mb3r Mar 9, 2016

Member

Looks good, lets merge asap 💃

Member

aciidb0mb3r commented Mar 9, 2016

Looks good, lets merge asap 💃

mxcl added some commits Mar 4, 2016

Generate Xcode projects
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.
Less global state (-Resources.swift)
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.
@mxcl

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 9, 2016

Contributor

@swift-ci Please test

Contributor

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 }

This comment has been minimized.

@kylesyoon

kylesyoon Mar 9, 2016

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

@kylesyoon

kylesyoon Mar 9, 2016

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

This comment has been minimized.

@mxcl

mxcl Mar 9, 2016

Contributor

No.

@mxcl

mxcl Mar 9, 2016

Contributor

No.

This comment has been minimized.

@ketzusaka

ketzusaka Mar 9, 2016

Usually yes, unless you wrap the closure around params.

@ketzusaka

ketzusaka Mar 9, 2016

Usually yes, unless you wrap the closure around params.

This comment has been minimized.

@mxcl

mxcl Mar 9, 2016

Contributor

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

@mxcl

mxcl Mar 9, 2016

Contributor

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

@mxcl

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 9, 2016

Contributor

tap tap tap… come on CI.

Contributor

mxcl commented Mar 9, 2016

tap tap tap… come on CI.

mxcl added a commit that referenced this pull request Mar 10, 2016

Merge pull request #174 from mxcl/xcodeproj
Xcode Project Generation

@mxcl mxcl merged commit 5040f9e into apple:master Mar 10, 2016

2 checks passed

Swift Test Linux Platform Build finished. 7990 tests run, 0 skipped, 0 failed.
Details
Swift Test OS X Platform Build finished. 32124 tests run, 0 skipped, 0 failed.
Details

@mxcl mxcl deleted the mxcl:xcodeproj branch Mar 10, 2016

@aciidb0mb3r

This comment has been minimized.

Show comment
Hide comment
@aciidb0mb3r

aciidb0mb3r Mar 10, 2016

Member

😍😍👍👍

Member

aciidb0mb3r commented Mar 10, 2016

😍😍👍👍

@aciidb0mb3r

This comment has been minimized.

Show comment
Hide comment
@aciidb0mb3r

aciidb0mb3r Mar 10, 2016

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}"

Member

aciidb0mb3r commented Mar 10, 2016

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}"

@aciidb0mb3r

This comment has been minimized.

Show comment
Hide comment
@aciidb0mb3r

aciidb0mb3r Mar 10, 2016

Member

which swiftc gives correct path

Member

aciidb0mb3r commented Mar 10, 2016

which swiftc gives correct path

@mxcl

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 10, 2016

Contributor

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.

Contributor

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.

@aciidb0mb3r

This comment has been minimized.

Show comment
Hide comment
@aciidb0mb3r

aciidb0mb3r Mar 10, 2016

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 😔

Member

aciidb0mb3r commented Mar 10, 2016

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

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 10, 2016

Contributor

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.

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 10, 2016

Contributor

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.

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 10, 2016

Contributor

A workaround for now is:

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

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

This comment has been minimized.

Show comment
Hide comment
@bhargavg

bhargavg Mar 10, 2016

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

Collaborator

bhargavg commented Mar 10, 2016

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

This comment has been minimized.

Show comment
Hide comment
@kostiakoval

kostiakoval Mar 10, 2016

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.

Collaborator

kostiakoval commented Mar 10, 2016

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

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 10, 2016

Contributor

@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.

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@mxcl

mxcl Mar 10, 2016

Contributor

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)
Contributor

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