tweag / clodl

Turn dynamically linked ELF binaries and libraries into self-contained closures.
BSD 3-Clause "New" or "Revised" License
170 stars 6 forks source link

Simplify clodl implementation #36

Closed facundominguez closed 3 years ago

facundominguez commented 3 years ago

So far, library_closure was implemented with a bunch of rules each trying to do a small piece of work. It turns out, however, that the composition of these rules introduced more issues than it solved.

  1. There was a rule to rename inputs so they looked like libraries to the cc_binary rule that we had.
  2. There was a rule to collect the paths of dependencies of a set of source libraries.
  3. There was a rule to transform the collected paths into a couple of files with arguments for the linker.
  4. There was a rule to output the runfiles of the source libraries.
  5. There was a cc_binary rule to link all the runfiles and the source libraries.
  6. There was a final rule to put all the libraries in a zip file.

Each step had its own set of quirks.

All these quirks together made challenging to fix an issue without introducing new ones. Therefore this PR, which implements library_closure on a single rule.

Firstly, there is some setup to collect the paths to required tools like the C compiler, scanelf and ldd. Then we collect the dependencies, link them together, and build the zip file in a single run_shell action. Thus, no bazel constraint crops up between these tasks. There is no longer a composition problem.

Additionally, I dropped support for the outzip attribute of library_closure that would allow the caller to choose the name of the output zip file. This was a feature supporting sparkle that isn't hard to address in sparkle itself.

facundominguez commented 3 years ago

cc @mboes

facundominguez commented 3 years ago

I tested this with sparkle already and it works.