Skip to content

Commit a8c9373

Browse files
authored
[file-packager] Drop support for --embed with JS output (#25049)
The object file output is more efficient way of embedding and has been recommended via a warning since #16050 (early 2022). With this simplification the file packager now has essentially two output modes: 1. Preloading via JS 2. Embedding via and object file
1 parent cf7b0e1 commit a8c9373

File tree

3 files changed

+75
-92
lines changed

3 files changed

+75
-92
lines changed

ChangeLog.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ See docs/process.md for more on how version tagging works.
2020

2121
4.0.20 (in development)
2222
-----------------------
23+
- The standalone `file_packager.py` script no longer supports `--embed` with JS
24+
output (use `--obj-output` is now required for embedding data). This usage
25+
has been producing a warning since #16050 which is now an error. (#25049)
2326
- Embind now requires C++17 or newer. See #24850.
2427

2528
4.0.19 - 11/04/25

test/test_other.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1998,7 +1998,6 @@ def test_export_from_archive(self):
19981998
'embed_twice': (['--embed-file', 'somefile.txt', '--embed-file', 'somefile.txt'],),
19991999
'preload': (['--preload-file', 'somefile.txt', '-sSTRICT'],),
20002000
'preload_closure': (['--preload-file', 'somefile.txt', '-O2', '--closure=1'],),
2001-
'preload_and_embed': (['--preload-file', 'somefile.txt', '--embed-file', 'hello.txt'],),
20022001
})
20032002
@requires_node
20042003
def test_include_file(self, args):
@@ -3953,8 +3952,8 @@ def test_file_packager_embed(self):
39533952
create_file('data.txt', 'hello data')
39543953

39553954
# Without --obj-output we issue a warning
3956-
err = self.run_process([FILE_PACKAGER, 'test.data', '--embed', 'data.txt', '--js-output=data.js'], stderr=PIPE).stderr
3957-
self.assertContained('--obj-output is recommended when using --embed', err)
3955+
err = self.expect_fail([FILE_PACKAGER, 'test.data', '--embed', 'data.txt', '--js-output=data.js'])
3956+
self.assertContained('error: --obj-output is required when using --embed', err)
39583957

39593958
self.run_process([FILE_PACKAGER, 'test.data', '--embed', 'data.txt', '--obj-output=data.o'])
39603959

@@ -3975,6 +3974,10 @@ def test_file_packager_embed(self):
39753974
output = self.run_js('a.out.js')
39763975
self.assertContained('hello data', output)
39773976

3977+
def test_file_packager_preload_and_embed(self):
3978+
create_file('data.txt', 'hello data')
3979+
self.assert_fail([FILE_PACKAGER, 'test.data', '--embed', 'data.txt', '--preload', 'data.txt'], 'file_packager: error: --preload and --embed are mutually exclusive (See https://github.com/emscripten-core/emscripten/issues/24803)')
3980+
39783981
def test_file_packager_export_es6(self):
39793982
create_file('smth.txt', 'hello data')
39803983
self.run_process([FILE_PACKAGER, 'test.data', '--export-es6', '--preload', 'smth.txt', '--js-output=dataFileLoader.js'])

tools/file_packager.py

