[WIP] SR-237 Refactor build-script argument parsing logic into separate module
#11827
Conversation
Rostepher
force-pushed
the
build-me-a-swift
branch
from
c25def9 to
d3e2fd5
Compare
|
I'm going to add tests that parse in the preset file and ensure that all the presets still work under this new parser. If there's any conflicts I'll consider that a bug in the new implementation. In addition I have planned to manually get all the flags accepted by the current parser implementation and have a static test against that list to ensure all flags are supported. |
Rostepher
force-pushed
the
build-me-a-swift
branch
from
d3e2fd5 to
e38dc1f
Compare
utils/build_swift/defaults.py
Outdated
There was a problem hiding this comment.
No. We want these to be configurable from the command line. For our smoke tests (where we only test X86), we only want to build the X86 LLVM target.
There was a problem hiding this comment.
For the purpose of this PR I don't think we should change this value. I don't want to change too much in a single PR. I copied this value from the previous default. It won't be too late to change it in a follow-up PR.
There was a problem hiding this comment.
Can you keep this as --release-debuginfo. Otherwise, it is ambiguous against --debug.
There was a problem hiding this comment.
The default for this should be true. It is not expensive and can be useful for tooling purposes.
There was a problem hiding this comment.
For the purpose of this PR I am hesitant to change any default values since I'm already replacing the whole argument parser. I'd rather hold off on reworking the defaults to be more sensible until we have this merged.
There was a problem hiding this comment.
I thought the default was to have this enabled.
There was a problem hiding this comment.
I can double check as I make a cleanup pass over this again to make sure all the defaults are preserved (in combination with the new unit-tests).
There was a problem hiding this comment.
Can you make it clear that this only makes the swift compiler, not the stdlib in debug?
There was a problem hiding this comment.
Can you make it clearer here that "the swift compiler" is not controlled by this option?
There was a problem hiding this comment.
Whats up with the format string here?
There was a problem hiding this comment.
The magical %(default)s format string inputs the default value when the argument parser outputs the help message.
There was a problem hiding this comment.
Just to double check (I didn't look), did you preserve the behavior where --skip-build-llvm builds enough of LLVM to compile Swift archives? (I think it tablegens some things and produces a few utilities, but thats it).
|
I am going to do another pass over this after Ross pings me. He said he is going to make some other changes. |
Rostepher
force-pushed
the
build-me-a-swift
branch
3 times, most recently
from
22d2836 to
27783ab
Compare
…ParserBuilder DSL based on Rintaro's work last year.
…s and fixed one failing test for stdlib-deployment-targets which I couldn't preserve the old behavior with 1:1.
Rostepher
force-pushed
the
build-me-a-swift
branch
from
27783ab to
5fb944c
Compare
…type for the time being.
|
Build failed |
|
@swift-ci please test |
|
@swift-ci please smoke test |
|
Build failed |
|
@swift-ci please smoke test |
|
Closing in favor #12873 and a handful of follow-up PRs which should make reviews much easier. |
3 participants
Purpose
This PR is part of my ongoing effort to refactor
build-scriptandbuild-script-implinto a single script. Here I have re-used part of @rintaro'sargparser_buildermodule from PR #2190 and then implemented a new argument parser for the main script mode.This PR is still a WIP, I still want to add testing for the
argparser_buildermodule and then integrate it with the main script. There's still some other groundwork to finish before that can be accomplished.rdar://problem/34336890