forked from swiftlang/swift
-
Notifications
You must be signed in to change notification settings - Fork 30
Simplify build script #2733
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
Merged
Merged
Simplify build script #2733
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
0b1702f
[WASM] Simplify Foundation build script
kateinoigakukun f852547
[WASM] Remove WASI ICU related code from CMake build system
kateinoigakukun 81af4f3
Refactoring build-toolchain script
kateinoigakukun 7359de5
[WASM] Fix prebuilt libraries path
kateinoigakukun 1923cef
[WASM] Always install ubuntu wasi-sdk
kateinoigakukun 62f1454
Fix unarchive command
kateinoigakukun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
#!/bin/bash | ||
|
||
set -eux | ||
|
||
SOURCE_PATH="$( cd "$(dirname "$0")/../../../" && pwd )" | ||
BUILD_SDK_PATH="$SOURCE_PATH/build-sdk" | ||
|
||
install_libxml2() { | ||
LIBXML2_URL="https://github.com/swiftwasm/libxml2-wasm/releases/download/1.0.0/libxml2-wasm32-unknown-wasi.tar.gz" | ||
curl -L "$LIBXML2_URL" | tar xz | ||
rm -rf "$BUILD_SDK_PATH/libxml2" | ||
mv libxml2-wasm32-unknown-wasi "$BUILD_SDK_PATH/libxml2" | ||
} | ||
|
||
install_icu() { | ||
ICU_URL="https://github.com/swiftwasm/icu4c-wasi/releases/download/0.5.0/icu4c-wasi.tar.xz" | ||
curl -L "$ICU_URL" | tar Jx | ||
rm -rf "$BUILD_SDK_PATH/icu" | ||
mv icu_out "$BUILD_SDK_PATH/icu" | ||
} | ||
|
||
install_wasi-sdk() { | ||
# We only use wasi-sysroot and do not use binaries in wasi-sdk, | ||
# so build machine's os and wasi-sdk's host os don't have to be matched | ||
WASI_SDK_URL="https://github.com/swiftwasm/wasi-sdk/releases/download/0.2.2-swiftwasm/dist-ubuntu-18.04.zip" | ||
|
||
curl -L -o dist-wasi-sdk.zip "$WASI_SDK_URL" | ||
unzip -u dist-wasi-sdk.zip -d . | ||
|
||
WASI_SDK_TAR_PATH=$(find . -type f -name "wasi-sdk-*") | ||
WASI_SDK_FULL_NAME=$(basename "$WASI_SDK_TAR_PATH" -linux.tar.gz) | ||
tar xfz "$WASI_SDK_TAR_PATH" | ||
|
||
rm -rf "$BUILD_SDK_PATH/wasi-sdk" | ||
mv "$WASI_SDK_FULL_NAME" "$BUILD_SDK_PATH/wasi-sdk" | ||
} | ||
|
||
workdir=$(mktemp -d) | ||
pushd "$workdir" | ||
|
||
mkdir -p "$BUILD_SDK_PATH" | ||
|
||
install_libxml2 | ||
install_icu | ||
install_wasi-sdk |
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am unsure about moving ICU download and unpacking process from CMake to a shell script. If, for example, we're interested in making SwiftWasm work on Windows, wouldn't we need to move it back to CMake again?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main purpose is minimizing platform specific code from cmake build system.
This kind of external library information should be separated from the use-site because the use-site doesn't care which flavor of libraries they are using.
If we want to support non Unix-like platform as a host environment, we need use rewrite a downloader script in more portable language. But this separation allows the CMake build system to be more flexible.
The build system flexibility will be helpful when we stop forking and only build the toolchain like compnerd/swift-build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't CMake more portable than bash from this perspective then? And would moving this code a separate CMake file make this more maintainable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the use and injection of build parameters should be separated.
If using and injecting are integrated in a same script, then the build system will only be able to build a single flavor.
I think such a change is unlikely to be accepted upstream.
Writing a script in CMake to download a file for injection is fine, but that script should be isolated from the project's build system.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, thanks for the clarification!
I'm thinking about playing with Swift on Windows at some point, and that made me consider the future of SwiftWasm on that platform as well. Proliferation of shell scripts in our build system will only complicate porting efforts, so I wonder if we should have a coherent strategy for how this evolves moving forward...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll just mention I sometimes work in really bad network conditions, so having all dependencies downloaded before build is essential. I've always commented out this ICU download in my local copy.