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

Disable fastcomp installation for sdk versions 2.0.0 and above #581

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 46 additions & 38 deletions emsdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
print('Note: fastcomp is not longer available in version 2.0.0 and above')
print('Note: fastcomp is not longer available in version 2.0.0 and above. please use either the upstream backend, or an earlier fastcomp version')

return name
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error this produces seems pretty good to me:

$ ./emsdk install 2.0.0-fastcomp
Note: fastcomp is not longer available in version 2.0.0 and above.
Error: No tool or SDK found by name '2.0.0-fastcomp'.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love the fact that it depends on 2.0.0-fastcomp not existing and being errored on elsewhere, when we know this should fail right here. But that's a reasonable error.

return 'sdk-releases-%s-%s-64bit' % (backend, release_hash)
return name


def main():
global BUILD_FOR_TESTING, ENABLE_LLVM_ASSERTIONS, TTY_OUTPUT

Expand Down Expand Up @@ -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('')
Expand Down Expand Up @@ -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()
Expand Down