Skip to content

enable static linking of the swift runtime libraries by default on supported platforms #3905

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

enable static linking of the swift runtime libraries by default on supported platforms #3905

wants to merge 5 commits into from
+195 −72

Conversation

tomerd
Copy link
Contributor

@tomerd tomerd commented Dec 3, 2021 •
edited
Loading

motivation: on some platforms (eg Linux) statically linking the swift runtime libraries makes more sense to allow better mobility of execuables

changes:

  • deprecate the opt-in "static-swift-stdlib" CLI flag
  • introduce an opt-out "disable-static-swift-runtime" CLI flag
  • change behavior to automatically build/link with "-static-stdlib" on supported platforms (Linux and WASI for now)
  • adjust tests

@tomerd tomerd changed the title enable static linking of the swift runtime libraries by default on su… enable static linking of the swift runtime libraries by default on supported platforms Dec 3, 2021
@tomerd
Copy link
Contributor Author

tomerd commented Dec 3, 2021

@neonichu @abertelrud @ktoso @weissi @compnerd @MaxDesiatov following on the discussion from https://forums.swift.org/t/pre-pitch-statically-linking-the-swift-runtime-libraries-by-default-on-linux/ this is a draft implementation. does this resonate with you? if so, I will make a formal pitch with a link to this PR as the impl

@tomerd tomerd force-pushed the feature/static-link-stdlib-linux branch 2 times, most recently from 89f8989 to cb4a28b Compare December 3, 2021 04:34
@MaxDesiatov
Copy link
Contributor

cc @kateinoigakukun

@tomerd tomerd self-assigned this Jan 13, 2022
@tomerd tomerd force-pushed the feature/static-link-stdlib-linux branch from 0e90fd4 to 45d8915 Compare February 14, 2022 19:25
tomerd
tomerd commented Feb 14, 2022
View reviewed changes
Tests/BuildTests/BuildPlanTests.swift
let testPathExtension = testBuildDescription.binary.extension
XCTAssertEqual(testPathExtension, "wasm")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MaxDesiatov one of the key pieces of feedback to this proposal was that this should only apply in release mod (since if you are trying to debug you would need to have the toolchain installed by definition).

ptal at the changes to the WASI tests: expanded them to cover both debug and release modes

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, thanks!

tomerd
tomerd commented Feb 14, 2022
View reviewed changes
Tests/BuildTests/BuildPlanTests.swift
@@ -1959,7 +1952,7 @@ final class BuildPlanTests: XCTestCase {
XCTAssertMatch(executablePathExtension, "exe")
}

func testWASITarget() throws {
func testWASITargetDebug() throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MaxDesiatov

tomerd
tomerd commented Feb 14, 2022
View reviewed changes
Tests/BuildTests/BuildPlanTests.swift
@@ -2049,6 +2041,94 @@ final class BuildPlanTests: XCTestCase {
XCTAssertEqual(testPathExtension, "wasm")
}

func testWASITargetRelease() throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MaxDesiatov

@tomerd tomerd force-pushed the feature/static-link-stdlib-linux branch from 45d8915 to dd03aca Compare February 14, 2022 19:29
@tomerd
Copy link
Contributor Author

tomerd commented Feb 14, 2022

@swift-ci please smoke test

@tomerd tomerd added the ready Author believes the PR is ready to be merged & any feedback has been addressed label Feb 14, 2022
@tomerd
Copy link
Contributor Author

tomerd commented Feb 15, 2022

@swift-ci please smoke test

ktoso
ktoso approved these changes Feb 15, 2022
View reviewed changes
Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Wohoo :)

@tomerd
Copy link
Contributor Author

tomerd commented Feb 15, 2022

@swift-ci please smoke test

1 similar comment
@tomerd
Copy link
Contributor Author

tomerd commented Feb 17, 2022

@swift-ci please smoke test

@tomerd
Copy link
Contributor Author

tomerd commented Feb 24, 2022

@swift-ci please smoke test Linux

