SR-2100 - Add support for multi-lib systems #1477
Conversation
TestFoundation/TestURLSession.swift
Outdated
| @@ -30,7 +30,7 @@ class TestURLSession : LoopbackServerTest { | |||
| ("test_finishTaskAndInvalidate", test_finishTasksAndInvalidate), | |||
| ("test_taskError", test_taskError), | |||
| ("test_taskCopy", test_taskCopy), | |||
| ("test_cancelTask", test_cancelTask), | |||
| // ("test_cancelTask", test_cancelTask), | |||
There was a problem hiding this comment.
Why has this test been disabled?
There was a problem hiding this comment.
This test resulted in a odd intermittent failure, but i never bothered to track why since i put all my effort on SR-2100 and it was clearly unrelated to my modifications on the build script, maybe this test failure was specific of my build setup, likely due my firewall.
I forgot to uncomment before doing the pull request since i commented that long ago on my multilib branch, I will re-enable. Please, ask the swift-ci bot to retest it just to be sure.
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
Seems reasonable to me. |
|
@swift-ci please test |
configure
Outdated
| @@ -136,6 +136,7 @@ def main(): | |||
| config.script_path = config.source_root.path_by_appending("build.py") | |||
| config.build_script_path = config.source_root.path_by_appending("build.ninja") | |||
| config.config_path = config.source_root.path_by_appending(".configuration") | |||
| config.libdir = os.getenv("LIBDIR", "lib") | |||
There was a problem hiding this comment.
I'm always a little scared about repeatable builds when the config script picks up from the build-host environment. Is there a way to do this via the config setup that we have instead?
There was a problem hiding this comment.
Sure, i can add it as a configure command line option, creating a "--libdir" switch using the parser with a default "lib" value. It will imply in a minor modification on build-script-impl at swift repository, but there is no problem since nobody is reviewing that pull request.
There was a problem hiding this comment.
Yeah, I'd prefer this approach. Thanks!
|
@Capent What's the corresponding build-script pull request? |
|
@millenomi It's here. |
|
Please test with following pull request: @swift-ci please test |
|
@millenomi This swift-ci test will not work, because the pull request swiftlang/swift#15260 requires the following pull requests to work: I should have explained myself better in my last answer, sorry! The issue with the swiftlang/swift#15260 is that it integrates also the changes needed to build all 'SR-2100' pull requests that i did these repositories above, since it incorporates the "master" build script, and the build script expects the new command line arguments from the other repositories as well. As it is, this pull request here is designed to work both with current build script from swift, and the one from the pull request, since it define default values and you can omit the suffix from the build command (the --libdir-suffix is optional). |
|
Please let me know if assistance is required in merging. LGTM. |
configure
Outdated
| parser.add_argument('--reconfigure', dest='reconfigure', action="store_true", help="re-generate the build script given the previous configuration") | ||
| parser.add_argument('-v', '--verbose', dest='verbose', action="store_true") | ||
| args, extras = parser.parse_known_args() | ||
|
|
||
| config.libdir = "lib" + args.libdir_suffix | ||
|
|
There was a problem hiding this comment.
If the default is None, concatenation without that flag passed will throw an exception. Perhaps check?
There was a problem hiding this comment.
True, it will throw an exception. Changed for a explicit default before the parse, and then check before concatenating the string.
|
Actually, one problem. |
|
I believe that this change will do it. |
|
@swift-ci please test |
|
Can we rebase these changes onto master, instead of merging upstream tracking branches in? It will be cleaner in the history when it's finally merged if we do this. |
|
@alblue Ok, i will try, but it may make a mess out of this chat history. |
set library dir name, default to "lib", so platforms that uses "lib64" can set this as the libdir name. This works by setting a new LIBDIR variable, used on file copies by the installer
|
@swift-ci please test |
|
This PR no longer has an author. I'm closing it for now, but please comment here if you want to take ownership of this. |
SR-2100 - Add support for multi-lib systems
Create the --libdir build option on config.py that set library dir name, default to "lib",
so platforms that uses "lib64" can set this as the libdir name.
This works by setting a new LIBDIR variable, used on file copies by the installer.