[PATCH] D65819: [Driver][Bundler] Improve bundling of object files.

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld requested changes to this revision.
Hahnfeld added a comment.
This revision now requires changes to proceed.

The code changes look good to me, but the test doesn't pass on x86. We've faced the same problem when `clang-offload-bundler` was initially committed and the current testing is the best we were able to do.



================
Comment at: test/Driver/clang-offload-bundler.c:223-227
 // Check object bundle/unbundle. The content should be bundled into an ELF
 // section (we are using a PowerPC little-endian host which uses ELF). We
 // have an already bundled file to check the unbundle and do a dry run on the
 // bundling as it cannot be tested in all host platforms that will run these
 // tests.
----------------
This still holds: I can't partially link PPC object files on x86_64. Please revert the test changes to not actually perform the bundling.


================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:373-375
 /// use incremental linking to produce the resulting object. We also add section
 /// with a single byte to state the name of the component the main object file
 /// (the one we are bundling into) refers to.
----------------
This isn't true anymore.


================
Comment at: tools/clang-offload-bundler/ClangOffloadBundler.cpp:377
 ///
-/// To unbundle, we use just copy the contents of the designated section. If the
-/// requested bundle refer to the main object file, we just copy it with no
-/// changes.
+/// To unbundle, we use just copy the contents of the designated section.
 class ObjectFileHandler final : public FileHandler {
----------------
I know this has been wrong before, but can you please fix to `we just copy` without `use`?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65819/new/

https://reviews.llvm.org/D65819