[SR-1738] Allow only static libraries to be built #3027

modocache · 2016-06-15T17:47:22Z

What's in this pull request?

This splits the --build-swift-stdlib and --build-swift-sdk-overlay arguments into dynamic and static variants, which makes the following build command possible:

utils/build-script -- \
  --build-swift-dynamic-stdlib=0 --build-swift-dynamic-sdk-overlay=0 \
  --build-swift-static-stdlib=1 --build-swift-static-sdk-overlay=0

This command produces only static libraries for the stdlib, and no SDK overlay libraries at all. Many other finely-grained build options are now possible.

@gribozavr This pull request is built on top of #3020 and #3043; it'll be easier to review and merge those first. Thanks for all your feedback on this! 🙇

Resolved bug number: (SR-1738)

Before merging this pull request to apple/swift repository:

Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform	Comment
All supported platforms	@swift-ci Please smoke test
All supported platforms	@swift-ci Please smoke test and merge
OS X platform	@swift-ci Please smoke test OS X platform
Linux platform	@swift-ci Please smoke test Linux platform

Validation Testing

Platform	Comment
All supported platforms	@swift-ci Please test
All supported platforms	@swift-ci Please test and merge
OS X platform	@swift-ci Please test OS X platform
OS X platform	@swift-ci Please benchmark
Linux platform	@swift-ci Please test Linux platform

Lint Testing

Language	Comment
Python	@swift-ci Please Python lint

Note: Only members of the Apple organization can trigger swift-ci.

modocache · 2016-06-15T17:47:53Z

@swift-ci please test

modocache · 2016-06-16T19:10:26Z

@swift-ci please test

modocache · 2016-06-16T19:12:37Z

@gribozavr This was a sort of work-in-progress, but I've updated it to something I think is good to go. It's based on the work from #3020 and #3043, so it'd be easier to review/merge those before looking at this. Let me know what you think -- thanks!

gribozavr · 2016-06-18T23:57:31Z

Thank you! Would you mind rebasing and fixing the conflicts?

modocache · 2016-06-19T17:24:07Z

Thanks @gribozavr. It's weird, though, this rebased cleanly onto master. I wonder if GitHub determines conflicts differently somehow... oh well!

@swift-ci please test

gribozavr · 2016-06-20T00:45:13Z

utils/build-script-impl

Could we keep build-swift-stdlib and build-swift-sdk-overlay as aliases for --build-swift-static-xyz=1 --build-swift-dynamic-xyz=1? We are using these in build presets, would be best to not break them.

@gribozavr Done! I also updated the documentation in build-script-impl and the source code comments in CMakeLists.txt.

modocache · 2016-06-20T14:11:34Z

@swift-ci please test

modocache · 2016-06-20T20:48:17Z

Looks like the OS X build has stalled for the past 7 hours. Kicking it.

@swift-ci please test

modocache · 2016-06-21T02:40:15Z

@gribozavr OK! I think this is good to go.

gribozavr · 2016-06-21T21:52:35Z

utils/build-script-impl

Shouldn't it turn off the static one as well?

Also, would you mind moving the alias processing into Python? (Possibly in a future change.)

Could you add support for build-swift-sdk-overlay=0? Some of our internal presets use it.

gribozavr · 2016-06-21T21:53:33Z

Thank you! I'm running a few tests and I'll merge soon!

gribozavr · 2016-06-21T23:35:37Z

@modocache Sorry, compatibility with build-swift-sdk-overlay=0 is a blocker for us. Would you mind taking a look?

modocache · 2016-06-22T16:49:25Z

@gribozavr OK, I've addressed your comments on both moving the logic into Python and on build-swift-sdk-overlay 👍

modocache · 2016-06-23T01:36:18Z

@swift-ci please test

modocache · 2016-06-23T18:57:28Z

@gribozavr Is there a way to test the compatibility concerns you mentioned? @swift-ci consistently passes for me. Could you confirm whether this passes, and whether it can be merged? Sorry for all the pings 😅

gribozavr · 2016-06-23T20:34:34Z

Sorry, I was reviewing other patches. I'm running a test locally now.

gribozavr · 2016-06-23T20:45:46Z

@modocache Sorry, this change does not yet fix the issue. The command that we care about is:

./utils/build-script -R --build-swift-sdk-overlay=0

It should not build the overlay at all.

gribozavr · 2016-06-23T20:46:42Z

utils/build-script

Could you add an else clause that sets both to False?

Wouldn't that overwrite whatever was passed to --build-swift-dynamic-stdlib and --build-swift-static-stdlib? For example, consider the following invocation:

utils/build-script --build-swift-static-stdlib=1

Since --build-swift-stdlib is not passed in, the else statement would be executed, which would set --build-swift-static-stdlib=0, thus negating the desired invocation. Or am I missing something?

You're right. We probably want it to be a tristate then. If --build-swift-stdlib is not passed on the command line, do nothing, if it is passed, then it should override other args.

Once this PR is merged, I'll try to remove the uses of legacy parameters internally.

Is there a good way to determine whether --build-swift-stdlib is specified on the command line? Right now it seems like the following are indistinguishable:

