Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

utils: Provide error message for missing build preset options #254

Merged
merged 1 commit into from
Dec 6, 2015
Merged

utils: Provide error message for missing build preset options #254

merged 1 commit into from
Dec 6, 2015

Conversation

13rac1
Copy link
Contributor

@13rac1 13rac1 commented Dec 5, 2015

Adds a try/catch block to utils/SwiftBuildSupport.py to catch ConfigParser.InterpolationMissingOptionError caused by missing required preset options such as: install_destdir.

Changes this:

$ ./build-script --preset=buildbot_linux_1404
Traceback (most recent call last):
  File "./build-script", line 755, in <module>
    sys.exit(main())
  File "./build-script", line 749, in main
    return main_preset()
  File "./build-script", line 77, in main_preset
    args.preset_substitutions, args.preset_file_names, args.preset)
  File "/home/user/swift/utils/SwiftBuildSupport.py", line 169, in get_preset_options
    _get_preset_options_impl(config, substitutions, preset_name)
  File "/home/user/swift/utils/SwiftBuildSupport.py", line 147, in _get_preset_options_impl
    _get_preset_options_impl(config, substitutions, mixin)
  File "/home/user/swift/utils/SwiftBuildSupport.py", line 137, in _get_preset_options_impl
    for o, a in config.items(section_name):
  File "/usr/lib/python2.7/ConfigParser.py", line 655, in items
    for option in options]
  File "/usr/lib/python2.7/ConfigParser.py", line 691, in _interpolate
    self._interpolate_some(option, L, rawval, section, vars, 1)
  File "/usr/lib/python2.7/ConfigParser.py", line 723, in _interpolate_some
    option, section, rest, var)
ConfigParser.InterpolationMissingOptionError: Bad value substitution:
    section: [preset: mixin_linux_installation]
    option : install-destdir
    key    : install_destdir
    rawval : 

To this:

$ ./build-script --preset=buildbot_linux_1404
./build-script: missing option 'install_destdir' for preset: mixin_linux_installation

@bitjammer
Copy link
Contributor

Thank you, nice improvement! If I may, could I ask you to take this one step further and:

  • Don't sys.exit(1) right then and there
  • Store some kind of missing_options bool
  • Exit just after the loop terminates if missing_options == True

This will emit all of the missing interpolations instead of wasting time running the command several times to find out you've missed another option.

@13rac1
Copy link
Contributor Author

13rac1 commented Dec 6, 2015

Yes, sounds good.

Adds a try/catch block to utils/SwiftBuildSupport.py to catch
ConfigParser.InterpolationMissingOptionError exceptions caused by
missing required preset options. Stores all missing options to
print before exit(1) in get_preset_options().
@13rac1
Copy link
Contributor Author

13rac1 commented Dec 6, 2015

Ok, hows this?

The exception occurs in the items() generator:

        for o, a in config.items(section_name):

Which breaks out of the entire for loop, so I've changed it to use config.options() and config.get().
_get_preset_options_impl() itself is recursive (you can see it called twice in the traceback above). The missing options list must return back down the call stack to the get_preset_options() to collect all missing options. The same code path build_script_opts and build_script_impl_opts take.

@13rac1
Copy link
Contributor Author

13rac1 commented Dec 6, 2015

Output example:

user@server $ ./build-script --preset=buildbot_linux_140
./build-script: preset 'buildbot_linux_140' not found
user@server $ ./build-script --preset=buildbot_linux_1404 install_destdir="/tmp"
./build-script: missing option(s) for preset 'buildbot_linux_1404': installable_package
user@server $ ./build-script --preset=buildbot_linux_1404 
./build-script: missing option(s) for preset 'buildbot_linux_1404': install_destdir, installable_package

@bitjammer
Copy link
Contributor

Fantastic. Let's pull it in.

bitjammer added a commit that referenced this pull request Dec 6, 2015
utils: Provide error message for missing build preset options
@bitjammer bitjammer merged commit 6fb18a3 into swiftlang:master Dec 6, 2015
@13rac1 13rac1 deleted the configparser-missing-option branch December 6, 2015 18:30
slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018
…0-merge-master

Merge darwin/libdispatch-890 to master
slavapestov pushed a commit to slavapestov/swift that referenced this pull request Nov 27, 2018
…0-merge-master

Merge darwin/libdispatch-890 to master

Signed-off-by: Daniel A. Steffen <[email protected]>
kateinoigakukun added a commit to kateinoigakukun/swift that referenced this pull request Mar 1, 2020
…to-thick-thunk

[WASM] Skip thunk generation for throws func
freak4pc pushed a commit to freak4pc/swift that referenced this pull request Sep 28, 2022
Add SwiftLint Swift 4.2 compatibility version hash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants