[build] Allow cross-compiling build-script products for non-Darwin hosts too #36917

finagolfin · 2021-04-14T21:12:41Z

To that end, move the --cross-compile-deps-path flag from build-script-impl to a publicly documented build-script flag and use it for build-script products' library dependencies too. Generate a SPM destination JSON file that can be used both for cross-compiling these build-script products and by users for their own Swift packages.

Also, add a native_toolchain_path() method, that uses a prebuilt toolchain if available, and pass an install prefix in to swift-driver and sourcekit-lsp.

I use this patch to cross-compile SPM from linux to Android, along with the since-merged #33724 and swiftlang/swift-package-manager#3403.

@shahmishal, since you added support for cross-compiling SPM from macOS x86_64 to arm64, maybe you could review this? I kept this separate from your macOS config, but one additional change that could be made is to rename the SPM bootstrap script's --cross-compile-hosts flag to --cross-compile-host and only ever pass in one host at a time. That'll work because I only pass in one cross-compilation host at a time for these non-Darwin hosts, and you only support cross-compiling for a single Mac host, ie macOS arm64.

~~Once this in,~~ I ~~will look into extending~~have also extended it to cross-compile sourcekit-lsp, ~~which I have never cross-compiled~~.

utils/swift_build_support/swift_build_support/products/product.py

finagolfin · 2021-04-14T21:24:40Z

utils/swift_build_support/swift_build_support/products/swiftpm.py

-            for cross_compile_host in self.args.cross_compile_hosts:
-                helper_cmd += [cross_compile_host]
+            if host_target == self.args.host_target:
+                for cross_compile_host in self.args.cross_compile_hosts:


No need for a loop here if we change the SPM bootstrap flag to --cross-compile-host.

finagolfin · 2021-06-24T04:10:36Z

Updated this pull to generate a SPM configuration file to cross-compile for Android, which matches the latest version of my pull to cross-compile SPM with itself, swiftlang/swift-package-manager#3403. I'm able to use these two pulls with the latest official trunk snapshot build of the Swift toolchain to cross-compile SPM for Android.

@shahmishal, can you review or recommend someone who might?

utils/swift_build_support/swift_build_support/products/swiftpm.py

utils/swift_build_support/swift_build_support/products/product.py

finagolfin · 2021-07-01T16:12:25Z

Rebased and ran this through the Python linter, then tweaked it to pass that.

I used to merely dislike Python and its mandatory indentation, now I hate it and its linter: it's a cruel joke.

@varungandhi-apple, would you run the CI on this?

varungandhi-apple · 2021-07-01T17:04:46Z

@swift-ci test

swift-ci · 2021-07-01T18:43:53Z

Build failed
Swift Test OS X Platform
Git Sha - 2ca0c7c95c160cb0071da14e87f839c4806ac02c

finagolfin · 2021-07-02T02:16:23Z

Mac CI failure is unrelated, Win CI appears to have been broken then.

finagolfin · 2021-07-02T12:05:12Z

Rebased and made an ARM change that I had missed when generating the JSON file.

finagolfin · 2021-07-05T05:55:58Z

Rebased, added support for cross-compiling swift-driver too, and added a check to use an existing SPM JSON file if it was written previously. I just submitted the required patch to use this with swift-driver in swiftlang/swift-driver#743.

utils/swift_build_support/swift_build_support/products/swiftdriver.py

artemcm · 2021-07-07T16:10:46Z

@swift-ci test

finagolfin · 2021-07-08T19:17:16Z

Rebased and added a new commit with the prefix flag for swift-driver. I also made two changes that will make this pull work in more scenarios. There are three primary scenarios it needs to handle:

Obviously everything should keep working for native compilation, this is tested by the CI.
You could build the native compiler from scratch then build a cross-compiled host toolchain with this pull, this is ~~untested by me or any CI, since I'm only adding this capability for non-Darwin hosts~~ now tested with the July 24 trunk source snapshot and works fine.
Use a prebuilt official snapshot toolchain to cross-compile these build-script products: that's how I initially tested this pull with the July 7 trunk snapshot.

Since 2. wasn't tested, I had made some mistakes in that scenario, which I call out in comments below.

utils/swift_build_support/swift_build_support/products/product.py

utils/swift_build_support/swift_build_support/products/swiftpm.py

finagolfin · 2021-07-09T03:23:34Z

@drexin, since you're up, let's see if this still passes the CI.

drexin · 2021-07-09T04:05:36Z

@swift-ci test

finagolfin · 2021-10-19T08:23:57Z

@neonichu, since this pull primarily adds a way to cross-compile SPM for non-Darwin platforms, mind reviewing? We should probably also run it through the CI again since it's been more than a month since it passed the CI and was ready for review, I've gone ahead and rebased.

neonichu · 2021-10-19T17:27:17Z

@swift-ci please smoke test

neonichu · 2021-10-19T17:29:00Z

@buttaface I'm not really familiar with any of the build scripts in the Swift repo, so not the right person to review.

finagolfin · 2021-10-27T19:51:26Z

@gottesmm, would you review?

finagolfin · 2021-11-29T18:02:38Z

Rebased and added support for cross-compiling sourcekit-lsp too, as I said I would when I opened this pull more than seven months ago. I used this pull and the sourcekit-lsp change to successfully cross-compile the last trunk source snapshot of the toolchain before the break for Android.

finagolfin · 2021-12-02T16:25:35Z

@shahmishal, can we get this in before the 5.6 branch? It's been ready for more than two months now and @gottesmm told me he'd like to get it in, but he was busy a month ago.

shahmishal · 2021-12-03T18:04:50Z

@edymtt Can you help review this change?

shahmishal · 2021-12-03T18:05:02Z

@swift-ci test

finagolfin · 2021-12-04T06:17:38Z

Windows CI failure is single unrelated concurrency test.

finagolfin · 2021-12-06T23:46:42Z

@edymtt, do you think we can get this in?

shahmishal · 2021-12-06T23:58:58Z

I think @gottesmm is reviewing it now.

gottesmm

Looks really good. A few quick questions. I am fine with you answering them after we merge since if the tests pass I am ok with that later explanation.

gottesmm · 2021-12-07T22:09:07Z

utils/swift_build_support/swift_build_support/build_script_invocation.py

@@ -669,7 +673,13 @@ def execute(self):
                #
                # This just maintains current behavior.
                if index > last_impl_index:
-                    self._execute(pipeline, [self.args.host_target])
+                    non_darwin_cross_compile_hostnames = [


@buttaface can you add a comment here (or fix up the comment above) explaining that we support it for non-darwin platforms. I am fine with a follow on commit fixing the comment.

I simply removed the comments above, since it is no longer true. Darwin platforms were already cross-compiling in a different way, and this change adds support for non-Darwin platforms.

gottesmm · 2021-12-07T22:09:52Z

utils/swift_build_support/swift_build_support/products/benchmarks.py

-    toolchain_path = swiftpm.SwiftPM.get_install_destdir(args,
-                                                         host_target,
-                                                         product.build_dir)
+    toolchain_path = product.host_install_destdir(host_target)


I am fine with this if the tests pass (which it looks like they did).

gottesmm · 2021-12-07T22:10:48Z

utils/swift_build_support/swift_build_support/products/indexstoredb.py

+                            resource_path=resource_dir)
+        ]
+
+    if action == 'install' and product.product_name() == "sourcekitlsp":


Shouldn't this also be guarded by the not is_darwin_host since you are trying to not effect the Darwin path?

While most of these changes only affect non-Darwin and are guarded by such checks, there are a few changes that are applied generally, ie for Darwin too, which I called out separately in my notes for the first commit and the second commit:

Also, add a native_toolchain_path() method, that uses a prebuilt toolchain if available, and pass an install prefix in to swift-driver and sourcekit-lsp.

This was needed because non-Darwin uses a different install prefix for cross-compilation hosts, so I submitted pulls for those products' build scripts to accept an install prefix or actually start using the prefix passed in, which this pull now passes in for all platforms.

