-
Notifications
You must be signed in to change notification settings - Fork 682
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
Disable fastcomp installation for sdk versions 2.0.0 and above #581
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2635,6 +2635,50 @@ def error_on_missing_tool(name): | |
return 1 | ||
|
||
|
||
def expand_sdk_name(name): | ||
if name in ('latest', 'sdk-latest', 'latest-64bit', 'sdk-latest-64bit'): | ||
# This is effectly the default SDK | ||
return str(find_latest_releases_sdk('upstream')) | ||
elif name in ('latest-fastcomp', 'latest-releases-fastcomp'): | ||
return str(find_latest_releases_sdk('fastcomp')) | ||
elif name in ('latest-upstream', 'latest-clang-upstream', 'latest-releases-upstream'): | ||
return str(find_latest_releases_sdk('upstream')) | ||
elif name in ('tot', 'sdk-tot'): | ||
return str(find_tot_sdk('upstream')) | ||
elif name == 'tot-upstream': | ||
return str(find_tot_sdk('upstream')) | ||
elif name in ('tot-fastcomp', 'sdk-nightly-latest'): | ||
return str(find_tot_sdk('fastcomp')) | ||
else: | ||
# check if it's a release handled by an emscripten-releases version, | ||
# and if so use that by using the right hash. we support a few notations, | ||
# x.y.z[-(upstream|fastcomp_]) | ||
# sdk-x.y.z[-(upstream|fastcomp_])-64bit | ||
# TODO: support short notation for old builds too? | ||
backend = None | ||
fullname = name | ||
if '-upstream' in fullname: | ||
fullname = name.replace('-upstream', '') | ||
backend = 'upstream' | ||
elif '-fastcomp' in fullname: | ||
fullname = fullname.replace('-fastcomp', '') | ||
backend = 'fastcomp' | ||
fullname = fullname.replace('sdk-', '').replace('-64bit', '').replace('tag-', '') | ||
releases_info = load_releases_info()['releases'] | ||
release_hash = get_release_hash(fullname, releases_info) | ||
if release_hash: | ||
if backend is None: | ||
if version_key(fullname) >= (1, 39, 0): | ||
backend = 'upstream' | ||
else: | ||
backend = 'fastcomp' | ||
if backend == 'fastcomp' and version_key(fullname) >= (2, 0, 0): | ||
print('Note: fastcomp is not longer available in version 2.0.0 and above') | ||
return name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it might be safer to error here? both code-wise it would be clearer that everything halts here. and the error for users would be better I think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error this produces seems pretty good to me:
Basically we are failing expand the input given by the user and then the calling code displays the same error the would see if they had made any other kind of typo or something. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love the fact that it depends on |
||
return 'sdk-releases-%s-%s-64bit' % (backend, release_hash) | ||
return name | ||
|
||
|
||
def main(): | ||
global BUILD_FOR_TESTING, ENABLE_LLVM_ASSERTIONS, TTY_OUTPUT | ||
|
||
|
@@ -2808,47 +2852,10 @@ def extract_bool_arg(name): | |
return 1 | ||
sys.argv = [x for x in sys.argv if not len(x) == 0] | ||
|
||
releases_info = load_releases_info()['releases'] | ||
|
||
# Replace meta-packages with the real package names. | ||
if cmd in ('update', 'install', 'activate'): | ||
for i in range(2, len(sys.argv)): | ||
arg = sys.argv[i] | ||
if arg in ('latest', 'sdk-latest', 'latest-64bit', 'sdk-latest-64bit'): | ||
# This is effectly the default SDK | ||
sys.argv[i] = str(find_latest_releases_sdk('upstream')) | ||
elif arg in ('latest-fastcomp', 'latest-releases-fastcomp'): | ||
sys.argv[i] = str(find_latest_releases_sdk('fastcomp')) | ||
elif arg in ('latest-upstream', 'latest-clang-upstream', 'latest-releases-upstream'): | ||
sys.argv[i] = str(find_latest_releases_sdk('upstream')) | ||
elif arg in ('tot', 'sdk-tot'): | ||
sys.argv[i] = str(find_tot_sdk('upstream')) | ||
elif arg == 'tot-upstream': | ||
sys.argv[i] = str(find_tot_sdk('upstream')) | ||
elif arg in ('tot-fastcomp', 'sdk-nightly-latest'): | ||
sys.argv[i] = str(find_tot_sdk('fastcomp')) | ||
else: | ||
# check if it's a release handled by an emscripten-releases version, | ||
# and if so use that by using the right hash. we support a few notations, | ||
# x.y.z[-(upstream|fastcomp_]) | ||
# sdk-x.y.z[-(upstream|fastcomp_])-64bit | ||
# TODO: support short notation for old builds too? | ||
backend = None | ||
if '-upstream' in arg: | ||
arg = arg.replace('-upstream', '') | ||
backend = 'upstream' | ||
elif '-fastcomp' in arg: | ||
arg = arg.replace('-fastcomp', '') | ||
backend = 'fastcomp' | ||
arg = arg.replace('sdk-', '').replace('-64bit', '').replace('tag-', '') | ||
release_hash = get_release_hash(arg, releases_info) | ||
if release_hash: | ||
if backend is None: | ||
if version_key(arg) >= (1, 39, 0): | ||
backend = 'upstream' | ||
else: | ||
backend = 'fastcomp' | ||
sys.argv[i] = 'sdk-releases-%s-%s-64bit' % (backend, release_hash) | ||
sys.argv[i] = expand_sdk_name(sys.argv[i]) | ||
|
||
if cmd == 'list': | ||
print('') | ||
|
@@ -2879,6 +2886,7 @@ def installed_sdk_text(name): | |
key=lambda x: [int(v) if v.isdigit() else -1 for v in x.split('.')], | ||
reverse=True, | ||
) | ||
releases_info = load_releases_info()['releases'] | ||
for ver in releases_versions: | ||
print(' %s %s' % (ver, installed_sdk_text('sdk-releases-upstream-%s-64bit' % get_release_hash(ver, releases_info)))) | ||
print() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.