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

New build system #109

Merged
merged 5 commits into from
Jan 24, 2021
Merged

New build system #109

merged 5 commits into from
Jan 24, 2021

Conversation

yury
Copy link
Contributor

@yury yury commented Jan 14, 2021

Pre:

I think Xcode builtin spm support is very sad.

  1. If you delete DerivedData you have to close Xcode and redownload all deps (with xcframeworks they are really heavy)
  2. Can't actually link to XCFramework (you have to use abs path to derived data)
  3. Really poor and only one way to edit deps via UI
  4. Updates not reliable (sometimes you have to delete DerivedData)

I propose to use Package.swift directly and reference deps XCFrameworks from .build/artifacts

I put Package swift in xcfs (read as xcframeworks) folder. And change reps in targets to xcfs/.build/artifacts/xcfs/.... so we have relative paths to XCFrameworks now.

Another improvements:

  1. I use archives for building frameworks, so we have dsyms and call safe layer for compatibility
  2. xcframeworks now have dsyms
  3. I added GitHub Actions, so we can publish sha for libs on release

Other thoughts and plans:

  1. This is work in progress, I just want to show you direction and get feedback early
  2. Pland to upgrade to openssl 1.1.1i. (I have lib compiled, but I need to update ssh_cmd)
  3. We need to drop i386 and include arm64 from simulators

@holzschu
Copy link
Owner

That sounds interesting. I agree with all the issues you listed for binary swift packages. The only way I have to force update for a specific package is to delete it and then add it again. It's really not convenient.

It would be good to invite comments by other users of these packages.

@yury
Copy link
Contributor Author

yury commented Jan 15, 2021

I forget to add, that to build all xcframeworks you can run swift run --package-path xcfs build from the root of the project.

@yury
Copy link
Contributor Author

yury commented Jan 18, 2021

👋
I upgraded ssh-keygen to openssl 1.1.1i (ref #110).
Enabled arm64 simulator for all frameworks and also fix curl_ios_static.

@yury yury marked this pull request as ready for review January 18, 2021 12:43
@holzschu
Copy link
Owner

Thank you very much. That PR will be extremely useful and valuable. I'll review it tonight.

Copy link

@kkebo kkebo left a comment

Choose a reason for hiding this comment

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

I'm sorry if this is none of my business, but I'm concerned about a few things.

try cd(".build") {
let zip = "\(scheme).xcframework.zip"
try sh("zip -r \(zip) \(scheme).xcframework")
let chksum = try sha(path: zip)
Copy link

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I will monitor that change. I don't know why this swift package compute-checksum requires manifest to do it's job.

Copy link
Owner

Choose a reason for hiding this comment

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

Hi,
the line let chksum = try sha(path: zip) does not work with swift 5 (the error message says: Illegal instruction).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@holzschu
Local or GitHub actions? How I can repro that?

Copy link
Owner

Choose a reason for hiding this comment

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

I have only tested local actions. I assume it affects all package building using Fmake. To reproduce: clone this repository, type swift run --package-path xcfs build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Local is fine, see logs:
https://gist.github.com/yury/5cfb388d69bfaf271c3c8ee39824cfbc

yury@mbpm1 ~ % swift --version
swift-driver version: 1.26.9 Apple Swift version 5.5 (swiftlang-1300.0.31.1 clang-1300.0.29.1)
Target: x86_64-apple-macosx12.0

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for testing. I'm puzzled: what could be causing the Illegal instruction error? I'll keep searching.

zip --symlinks -r ios_system.xcframework.zip ios_system.xcframework
Swift/ErrorType.swift:200: Fatal error: Error raised at top level: FMake.BuildError.unexpectedStatusCode
./createXcFrameworks.sh: line 2: 51765 Illegal instruction: 4  swift run --package-path xcfs build


_ = Package(
name: "deps",
platforms: [.macOS("11")],
Copy link

Choose a reason for hiding this comment

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

Is it okay to drop support for macOS Catalina or earlier as a build environment?

let args = ProcessInfo.processInfo.arguments

var schemes: [String]
if args.count > 1 && args[1] != "all" {
Copy link

Choose a reason for hiding this comment

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

I recommend to use swift-argument-parser.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You recommend to replace 3 lines of code with another 3 lines of code + 1mb of code dependency....

I don't think it is worth it for now.

Copy link

@kkebo kkebo Jan 23, 2021

Choose a reason for hiding this comment

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

You are right for now, but I think it's worth replacing for readability and extensibility of code. swift-argument-parser is de facto standard to create a CLI tool in Swift now.

@holzschu holzschu merged commit 5bd05f9 into holzschu:master Jan 24, 2021
@yury
Copy link
Contributor Author

yury commented Jan 24, 2021

Cool! Can you create tag v2.7.0 for test? GitHub Actions will build all frameworks

@holzschu
Copy link
Owner

holzschu commented Jan 24, 2021

It is done.

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

3 participants