[file-packager] Drop support for --embed with JS output

test/test_other.py

@@ -3932,8 +3932,8 @@ def test_file_packager_embed(self):
     create_file('data.txt', 'hello data')
 
     # Without --obj-output we issue a warning
-    err = self.run_process([FILE_PACKAGER, 'test.data', '--embed', 'data.txt', '--js-output=data.js'], stderr=PIPE).stderr
-    self.assertContained('--obj-output is recommended when using --embed', err)
+    err = self.expect_fail([FILE_PACKAGER, 'test.data', '--embed', 'data.txt', '--js-output=data.js'])
+    self.assertContained('error: --obj-output is now required when using --embed', err)
 
     self.run_process([FILE_PACKAGER, 'test.data', '--embed', 'data.txt', '--obj-output=data.o'])
 

tools/file_packager.py

@@ -455,9 +455,10 @@ def main():  # noqa: C901, PLR0912, PLR0915
   options.has_embedded = any(f.mode == 'embed' for f in data_files)
 
   if options.has_preloaded and options.has_embedded:
-    diagnostics.warn('support for using --preload and --embed in the same command is scheduled '
-        'for deprecation.  If you need this feature please comment at '
-        'https://github.com/emscripten-core/emscripten/issues/24803')
+    diagnostics.error('--preload and --embed now now mutually exclusive (See https://github.com/emscripten-core/emscripten/issues/24803')
+
+  if options.has_embedded and not options.obj_output:
+    diagnostics.error('--obj-output is now required when using --embed.  This outputs an object file for linking directly into your application is more efficient than the old JS encoding')
 
   if options.separate_metadata and (not options.has_preloaded or not options.jsoutput):
     diagnostics.error('cannot separate-metadata without both --preloaded files and a specified --js-output')
@@ -543,36 +544,37 @@ def was_seen(name):
   data_files = sorted(data_files, key=lambda file_: file_.dstpath)
   data_files = [file_ for file_ in data_files if not was_seen(file_.dstpath)]
 
-  metadata = {'files': []}
-
   if options.obj_output:
     if not options.has_embedded:
       diagnostics.error('--obj-output is only applicable when embedding files')
     generate_object_file(data_files)
-    if not options.has_preloaded:
-      return 0
+  else:
+    metadata = {'files': []}
 
-  ret = generate_js(data_target, data_files, metadata)
+    ret = generate_preload_js(data_target, data_files, metadata)
 
-  if options.force or len(data_files):
-    if options.jsoutput is None:
-      print(ret)
-    else:
-      # Overwrite the old jsoutput file (if exists) only when its content
-      # differs from the current generated one, otherwise leave the file
-      # untouched preserving its old timestamp
-      if os.path.isfile(options.jsoutput):
-        old = utils.read_file(options.jsoutput)
-        if old != ret:
-          utils.write_file(options.jsoutput, ret)
+    if options.force or len(data_files):
+      if options.jsoutput is None:
+        print(ret)
       else:
-        utils.write_file(options.jsoutput, ret)
-      if options.separate_metadata:
-        utils.write_file(options.jsoutput + '.metadata', json.dumps(metadata, separators=(',', ':')))
+        # Overwrite the old jsoutput file (if exists) only when its content
+        # differs from the current generated one, otherwise leave the file
+        # untouched preserving its old timestamp
+        if os.path.isfile(options.jsoutput):
+          old = utils.read_file(options.jsoutput)
+          if old != ret:
+            utils.write_file(options.jsoutput, ret)
+        else:
+          utils.write_file(options.jsoutput, ret)
+        if options.separate_metadata:
+          utils.write_file(options.jsoutput + '.metadata', json.dumps(metadata, separators=(',', ':')))
 
   if options.depfile:
+    output = options.jsoutput
+    if options.obj_output:
+      output = options.obj_output
     with open(options.depfile, 'w') as f:
-      for target in (data_target, options.jsoutput):
+      for target in (data_target, output):
         if target:
           f.write(escape_for_makefile(target))
           f.write(' \\\n')
@@ -592,7 +594,7 @@ def escape_for_makefile(fpath):
   return fpath.replace('$', '$$').replace('#', '\\#').replace(' ', '\\ ')
 
 
-def generate_js(data_target, data_files, metadata):
+def generate_preload_js(data_target, data_files, metadata):
   # emcc will add this to the output itself, so it is only needed for
   # standalone calls
   if options.from_emcc:
@@ -640,6 +642,7 @@ def generate_js(data_target, data_files, metadata):
   # Set up folders
   partial_dirs = []
   for file_ in data_files:
+    assert file_.mode == 'preload'
     dirname = os.path.dirname(file_.dstpath)
     dirname = dirname.lstrip('/') # absolute paths start with '/', remove that
     if dirname != '':
@@ -651,49 +654,45 @@ def generate_js(data_target, data_files, metadata):
                    % (json.dumps('/' + '/'.join(parts[:i])), json.dumps(parts[i])))
           partial_dirs.append(partial)
 
