Python-only build script #23038

drodriguez · 2019-03-02T02:09:25Z

Applies to SR-237.

This is a partial reimplementation of build-script-impl into Python. This was necessary if we wanted to build on Windows with the build-script. At the same time it includes fixes for compatibility with Python 3 (keeping code backwards compatible with Python 2).

Currently it can build and test CMark, LLVM, Swift and LLDB (the test fail in Windows, but that's not the point).

I tried to follow the design that I can intuited in the current Python code, but I have to diverge in some points (because I wanted to keep the usage of build-script-impl untouched during the migration). A new parameter --native-impl allows to use the new code while still allowing the usage of the old code.

I hope the ideas I introduce makes sense and we can move forward with this. It would be great to have the Windows builds using the same process than the rest of the builds, and I think it would be a requirement for being integrated with the community CI. This rewrites will also help to future modifications of the build script, since information and process will not have to be duplicated in two places.

Any feedback is appreciated.

Changes

Move HostSpecificConfiguration into its own file to be shared between the main script and the products that need to make use of it to recover CMake targets (mostly Swift, at the moment). There's also changes to avoid depending on the invocation object directly and be less tied to it.
As a transitional measure, much of the code of BuildScriptInvocation has been moved into BuildScriptBase, and it is shared with its subtypes BuildScriptInvocation and BuildScriptNative. Base will invoke different (abstract) methods, that the subtypes implement to either invoke build-script-impl with the right arguments, or start executing some Python code for each product.
There is the new concept of "product builders", which implement the logic to build the products. Each product provides a class method to recover its product builder (or in the case of LLDB, one of the two possible product builders). These product builders receive the arguments, the toolchain, the workspace, the host,... and take care of massaging all those values into proper CMake or shell invocations.
A template for a builder using CMake is provided: CMakeProductBuilder (currently in products/product.py, but should be moved out). It implements the common logic for building with CMake, and provides many templated methods/properties for the products to customize. CMark, LLVM, Swift and LLDB use this builder as a base.
The CMake type learns a couple new tricks and help invoking CMake for generating the build files, and for both building targets and installing targets.
Some parameters have been copied from build-script-impl to build-script in order to get access to their values during the build process. All of them should be forwarded to build-script-impl. When the arguments where used in one of the build presets, I tried to keep compatibility with how the build-script-impl argument seemed to work.
I repurposed the parameters used for passing ICU paths for Android for Windows. This might not be very clever if we ever want to cross-compile Android from Windows, but for the time being, it works.
In many places I needed to modify how Python scripts were invoked for Windows. In some cases, a .cmd wrapper is provided (build-script.cmd, coverage-touch-tests.cmd), but in other cases, Python is invoked internally (like llvm-lit.py).
Many changes for Windows compatibility.
Many changes for Python 3 compatibility.

TODO

Transform the rest of the products to use CMake builder or the appropiate builder.
install actions do not work. merge, extractsymbols and package do not work either.
Find out about many of the arguments that do not seem to be used in build-script-impl. Is it worth it to bring them over, or can the code be removed?
Disable the invocation of build-script-impl to check the arguments if using the native implementation. It will not work on Windows (for example). I have a workaround locally, but it should be encoded into the script (no need to check the arguments against impl if we are not going to use it).
Modify Ninja, indexstore-db and sourcekit-lsp to adapt to the new idea of the product builders.
Check this script on Linux
Check this script on Darwin
Check all the build in Python 2.

/cc @Rostepher @shahmishal
/cc @benlangmuir: I saw that you modified the code recently to build indexstore-db and sourcekit-lsp. I think the new design I propose will work with those changes. If you have any feedback, please don't hesitate to comment.

ghost · 2019-03-02T03:32:22Z

Are TODOs in FIFO order? Make is pretty important. Nice writeup.

benlangmuir · 2019-03-04T17:45:34Z

I saw that you modified the code recently to build indexstore-db and sourcekit-lsp. I think the new design I propose will work with those changes. If you have any feedback, please don't hesitate to comment.

The only "interesting" part of building these is that you will need support for installing the earlier parts of the toolchain.

drodriguez · 2019-03-04T18:49:23Z

@openloop: the order was as they were coming to my mind. No other intentional order.

@benlangmuir: All right. I wasn't sure why they were done after the rest of the projects had finished the build-test-install steps. That's something I will have to had into account. Thanks for pointing it out. Do you remember why they have to be installed? Dispatch and Foundation use Swift in the build directory without first installing it. Can those other do the same?

Rostepher · 2019-03-04T19:09:22Z

As a quick note, I think non-build-script changes should be split out of this PR. I see a lot of changes to various .swift.gyb files and scripts in utils. I'd recommend splitting those updates out into separate PRs to avoid making one massive PR that becomes un-reviewable and worse yet un-testable.

drodriguez · 2019-03-04T19:14:00Z

@Rostepher: Totally agree. There's a mix here between compatibility with Python 3, compatibility with Windows and the new build script. Most of the changes for Python 3 should be sent independently. I'm working in all at the same time because it makes sense to have everything in the same branch (for me). But I will have to split them in smaller PRs. And I will do it when I'm sure they are mostly stable and unchanging.

benlangmuir · 2019-03-04T19:19:11Z

Do you remember why they have to be installed? Dispatch and Foundation use Swift in the build directory without first installing it. Can those other do the same?

Passing through all of the paths doesn't scale, particularly for tools that are downstream of swiftpm or Foundation. There's more information at https://forums.swift.org/t/rfc-building-swift-packages-in-build-script/18920

drodriguez · 2019-03-04T20:39:05Z

That write up is great. Thanks for sharing. I wonder why both build-script-impl and build-script try to do all build phases, then all test phases, and then all install phases. It would be simpler to do build/test/install for each product. That way you will have a simple dependency tree. Did you figure out if there is a reason for it? (I can imagine that building is faster than testing to find early problems, but doesn't feel like a strong reason).

benlangmuir · 2019-03-04T21:15:54Z

I don't know if there is a principled reason for the current state of affairs. I suggest not trying to change the order of operations at the same time as rewriting the infrastructure.

compnerd · 2019-03-07T16:49:13Z

utils/coverage/coverage-touch-tests

@@ -86,7 +86,7 @@ def main():
    console.setLevel(level=args.log.upper())
    logging.debug(args)
    environment = os.environ.copy()
-    environment['GIT_EXTERNAL_DIFF'] = script_path
+    environment['GIT_EXTERNAL_DIFF'] = script_path.replace('\\', '/')


I could've sworn that that there was a path utility to convert the slashes in python.

It looks like os.path.normpath does what you're looking for.

It does the opposite if I read the documentation right: “On Windows, it converts forward slashes to backward slashes.”. In this case, for some reason, GIT_EXTERNAL_DIFF was removing the backward slashes from the path, but worked when I provided the forward slashes. I can add a comment explaining why it is needed.

The changes in gyb_syntax_support are simple rewrites of the import statements to work both in Python 3 and 2. The changes in the gyb files themselves are some of the problems found by compiling, like expecting integer division and printing floats, and usage of xrange/range.

In many cases, the changes are related to the handling of files, using io.open instead of open to improve compatibility.

For some reason, GIT_EXTERNAL_DIFF in Windows remove the Windows slashes of the value. Fortunately, replacing the Windows path separator for the Linux one seems to work and solves that problem.

Switch the ambigous relative imports to Python 3 relative imports (supported in Python 2.7). Only perform the renaming of the main script in Python 2, or Python 3 will fall into a recursive loop (if the workaround is removed, Python 2 falls into a similar recursive loop). Finally add a couple of decode() calls for the output/error of the shell utilities to be printed as text in Python 3.

drodriguez · 2020-01-10T16:50:23Z

Abandoning this efforts. It might still be useful for better Windows builds for most of the community, but I will not have time to keep improving this and it doesn't seem to be a lot of interest in improving the build system in this direction.

benlangmuir requested review from Rostepher and compnerd March 4, 2019 17:34

drodriguez force-pushed the build-script-windows-python branch from 869b16b to 3baed25 Compare March 6, 2019 02:23

compnerd reviewed Mar 7, 2019

View reviewed changes

drodriguez added 18 commits March 11, 2019 14:11

[build-script] Compatibility with Python 3

84fc494

[build-script] Improve compatibility with Windows platforms.

58a2401

[build-script] Add Windows CMD for build-script. Invokes original.

ad14c88

[utils] Improve compatibility with Python 3.

fd5d42c

In many cases, the changes are related to the handling of files, using io.open instead of open to improve compatibility.

[coverage-touch-tests] Improve compatibility with Windows.

5b718aa

For some reason, GIT_EXTERNAL_DIFF in Windows remove the Windows slashes of the value. Fortunately, replacing the Windows path separator for the Linux one seems to work and solves that problem.

[build-script][WIP] Rewrite build-script-impl in Python for Windows

65af05b

[build-script][WIP] Support for LLDB in native Python

ba6eac4

[build-script][WIP] Support for LibDispatch in native Python

efe8549

[build-script][WIP] Support for Foundation in native Python

595ea4c

[build-script][WIP] Support for XCTest in native Python

a2b996d

[test] Make some Driver test compatible with Python 3

bed35e3

[test] Make binary_sub.py compatible with Python 3

5b639d2

[test] Make tokens_unknown_and_invalid.swift compatible with Python 3

77a3b89

[test] Make lit.cfg compatible with Python 3

5191119

[test] Make validate_parse.py compatible with Python 3

730047e

[test] Make jobstats.py compatible with Python 3

6c54372

drodriguez added 7 commits March 11, 2019 14:11

[test] Make line-directive compatible with Python 3

1cb670a

[utils] Make process-stats-dir.py compatible with Python 3

678efb2

[utils] Make remote-run compatible with Python 3

6cb771e

[test] Make round-trip-syntax-test compatible with Python 3

c0f97a4

[test] Make scale-test compatible with Python 3

36af35b

[test] Make update_checkout.test-sh compatible with Python 3

6e89ae8

[test] Fix FixedPointConversion tests

0c6a2c3

drodriguez force-pushed the build-script-windows-python branch from 11d4787 to 0c6a2c3 Compare March 11, 2019 21:12

This was referenced Mar 13, 2019

Build script builders #23257

Closed

Initial Python 3 compatibility fixes #23256

Closed

[build-script] Improvements to CMakeOptions. #23303

Merged

Resolves [SR-9941] [SR-1805] remove python implicit import as it is deprecated #23272

Closed

nafu mentioned this pull request Mar 22, 2019

[SR-10150] Specify supported python version in README #23488

Merged

drodriguez removed the request for review from Rostepher April 8, 2019 15:41

drodriguez closed this Jan 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Python-only build script #23038

Python-only build script #23038

Uh oh!

drodriguez commented Mar 2, 2019 •

edited

Loading

Uh oh!

ghost commented Mar 2, 2019

Uh oh!

benlangmuir commented Mar 4, 2019

Uh oh!

drodriguez commented Mar 4, 2019

Uh oh!

Rostepher commented Mar 4, 2019

Uh oh!

drodriguez commented Mar 4, 2019

Uh oh!

benlangmuir commented Mar 4, 2019

Uh oh!

drodriguez commented Mar 4, 2019

Uh oh!

benlangmuir commented Mar 4, 2019

Uh oh!

compnerd Mar 7, 2019

Uh oh!

Rostepher Mar 7, 2019

Uh oh!

drodriguez Mar 8, 2019

Uh oh!

drodriguez commented Jan 10, 2020

Uh oh!

Uh oh!

Python-only build script #23038

Python-only build script #23038

Uh oh!

Conversation

drodriguez commented Mar 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

TODO

Uh oh!

ghost commented Mar 2, 2019

Uh oh!

benlangmuir commented Mar 4, 2019

Uh oh!

drodriguez commented Mar 4, 2019

Uh oh!

Rostepher commented Mar 4, 2019

Uh oh!

drodriguez commented Mar 4, 2019

Uh oh!

benlangmuir commented Mar 4, 2019

Uh oh!

drodriguez commented Mar 4, 2019

Uh oh!

benlangmuir commented Mar 4, 2019

Uh oh!

compnerd Mar 7, 2019

Choose a reason for hiding this comment

Uh oh!

Rostepher Mar 7, 2019

Choose a reason for hiding this comment

Uh oh!

drodriguez Mar 8, 2019

Choose a reason for hiding this comment

Uh oh!

drodriguez commented Jan 10, 2020

Uh oh!

Uh oh!

drodriguez commented Mar 2, 2019 •

edited

Loading