$ utils/build-script $ utils/build-script --build-swift-stdlib=0

Is there a good way to distinguish these two using the existing swift_build_support code? Or should I make some modifications there, or just inspect argv?

Thanks for all your help!

There's a way, but we'd need to stop using the optional_bool action. Since this is a short-lived compatibility aid, I think inspecting argv is acceptable.

I think action=optional_bool, default=None will have 3 possible state.
(is None, True, False).
~~Not confirmed though.~~
Confirmed, works as expected:

parser.add_argument( "--my-opt", action=arguments.action.optional_bool, default=None) args = parse_args( ... ) if args.my_opt is None: print("DEFAULT") elif args.my_opt: print("YES") else: print("NO")

Perfect, this is just what I need. Thanks, @rintaro!! I'll make the changes first thing tomorrow.

This splits the `--build-swift-stdlib` and `--build-swift-sdk-overlay` arguments into `dynamic` and `static` variants, which makes the following build command possible: ``` utils/build-script -- \ --build-swift-dynamic-stdlib=0 --build-swift-dynamic-sdk-overlay=0 \ --build-swift-static-stdlib=1 --build-swift-static-sdk-overlay=0 ``` This command produces *only* static libraries for the stdlib, and no SDK overlay libraries at all. Many other finely-grained build options are now possible.

modocache · 2016-06-24T16:19:33Z

@gribozavr OK, I addressed your comments, thanks to @rintaro's suggestion. Please have another look! I'd love to merge this before the weekend. 😎 ☀️

@swift-ci please test

gribozavr · 2016-06-24T17:41:09Z

@modocache Thank you, my tests pass now! Please merge when the CI succeeds!

modocache · 2016-06-24T18:56:04Z

Thanks a ton, @gribozavr! 🙇

modocache changed the title ~~Sr 1738 build only static~~ [SR-1738] Allow *only* static library to be built Jun 15, 2016

modocache force-pushed the sr-1738-build-only-static branch 2 times, most recently from b7e07fb to b3997cb Compare June 16, 2016 19:09

modocache changed the title ~~[SR-1738] Allow *only* static library to be built~~ [SR-1738] Allow *only* static libraries to be built Jun 16, 2016

modocache mentioned this pull request Jun 16, 2016

[CMake] Allow *only* static stdlib to be built #2431

Closed

1 task

modocache force-pushed the sr-1738-build-only-static branch from b3997cb to a1e7a87 Compare June 19, 2016 17:22

gribozavr reviewed Jun 20, 2016
View reviewed changes

modocache force-pushed the sr-1738-build-only-static branch 2 times, most recently from 31cf9a8 to 144c270 Compare June 20, 2016 14:07

gribozavr reviewed Jun 21, 2016
View reviewed changes

modocache force-pushed the sr-1738-build-only-static branch from 144c270 to bed9b45 Compare June 22, 2016 16:48

gribozavr reviewed Jun 23, 2016
View reviewed changes

modocache force-pushed the sr-1738-build-only-static branch from bed9b45 to f12a132 Compare June 24, 2016 16:18

modocache merged commit 48e04b4 into swiftlang:master Jun 24, 2016

modocache deleted the sr-1738-build-only-static branch June 24, 2016 18:55

modocache mentioned this pull request Jun 27, 2016

[swift-3.0-preview-2-branch] build-script fixes #3234

Merged

1 task

modocache mentioned this pull request Aug 24, 2016

[CMake] Replace SWIFT_SDKS with SWIFT_CROSS_COMPILE_DEPLOYMENT_TARGETS #2835

Closed

1 task

modocache mannequin mentioned this pull request Dec 4, 2016

[SR-1738] [CMake] add_swift_library should take explicit SHARED/STATIC argument #44347

Closed

[SR-1738] Allow *only* static libraries to be built #3027

[SR-1738] Allow *only* static libraries to be built #3027

Uh oh!

Conversation

modocache commented Jun 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's in this pull request?

Resolved bug number: (SR-1738)

Uh oh!

modocache commented Jun 15, 2016

Uh oh!

modocache commented Jun 16, 2016

Uh oh!

modocache commented Jun 16, 2016

Uh oh!

gribozavr commented Jun 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

modocache commented Jun 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

modocache commented Jun 20, 2016

Uh oh!

modocache commented Jun 20, 2016

Uh oh!

modocache commented Jun 21, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gribozavr commented Jun 21, 2016

Uh oh!

gribozavr commented Jun 21, 2016

Uh oh!

modocache commented Jun 22, 2016

Uh oh!

modocache commented Jun 23, 2016

Uh oh!

modocache commented Jun 23, 2016

Uh oh!

gribozavr commented Jun 23, 2016

Uh oh!

gribozavr commented Jun 23, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro Jun 24, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

modocache commented Jun 24, 2016

Uh oh!

gribozavr commented Jun 24, 2016

Uh oh!

modocache commented Jun 24, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[SR-1738] Allow only static libraries to be built #3027

[SR-1738] Allow only static libraries to be built #3027

modocache commented Jun 15, 2016 •

edited

Loading

gribozavr commented Jun 18, 2016 •

edited

Loading

rintaro Jun 24, 2016 •

edited

Loading