-  if options.has_preloaded:
-    # Bundle all datafiles into one archive. Avoids doing lots of simultaneous
-    # XHRs which has overhead.
-    start = 0
-    with open(data_target, 'wb') as data:
-      for file_ in data_files:
-        file_.data_start = start
-        curr = utils.read_binary(file_.srcpath)
-        file_.data_end = start + len(curr)
-        start += len(curr)
-        data.write(curr)
-
-    if start > 256 * 1024 * 1024:
-      diagnostics.warn('file packager is creating an asset bundle of %d MB. '
-          'this is very large, and browsers might have trouble loading it. '
-          'see https://hacks.mozilla.org/2015/02/synchronous-execution-and-filesystem-access-in-emscripten/'
-          % (start / (1024 * 1024)))
-
-    create_preloaded = '''
-          try {
-            // canOwn this data in the filesystem, it is a slice into the heap that will never change
-            await Module['FS_preloadFile'](name, null, data, true, true, false, true);
-            Module['removeRunDependency'](`fp ${name}`);
-          } catch (e) {
-            err(`Preloading file ${name} failed`);
-          }\n'''
-    create_data = '''// canOwn this data in the filesystem, it is a slice into the heap that will never change
-          Module['FS_createDataFile'](name, null, data, true, true, true);
-          Module['removeRunDependency'](`fp ${name}`);'''
-
-    finish_handler = create_preloaded if options.use_preload_plugins else create_data
+  # Bundle all datafiles into one archive. Avoids doing lots of simultaneous
+  # XHRs which has overhead.
+  start = 0
+  with open(data_target, 'wb') as data:
+    for file_ in data_files:
+      file_.data_start = start
+      curr = utils.read_binary(file_.srcpath)
+      file_.data_end = start + len(curr)
+      start += len(curr)
+      data.write(curr)
+
+  if start > 256 * 1024 * 1024:
+    diagnostics.warn('file packager is creating an asset bundle of %d MB. '
+        'this is very large, and browsers might have trouble loading it. '
+        'see https://hacks.mozilla.org/2015/02/synchronous-execution-and-filesystem-access-in-emscripten/'
+        % (start / (1024 * 1024)))
+
+  create_preloaded = '''
+        try {
+          // canOwn this data in the filesystem, it is a slice into the heap that will never change
+          await Module['FS_preloadFile'](name, null, data, true, true, false, true);
+          Module['removeRunDependency'](`fp ${name}`);
+        } catch (e) {
+          err(`Preloading file ${name} failed`);
+        }\n'''
+  create_data = '''// canOwn this data in the filesystem, it is a slice into the heap that will never change
+        Module['FS_createDataFile'](name, null, data, true, true, true);
+        Module['removeRunDependency'](`fp ${name}`);'''
 
-    if not options.lz4:
-      # Data requests - for getting a block of data out of the big archive - have
-      # a similar API to XHRs
-      code += '''
-      for (var file of metadata['files']) {
-        var name = file['filename']
-        Module['addRunDependency'](`fp ${name}`);
-      }\n'''
+  finish_handler = create_preloaded if options.use_preload_plugins else create_data
 
-  if options.has_embedded and not options.obj_output:
-    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')
+  if not options.lz4:
+    # Data requests - for getting a block of data out of the big archive - have
+    # a similar API to XHRs
+    code += '''
+    for (var file of metadata['files']) {
+      var name = file['filename']
+      Module['addRunDependency'](`fp ${name}`);
+    }\n'''
 
   catch_handler = ''
   if options.export_es6:
@@ -702,27 +701,14 @@ def generate_js(data_target, data_files, metadata):
           loadDataReject(error);
         })'''
 
-  for counter, file_ in enumerate(data_files):
+  for file_ in data_files:
     filename = file_.dstpath
     dirname = os.path.dirname(filename)
-    basename = os.path.basename(filename)
-    if file_.mode == 'embed':
-      if not options.obj_output:
-        # Embed (only needed when not generating object file output)
-        data = base64_encode(utils.read_binary(file_.srcpath))
-        code += "      var fileData%d = '%s';\n" % (counter, data)
-        # canOwn this data in the filesystem (i.e. there is no need to create a copy in the FS layer).
-        code += ("      Module['FS_createDataFile']('%s', '%s', atob(fileData%d), true, true, true);\n"
-                 % (dirname, basename, counter))
-    elif file_.mode == 'preload':
-      # Preload
-      metadata['files'].append({
-        'filename': file_.dstpath,
-        'start': file_.data_start,
-        'end': file_.data_end,
-      })
-    else:
-      assert 0
+    metadata['files'].append({
+      'filename': file_.dstpath,
+      'start': file_.data_start,
+      'end': file_.data_end,
+    })
 
   if options.has_preloaded:
     if not options.lz4:
@@ -768,14 +754,13 @@ def generate_js(data_target, data_files, metadata):
     metadata['remote_package_size'] = remote_package_size
     ret += "      var REMOTE_PACKAGE_SIZE = metadata['remote_package_size'];\n"
 
-    if options.use_preload_cache:
-      # Set the id to a hash of the preloaded data, so that caches survive over multiple builds
-      # if the data has not changed.
-      data = utils.read_binary(data_target)
-      package_uuid = 'sha256-' + hashlib.sha256(data).hexdigest()
-      metadata['package_uuid'] = str(package_uuid)
+    # Set the id to a hash of the preloaded data, so that caches survive over multiple builds
+    # if the data has not changed.
+    data = utils.read_binary(data_target)
+    package_uuid = 'sha256-' + hashlib.sha256(data).hexdigest()
+    metadata['package_uuid'] = str(package_uuid)
 
-      code += r'''
+    code += r'''
         var PACKAGE_UUID = metadata['package_uuid'];
         var IDB_RO = "readonly";
         var IDB_RW = "readwrite";