Lines changed: 66 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@
6767
subdir\file, in JS it will be subdir/file. For simplicity we treat the web platform as a *NIX.
6868
"""
6969

70-
import base64
7170
import ctypes
7271
import fnmatch
7372
import hashlib
@@ -138,11 +137,6 @@ def err(*args):
138137
print(*args, file=sys.stderr)
139138

140139

141-
def base64_encode(b):
142-
b64 = base64.b64encode(b)
143-
return b64.decode('ascii')
144-
145-
146140
def has_hidden_attribute(filepath):
147141
"""Win32 code to test whether the given file has the hidden property set."""
148142

@@ -455,9 +449,10 @@ def main(): # noqa: C901, PLR0912, PLR0915
455449
options.has_embedded = any(f.mode == 'embed' for f in data_files)
456450

457451
if options.has_preloaded and options.has_embedded:
458-
diagnostics.warn('support for using --preload and --embed in the same command is scheduled '
459-
'for deprecation. If you need this feature please comment at '
460-
'https://github.com/emscripten-core/emscripten/issues/24803')
452+
diagnostics.error('--preload and --embed are mutually exclusive (See https://github.com/emscripten-core/emscripten/issues/24803)')
453+
454+
if options.has_embedded and not options.obj_output:
455+
diagnostics.error('--obj-output is required when using --embed. This outputs an object file for linking directly into your application and is more efficient than the old JS encoding')
461456

462457
if options.separate_metadata and (not options.has_preloaded or not options.jsoutput):
463458
diagnostics.error('cannot separate-metadata without both --preloaded files and a specified --js-output')
@@ -543,8 +538,6 @@ def was_seen(name):
543538
data_files = sorted(data_files, key=lambda file_: file_.dstpath)
544539
data_files = [file_ for file_ in data_files if not was_seen(file_.dstpath)]
545540

546-
metadata = {'files': []}
547-
548541
if options.depfile:
549542
targets = []
550543
if options.obj_output:
@@ -566,26 +559,26 @@ def was_seen(name):
566559
if not options.has_embedded:
567560
diagnostics.error('--obj-output is only applicable when embedding files')
568561
generate_object_file(data_files)
569-
if not options.has_preloaded:
570-
return 0
562+
else:
563+
metadata = {'files': []}
571564

572-
ret = generate_js(data_target, data_files, metadata)
565+
ret = generate_preload_js(data_target, data_files, metadata)
573566

574-
if options.force or data_files:
575-
if options.jsoutput is None:
576-
print(ret)
577-
else:
578-
# Overwrite the old jsoutput file (if exists) only when its content
579-
# differs from the current generated one, otherwise leave the file
580-
# untouched preserving its old timestamp
581-
if os.path.isfile(options.jsoutput):
582-
old = utils.read_file(options.jsoutput)
583-
if old != ret:
584-
utils.write_file(options.jsoutput, ret)
567+
if options.force or data_files:
568+
if options.jsoutput is None:
569+
print(ret)
585570
else:
586-
utils.write_file(options.jsoutput, ret)
587-
if options.separate_metadata:
588-
utils.write_file(options.jsoutput + '.metadata', json.dumps(metadata, separators=(',', ':')))
571+
# Overwrite the old jsoutput file (if exists) only when its content
572+
# differs from the current generated one, otherwise leave the file
573+
# untouched preserving its old timestamp
574+
if os.path.isfile(options.jsoutput):
575+
old = utils.read_file(options.jsoutput)
576+
if old != ret:
577+
utils.write_file(options.jsoutput, ret)
578+
else:
579+
utils.write_file(options.jsoutput, ret)
580+
if options.separate_metadata:
581+
utils.write_file(options.jsoutput + '.metadata', json.dumps(metadata, separators=(',', ':')))
589582

590583
return 0
591584

@@ -598,7 +591,7 @@ def escape_for_makefile(fpath):
598591
return fpath.replace('$', '$$').replace('#', '\\#').replace(' ', '\\ ')
599592

600593

601-
def generate_js(data_target, data_files, metadata):
594+
def generate_preload_js(data_target, data_files, metadata):
602595
# emcc will add this to the output itself, so it is only needed for
603596
# standalone calls
604597
if options.from_emcc:
@@ -646,6 +639,7 @@ def generate_js(data_target, data_files, metadata):
646639
# Set up folders
647640
partial_dirs = []
648641
for file_ in data_files:
642+
assert file_.mode == 'preload'
649643
dirname = os.path.dirname(file_.dstpath)
650644
dirname = dirname.lstrip('/') # absolute paths start with '/', remove that
651645
if dirname != '':
@@ -657,49 +651,45 @@ def generate_js(data_target, data_files, metadata):
657651
% (json.dumps('/' + '/'.join(parts[:i])), json.dumps(parts[i])))
658652
partial_dirs.append(partial)
659653

660-
if options.has_preloaded:
661-
# Bundle all datafiles into one archive. Avoids doing lots of simultaneous
662-
# XHRs which has overhead.
663-
start = 0
664-
with open(data_target, 'wb') as data:
665-
for file_ in data_files:
666-
file_.data_start = start
667-
curr = utils.read_binary(file_.srcpath)
668-
file_.data_end = start + len(curr)
669-
start += len(curr)
670-
data.write(curr)
671-
672-
if start > 256 * 1024 * 1024:
673-
diagnostics.warn('file packager is creating an asset bundle of %d MB. '
674-
'this is very large, and browsers might have trouble loading it. '
675-
'see https://hacks.mozilla.org/2015/02/synchronous-execution-and-filesystem-access-in-emscripten/'
676-
% (start / (1024 * 1024)))
677-
678-
create_preloaded = '''
679-
try {
680-
// canOwn this data in the filesystem, it is a slice into the heap that will never change
681-
await Module['FS_preloadFile'](name, null, data, true, true, false, true);
682-
Module['removeRunDependency'](`fp ${name}`);
683-
} catch (e) {
684-
err(`Preloading file ${name} failed`, e);
685-
}\n'''
686-
create_data = '''// canOwn this data in the filesystem, it is a slice into the heap that will never change
687-
Module['FS_createDataFile'](name, null, data, true, true, true);
688-
Module['removeRunDependency'](`fp ${name}`);'''
689-
690-
finish_handler = create_preloaded if options.use_preload_plugins else create_data
654+
# Bundle all datafiles into one archive. Avoids doing lots of simultaneous
655+
# XHRs which has overhead.
656+
start = 0
657+
with open(data_target, 'wb') as data:
658+
for file_ in data_files:
659+
file_.data_start = start
660+
curr = utils.read_binary(file_.srcpath)
661+
file_.data_end = start + len(curr)
662+
start += len(curr)
663+
data.write(curr)
664+
665+
if start > 256 * 1024 * 1024:
666+
diagnostics.warn('file packager is creating an asset bundle of %d MB. '
667+
'this is very large, and browsers might have trouble loading it. '
668+
'see https://hacks.mozilla.org/2015/02/synchronous-execution-and-filesystem-access-in-emscripten/'
669+
% (start / (1024 * 1024)))
670+
671+
create_preloaded = '''
672+
try {
673+
// canOwn this data in the filesystem, it is a slice into the heap that will never change
674+
await Module['FS_preloadFile'](name, null, data, true, true, false, true);
675+
Module['removeRunDependency'](`fp ${name}`);
676+
} catch (e) {
677+
err(`Preloading file ${name} failed`, e);
678+
}\n'''
679+
create_data = '''// canOwn this data in the filesystem, it is a slice into the heap that will never change
680+
Module['FS_createDataFile'](name, null, data, true, true, true);
681+
Module['removeRunDependency'](`fp ${name}`);'''
691682

692-
if not options.lz4:
693-
# Data requests - for getting a block of data out of the big archive - have
694-
# a similar API to XHRs
695-
code += '''
696-
for (var file of metadata['files']) {
697-
var name = file['filename']
698-
Module['addRunDependency'](`fp ${name}`);
699-
}\n'''
683+
finish_handler = create_preloaded if options.use_preload_plugins else create_data
700684

701-
if options.has_embedded and not options.obj_output:
702-
diagnostics.warn('--obj-output is recommended when using --embed. This outputs an object file for linking directly into your application is more efficient than JS encoding')
685+
if not options.lz4:
686+
# Data requests - for getting a block of data out of the big archive - have
687+
# a similar API to XHRs
688+
code += '''
689+
for (var file of metadata['files']) {
690+
var name = file['filename']
691+
Module['addRunDependency'](`fp ${name}`);
692+
}\n'''
703693

704694
catch_handler = ''
705695
if options.export_es6:
@@ -708,27 +698,14 @@ def generate_js(data_target, data_files, metadata):
708698
loadDataReject(error);
709699
})'''
710700

711-
for counter, file_ in enumerate(data_files):
701+
for file_ in data_files:
712702
filename = file_.dstpath
713703
dirname = os.path.dirname(filename)
714-
basename = os.path.basename(filename)
715-
if file_.mode == 'embed':
716-
if not options.obj_output:
717-
# Embed (only needed when not generating object file output)
718-
data = base64_encode(utils.read_binary(file_.srcpath))
719-
code += " var fileData%d = '%s';\n" % (counter, data)
720-
# canOwn this data in the filesystem (i.e. there is no need to create a copy in the FS layer).
721-
code += (" Module['FS_createDataFile']('%s', '%s', atob(fileData%d), true, true, true);\n"
722-
% (dirname, basename, counter))
723-
elif file_.mode == 'preload':
724-
# Preload
725-
metadata['files'].append({
726-
'filename': file_.dstpath,
727-
'start': file_.data_start,
728-
'end': file_.data_end,
729-
})
730-
else:
731-
assert 0
704+
metadata['files'].append({
705+
'filename': file_.dstpath,
706+
'start': file_.data_start,
707+
'end': file_.data_end,
708+
})
732709

733710
if options.has_preloaded:
734711
if not options.lz4:

0 commit comments

Comments
 (0)