godotengine / godotengine/godot-cpp
SConstruct host_platform
- Dominant language
- C++
- Stars
- 2.7k
- Forks
- 809
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 8
Description
Looking at the current SConstruct file (after #212), it seems like the `host_platform` value could potentially raise an error even before the `platform` parameter is evaluated. If you are not on a valid platform, but do supply a platform= assignment, then you'd end up with an error message saying you need to specify the platform because it couldn't be detected (which doesn't make any sense). I think we should not raise an error until after the parameter has been collected and evaluated. We also might consider changing the error message itself since our code only handles the 'linux', 'osx', and 'windows' cases anyway. It should instead inform the user of what values are permitted. Either that, or we need to raise an error in the latter if statement sequence to tell users that "build options are undefined for the [automatically?] selected platform: ".
So...
1. Change the following section:
# Try to detect the host platform automatically
# This is used if no `platform` argument is passed
if sys.platform.startswith('linux'):
host_platform = 'linux'
elif sys.platform == 'darwin':
host_platform = 'osx'
elif sys.platform == 'win32':
host_platform = 'windows'
else: # MOVE THIS LINE TO A SEPARATE `if not env['platform']:` LINE AFTER THE PARAM EVALS BELOW
raise ValueError('Could not detect platform automatically, please specify with platform=')
opts = Variables([], ARGUMENTS)
opts.Add(EnumVariable('platform', 'Target platform', host_platform,
allowed_values=('linux', 'osx', 'windows'),
ignorecase=2))
2. Add this to the end of the other if sequence:
if env['platform'] == 'linux':
...
elif env['platform'] == 'osx':
...
elif env['platform'] == 'windows':
...
# ADD THIS LINE
else:
raise ValueError('Build options are not defined for platform: ' + env['platform'])
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.