@tomerd
Copy link
Contributor Author

tomerd commented Feb 24, 2022

@swift-ci please smoke test

@tomerd tomerd force-pushed the feature/static-link-stdlib-linux branch from 50cbd94 to cda15a6 Compare March 1, 2022 01:02
@tomerd
Copy link
Contributor Author

tomerd commented Mar 1, 2022

@swift-ci please smoke test linux

@tomerd tomerd force-pushed the feature/static-link-stdlib-linux branch from 0a439b9 to 20cb939 Compare March 2, 2022 00:40
@tomerd
Copy link
Contributor Author

tomerd commented Mar 2, 2022

@swift-ci please smoke test

Kyle-Ye
Kyle-Ye reviewed Mar 7, 2022
View reviewed changes
@tomerd tomerd force-pushed the feature/static-link-stdlib-linux branch from 20cb939 to 47b7691 Compare March 22, 2022 23:33
@tomerd
Copy link
Contributor Author

tomerd commented Mar 22, 2022

@swift-ci smoke test

@tomerd tomerd force-pushed the feature/static-link-stdlib-linux branch from 47b7691 to a2d598e Compare March 22, 2022 23:35
@tomerd
Copy link
Contributor Author

tomerd commented Mar 22, 2022

@swift-ci smoke test

tomerd added 4 commits March 25, 2022 15:04
@tomerd
enable static linking of the swift runtime libraries by default on su…
f613192 
…pported platforms

motivation: on some platforms (eg Linux) statically linking the swift runtime libraries makes more sense to allow better mobility of execuables

changes:
* deprecate the opt-in "static-swift-stdlib" CLI flag
* introduce an opt-out "disable-static-swift-runtime" CLI flag
* change behavior to automatically build/link with "-static-stdlib" in release mode on supported platforms (Linux and WASI)
* adjust tests
@tomerd
fixup
345b263
@tomerd
fixup
0787cae
@tomerd
fixup
e4afd4d
@tomerd tomerd force-pushed the feature/static-link-stdlib-linux branch from a2d598e to e4afd4d Compare March 25, 2022 23:30
@tomerd
Copy link
Contributor Author

tomerd commented Mar 25, 2022

@swift-ci please smoke test

tomerd
tomerd commented Mar 26, 2022
View reviewed changes
@tomerd
Update Utilities/Docker/Dockerfile

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
ba038c8
@neonichu neonichu added next waiting for next merge window and removed pending evolution proposal ready Author believes the PR is ready to be merged & any feedback has been addressed labels Apr 11, 2022
@tomerd
Copy link
Contributor Author

tomerd commented Apr 12, 2022

@swift-ci please smoke test

@MaxDesiatov MaxDesiatov mentioned this pull request May 12, 2022
Closed
@kateinoigakukun kateinoigakukun mentioned this pull request Jun 14, 2022
Open
@MaxDesiatov MaxDesiatov added the WIP Work in progress label Oct 26, 2022
@tomerd
Copy link
Contributor Author

tomerd commented Oct 28, 2022

cc @MaxDesiatov

@MaxDesiatov MaxDesiatov assigned MaxDesiatov and unassigned tomerd Oct 28, 2022
@rauhul
Copy link
Member

rauhul commented Jan 14, 2024

This evolution proposal was accepted ~2 years ago; are we still planning on merging this change?

@MaxDesiatov
Copy link
Contributor

Yes, but the PR needs to be updated with conflicts resolved, tests enabled/fixed etc.

@kateinoigakukun kateinoigakukun mentioned this pull request Jan 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxDesiatov MaxDesiatov

@Kyle-Ye Kyle-Ye

@neonichu neonichu

@ktoso ktoso

@abertelrud abertelrud

@elsh elsh

@compnerd compnerd

At least 1 approving review is required to merge this pull request.

Assignees

@MaxDesiatov MaxDesiatov

Labels
next waiting for next merge window WIP Work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@tomerd @MaxDesiatov @rauhul @neonichu @ktoso @Kyle-Ye