[SR-237][build-script] Migrate more of build-script-impl to Python

[modocache]

What's in this pull request?

Six commits that each move a little more code out of the utils/build-script-impl shellscript, into Python. This is pursuant to the goals in SR-237.

These commits do not introduce any breaking changes--using the migrate_impl_args function introduced in #761, arguments meant for build-script-impl are routed to build-script instead.

Why merge this pull request?

It makes progress on SR-237. The goal is to eventually eliminate an entire step from the Swift build process, to simply have Python invoke CMake (instead of an additional shellscript being executed in between).

What are the downsides to merging this pull request?

This pull request contains multiple changes that could be broken up into individual pull requests. I opted against separate pull requests because I thought maintainers would find it noisy. Also, the sixth commit in this stack depends on commits four and five, and GitHub doesn't have a great way of specifying dependencies in pull requests.

[modocache]

Care for a code review, @practicalswift? 😉

Also, I've run flake8 from the top level directory of this repository, and I'm seeing about 120 errors (thankfully none of them added in this pull request). Does the .pep8 file not cover all violations in this repository? Or am I doing something wrong?

[gribozavr]

I feel like the tar() function itself should be doing chdir() -- and back. We have a WorkingDirectory class in utils/SwiftBuildSupport.py, by the way.

[modocache]

Can do!

[dabrahams]

Just want to say, thanks for doing this. You’re scratching a long-standing itch!

[modocache]

@dabrahams Yes of course! Glad to do it. I'm think the build scripts could be much simpler, but it will take some time to remove build-script-impl.

[modocache]

(Just FYI, I won't be at a computer for a few days. I'll update this around Sunday night or on Monday.)

[shahmishal]

@swift-ci Please test

[shahmishal]

@swift-ci Please test OS X platform

[practicalswift]

@modocache Reviewed - LGTM 👍 :-)

Thanks for cleaning this up!

[practicalswift]

@modocache Regarding the flake8 warnings - turns out that a couple of new warnings were added to flake8 recently. Thanks for making me aware of that. I've now submitted PR:s to fix the reported issues.

One warning remains in the code base (although currently ignored) if you want to take a look at it: E402 module level import not at top of file. I'm not sure that is fixable given the usage of sys.path.append(…).

[modocache]

@gribozavr I've hopefully addressed your comments! 👍

• [SR-237][build-script] Migrate more of build-script-impl to Python #1267 (comment) -- I no longer define tar(), instead using the Python standard library module tarfile. This avoids the question of whether tar() should also change directories. This would have been difficult to do anyway, as the build-script-impl chdir's into the directory specified by --install-symroot, which may not be part of either of the paths passed to the tar() function. tarfile uses gzip, so the compression should be the same as before.
• [SR-237][build-script] Migrate more of build-script-impl to Python #1267 (diff) -- I now exit with an error if the --symbols-package path is not absolute. I also detect the case in which --install-symroot is not specified, as this is a usage error.
• [SR-237][build-script] Migrate more of build-script-impl to Python #1267 (comment) -- Printing out the xcodebuild versions has been inlined into the swift_build_support.debug.print_xcodebuild_versions() function.

Please have another look and let me know if there are any other issues I can address.

[gribozavr]

@modocache Thank you!

If we would use tarfile, would we be able to switch to the xz format for compression? See #801. It brings significant savings.

Does the tarfile module allow changing the owner of the files in the archive? That is important to avoid leaking information from CI, as well as to avoid surprises during unarchival as root.

[modocache]

Oh! Sorry, I hadn't realized #801 was still open, I had misremembered that a pull request to use gzip had been merged. It looks like LZMA2 compression was introduced in Python 3.3, so I guess tarfile is no good for our 2.7 build scripts. I'll amend the commit with the old custom tar() function.

@gribozavr In #1267 (comment) you mention having tar() itself change directories, but I don't think that works for our use case here. The build script currently chdir's into the --install-symroot path, which tar() would not know about unless it took an extra parameter. So I think I'll keep the with WorkingDirectory(args.install_symroot): line as-is.

[modocache]

@gribozavr Updated! Let me know if you have any additional comments.

Also, could you provide me with a sample invocation of the "buildbot_osx_package" preset? It's the only preset in utils/build-presets.ini that uses the --symbols-package flag and I'd like to double-check that it still works as expected.

[gribozavr]

@swift-ci Please test

[gribozavr]

@modocache Thank you!

You can see an invocation example at the top of the CI page: https://ci.swift.org/view/Packages/job/oss-swift-package-osx/

Here's an example with all variables:

export TOOLCHAIN_VERSION=swift-DEVELOPMENT-SNAPSHOT-2016-02-15-a
export ARCHIVE="${TOOLCHAIN_VERSION}-osx.tar.gz"
export SYM_ARCHIVE="${TOOLCHAIN_VERSION}-osx-symbols.tar.gz"
export BUNDLE_IDENTIFIER="org.swift.30${YEAR}${MONTH}${DAY}a"
export DISPLAY_NAME="Xcode Swift DEVELOPMENT Snapshot ${YEAR}-${MONTH}-${DAY} (a)"
export TOOLCHAIN_NAME="${TOOLCHAIN_VERSION}"

export SWIFT_SOURCE_ROOT="${WORKSPACE}"
export SWIFT_BUILD_ROOT="${WORKSPACE}/build"
export SWIFT_INSTALLABLE_PACKAGE="${WORKSPACE}/${ARCHIVE}"
export SWIFT_INSTALL_DIR="${WORKSPACE}/swift-nightly-install"
export SWIFT_INSTALL_SYMROOT="${WORKSPACE}/swift-nightly-symroot"
export SWIFT_TOOLCHAIN_DIR="/Applications/Xcode.app/Contents/Developer/Toolchains/${TOOLCHAIN_NAME}.xctoolchain"
export SYMBOLS_PACKAGE="${WORKSPACE}/${SYM_ARCHIVE}"
export DARWIN_TOOLCHAIN_VERSION="3.0.${YEAR}${MONTH}${DAY}01"

"${WORKSPACE}/swift/utils/build-script" --preset="buildbot_osx_package" \
install_destdir="${SWIFT_INSTALL_DIR}" \
installable_package="${SWIFT_INSTALLABLE_PACKAGE}" \
install_toolchain_dir="${SWIFT_TOOLCHAIN_DIR}" \
install_symroot="${SWIFT_INSTALL_SYMROOT}" \
symbols_package="${SYMBOLS_PACKAGE}" \
darwin_toolchain_bundle_identifier="${BUNDLE_IDENTIFIER}" \
darwin_toolchain_display_name="${DISPLAY_NAME}" \
darwin_toolchain_xctoolchain_name="${TOOLCHAIN_NAME}" \
darwin_toolchain_version="${DARWIN_TOOLCHAIN_VERSION}" \
darwin_toolchain_alias="swift" 

[modocache]

Thanks, @gribozavr! Also thank you for pointing me to the invocations on the CI page, these will be very useful as I continue to work on SR-237.