These configuration changes that affect Darwin too appear to be working fine.

gottesmm · 2021-12-07T22:12:18Z

utils/swift_build_support/swift_build_support/products/swiftdriver.py

    if args.verbose_build:
        helper_cmd.append('--verbose')

+    if action == 'install':
+        helper_cmd += [
+            '--prefix', install_destdir + args.install_prefix


Same as my earlier question about --prefix. I just want to understand what the effect is.

Same answer as indexstoredb above.

…sts too To that end, move the --cross-compile-deps-path flag from build-script-impl to a publicly documented build-script flag and use it for build-script products' library dependencies too. Generate a SPM destination JSON file that can be used both for cross-compiling these build-script products and by users for their own Swift packages. Also, add a native_toolchain_path() method, that uses a prebuilt toolchain if available, and pass an install prefix in to swift-driver and sourcekit-lsp.

…e new host_install_destdir()

finagolfin · 2021-12-09T18:54:59Z

@MaxDesiatov, would you test and merge?

MaxDesiatov · 2021-12-09T18:55:42Z

@swift-ci please smoke test

MaxDesiatov · 2021-12-09T18:56:41Z

If CI passes, I hesitate to merge since there were changes requested by one of the reviews that I don't know if they were fully addressed. I'd prefer one of the reviewers or code owners to merge.

gottesmm · 2021-12-10T19:42:46Z

@DougGregor haven't they since built an ok toolchain? Can you remove your block?

gottesmm · 2021-12-11T21:02:48Z

I looked above and it seems that Doug's issue was dealt with explicitly here: #36917 (comment).

I am going to override the change request just so we can get this moving.

Responded to and dealt with here: #36917 (comment)

finagolfin · 2021-12-11T21:10:32Z

Thanks, this is the largest patch I apply for the upcoming Swift 5.6 and I apply it on my Android CI to cross-compile SPM for every trunk snapshot.

finagolfin commented Apr 14, 2021

View reviewed changes

utils/swift_build_support/swift_build_support/products/product.py Outdated Show resolved Hide resolved

finagolfin commented Apr 14, 2021

View reviewed changes

finagolfin mentioned this pull request Jun 22, 2021

Android: add cross-compilation configuration file flag swiftlang/swift-package-manager#3403

Merged

finagolfin force-pushed the swiftpm branch from 490fdbd to 83a0f00 Compare June 24, 2021 04:05

finagolfin commented Jun 24, 2021

View reviewed changes

utils/swift_build_support/swift_build_support/products/swiftpm.py Outdated Show resolved Hide resolved

finagolfin commented Jun 24, 2021

View reviewed changes

utils/swift_build_support/swift_build_support/products/product.py Outdated Show resolved Hide resolved

finagolfin force-pushed the swiftpm branch 2 times, most recently from ad6a515 to af829b1 Compare June 25, 2021 02:11

finagolfin force-pushed the swiftpm branch from af829b1 to 2ca0c7c Compare July 1, 2021 16:00

finagolfin force-pushed the swiftpm branch from 2ca0c7c to b7ef114 Compare July 2, 2021 12:04

finagolfin force-pushed the swiftpm branch from b7ef114 to c4bdcc7 Compare July 5, 2021 04:29

finagolfin mentioned this pull request Jul 5, 2021

[Build Script Helper] Add support for non-Darwin cross-compilation hosts, starting with Android swiftlang/swift-driver#743

Merged

finagolfin commented Jul 5, 2021

View reviewed changes

utils/swift_build_support/swift_build_support/products/swiftdriver.py Outdated Show resolved Hide resolved

finagolfin force-pushed the swiftpm branch from c4bdcc7 to 0f0c366 Compare July 5, 2021 13:58

finagolfin force-pushed the swiftpm branch from 0f0c366 to af1d10b Compare July 8, 2021 19:01

finagolfin commented Jul 8, 2021

View reviewed changes

utils/swift_build_support/swift_build_support/products/product.py Show resolved Hide resolved

finagolfin commented Jul 8, 2021

View reviewed changes

utils/swift_build_support/swift_build_support/products/product.py Outdated Show resolved Hide resolved

finagolfin commented Jul 8, 2021

View reviewed changes

utils/swift_build_support/swift_build_support/products/swiftpm.py Outdated Show resolved Hide resolved

finagolfin force-pushed the swiftpm branch from af1d10b to 7c991ea Compare July 9, 2021 07:06

finagolfin force-pushed the swiftpm branch from 8dda778 to e4707d1 Compare October 19, 2021 08:21

finagolfin force-pushed the swiftpm branch from e4707d1 to 5f15aa3 Compare November 29, 2021 17:55

finagolfin force-pushed the swiftpm branch from 5f15aa3 to 5abf1eb Compare November 29, 2021 18:35

shahmishal requested a review from edymtt December 3, 2021 18:06

gottesmm approved these changes Dec 7, 2021

View reviewed changes

finagolfin added 2 commits December 8, 2021 13:52

[build] Switch get_install_destdir() and other duplicated logic to th…

e9eb3b0

…e new host_install_destdir()

finagolfin force-pushed the swiftpm branch from 5abf1eb to e9eb3b0 Compare December 8, 2021 08:27

gottesmm merged commit 1ba8317 into swiftlang:main Dec 11, 2021

finagolfin deleted the swiftpm branch December 11, 2021 21:11

finagolfin mentioned this pull request Dec 12, 2021

[5.6][build] Allow cross-compiling build-script products for non-Darwin hosts too #40525

Closed

finagolfin mentioned this pull request Jul 11, 2023

build: bootstrap SwiftPM with forked and not host clang #64629

Merged

[build] Allow cross-compiling build-script products for non-Darwin hosts too #36917

[build] Allow cross-compiling build-script products for non-Darwin hosts too #36917

Conversation

finagolfin commented Apr 14, 2021 • edited Loading

finagolfin Apr 14, 2021 • edited Loading

Choose a reason for hiding this comment

finagolfin commented Jun 24, 2021

finagolfin commented Jul 1, 2021

varungandhi-apple commented Jul 1, 2021

swift-ci commented Jul 1, 2021

finagolfin commented Jul 2, 2021

finagolfin commented Jul 2, 2021

finagolfin commented Jul 5, 2021

artemcm commented Jul 7, 2021

finagolfin commented Jul 8, 2021 • edited Loading

finagolfin commented Jul 9, 2021

drexin commented Jul 9, 2021

finagolfin commented Oct 19, 2021

neonichu commented Oct 19, 2021

neonichu commented Oct 19, 2021

finagolfin commented Oct 27, 2021

finagolfin commented Nov 29, 2021

finagolfin commented Dec 2, 2021

shahmishal commented Dec 3, 2021

shahmishal commented Dec 3, 2021

finagolfin commented Dec 4, 2021

finagolfin commented Dec 6, 2021

shahmishal commented Dec 6, 2021

gottesmm left a comment

Choose a reason for hiding this comment

gottesmm Dec 7, 2021

Choose a reason for hiding this comment

finagolfin Dec 8, 2021

Choose a reason for hiding this comment

finagolfin Dec 8, 2021

Choose a reason for hiding this comment

gottesmm Dec 7, 2021

Choose a reason for hiding this comment

gottesmm Dec 7, 2021

Choose a reason for hiding this comment

finagolfin Dec 8, 2021 • edited Loading

Choose a reason for hiding this comment

gottesmm Dec 7, 2021

Choose a reason for hiding this comment

finagolfin Dec 8, 2021

Choose a reason for hiding this comment

finagolfin commented Dec 9, 2021

MaxDesiatov commented Dec 9, 2021

MaxDesiatov commented Dec 9, 2021

gottesmm commented Dec 10, 2021

gottesmm commented Dec 11, 2021

finagolfin commented Dec 11, 2021

finagolfin commented Apr 14, 2021 •

edited

Loading

finagolfin Apr 14, 2021 •

edited

Loading

finagolfin commented Jul 8, 2021 •

edited

Loading

finagolfin Dec 8, 2021 •

edited

Loading