Skip to content

Switch default backend from 'fastcomp' to 'upstream' #373

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

Merged
merged 2 commits into from
Oct 21, 2019
Merged
Show file tree
Hide file tree
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
40 changes: 30 additions & 10 deletions emsdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -1941,7 +1941,7 @@ def find_used_python():


def version_key(ver):
return list(map(int, re.split('[._-]', ver)))
return tuple(map(int, re.split('[._-]', ver)))


# A sort function that is compatible with both Python 2 and Python 3 using a
Expand Down Expand Up @@ -2710,14 +2710,29 @@ def extract_bool_arg(name):

releases_info = load_releases_info()['releases']

def report_upstream_by_default():
print('''\
** NOTICE **: The default SDK changed from `fastcomp` to `upstream`.
If you have problems, or wish to revert back to fastcomp for some other reason
you can add `-fastcomp` to explicitly install that fastcomp-based
SDK, .e.g ./emsdk install latest-fastcomp.
''', file=sys.stderr)

# 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', 'latest-fastcomp', 'latest-releases-fastcomp'):
if arg in ('latest', 'sdk-latest', 'latest-64bit', 'sdk-latest-64bit'):
# This is effectly the default SDK
report_upstream_by_default()
Copy link
Member

Choose a reason for hiding this comment

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

I think we can report this only if the version is high enough? Can maybe share code with the existing check later down.

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 version check make sense below, but here we know that we want to unconditionally select upstream right? We never want latest to point to fastcomp ever again, do we? And if we do we would revert this code.

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'):
report_upstream_by_default()
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'):
Expand All @@ -2728,17 +2743,23 @@ def extract_bool_arg(name):
# x.y.z[-(upstream|fastcomp_])
# sdk-x.y.z[-(upstream|fastcomp_])-64bit
# TODO: support short notation for old builds too?
upstream = False
backend = None
if '-upstream' in arg:
arg = arg.replace('-upstream', '')
upstream = True
backend = 'upstream'
elif '-fastcomp' in arg:
arg = arg.replace('-fastcomp', '')
upstream = False
backend = 'fastcomp'
arg = arg.replace('sdk-', '').replace('-64bit', '').replace('tag-', '')
release_hash = releases_info.get(arg, None) or releases_info.get('sdk-' + arg + '-64bit')
if release_hash:
sys.argv[i] = 'sdk-releases-%s-%s-64bit' % ('upstream' if upstream else 'fastcomp', release_hash)
if backend is None:
if version_key(arg) >= (1, 39, 0):
report_upstream_by_default()
backend = 'upstream'
else:
backend = 'fastcomp'
sys.argv[i] = 'sdk-releases-%s-%s-64bit' % (backend, release_hash)

if cmd == 'list':
print('')
Expand All @@ -2747,12 +2768,12 @@ def extract_bool_arg(name):
print('The *recommended* precompiled SDK download is %s (%s).' % (find_latest_releases_version(), find_latest_releases_hash()))
print()
print('To install/activate it, use one of:')
print(' latest [default (fastcomp) backend]')
print(' latest-upstream [upstream LLVM wasm backend]')
print(' latest [default (llvm) backend]')
print(' latest-fastcomp [legacy (fastcomp) backend]')
print('')
print('Those are equivalent to installing/activating the following:')
print(' %s' % find_latest_releases_version())
print(' %s-upstream' % find_latest_releases_version())
print(' %s-fastcomp' % find_latest_releases_version())
print('')
else:
print('Warning: your platform does not have precompiled SDKs available.')
Expand All @@ -2764,7 +2785,6 @@ def extract_bool_arg(name):
releases_versions.reverse()
for ver in releases_versions:
print(' %s' % ver)
print(' %s-upstream' % ver)
print()

# Use array to work around the lack of being able to mutate from enclosing
Expand Down
23 changes: 11 additions & 12 deletions test.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ def hack_emsdk(marker, replacement):
# Tests

print('test .emscripten contents (latest was installed/activated in test.sh)')
assert 'fastcomp' in open(os.path.expanduser('~/.emscripten')).read()
assert 'upstream' not in open(os.path.expanduser('~/.emscripten')).read()
assert 'fastcomp' not in open(os.path.expanduser('~/.emscripten')).read()
assert 'upstream' in open(os.path.expanduser('~/.emscripten')).read()

print('building proper system libraries')

Expand Down Expand Up @@ -114,23 +114,22 @@ def run_emsdk(cmd):
else:
emsdk = './emsdk'

test_lib_building(fastcomp_emcc, use_asmjs_optimizer=True)
test_lib_building(upstream_emcc, use_asmjs_optimizer=True)

print('update')
run_emsdk('update-tags')

print('test latest-releases-upstream')
run_emsdk('install latest-upstream')
run_emsdk('activate latest-upstream')
print('test latest-releases-fastcomp')
run_emsdk('install latest-fastcomp')
run_emsdk('activate latest-fastcomp')

test_lib_building(upstream_emcc, use_asmjs_optimizer=False)
test_lib_building(fastcomp_emcc, use_asmjs_optimizer=False)
assert open(os.path.expanduser('~/.emscripten')).read().count('LLVM_ROOT') == 1
assert 'upstream' in open(os.path.expanduser('~/.emscripten')).read()
assert 'fastcomp' not in open(os.path.expanduser('~/.emscripten')).read()

assert 'upstream' not in open(os.path.expanduser('~/.emscripten')).read()
assert 'fastcomp' in open(os.path.expanduser('~/.emscripten')).read()

print('verify version')
checked_call_with_output(upstream_emcc + ' -v', TAGS['latest'], stderr=subprocess.STDOUT)
checked_call_with_output(fastcomp_emcc + ' -v', TAGS['latest'], stderr=subprocess.STDOUT)

print('clear cache')
check_call(upstream_emcc + ' --clear-cache')
Expand Down Expand Up @@ -160,8 +159,8 @@ def run_emsdk(cmd):
print('another install must re-download')
checked_call_with_output(emsdk + ' install 1.38.33', expected='Downloading:', unexpected='already exist in destination')
run_emsdk('activate 1.38.33')
assert 'fastcomp' in open(os.path.expanduser('~/.emscripten')).read()
assert 'upstream' not in open(os.path.expanduser('~/.emscripten')).read()
assert 'fastcomp' in open(os.path.expanduser('~/.emscripten')).read()

print('test specific release (new, full name)')
run_emsdk('install sdk-1.38.33-upstream-64bit')
Expand Down