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

closures does not work in system without `unzip` #31

Open guibou opened 5 years ago

guibou commented 5 years ago

From current master:

Build works

# in git checkout of clodl
$ nix-shell
[nix-shell:~/tweag/clodl]$ bazel build //:clotestbin-cc-pie
INFO: Analysed target //:clotestbin-cc-pie (0 packages loaded).
INFO: Found 1 target...
Target //:clotestbin-cc-pie up-to-date:
  bazel-genfiles/clotestbin-cc-pie.sh
INFO: Elapsed time: 0.416s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action

Execution in the shell works:

[nix-shell:~/tweag/clodl]$ bazel-genfiles/clotestbin-cc-pie.sh
hello cc

However, if I'm leaving the shell, the execution does not work anymore:

[nix-shell:~/tweag/clodl]$ exit
$ bazel-genfiles/clotestbin-cc-pie.sh                                           
bazel-genfiles/clotestbin-cc-pie.sh: line 6: /tmp/tmp.cO8LRRZISw/clotestbin-cc-pie-closure_wrapper: No such file or directory

It may come from the behavior of mktemp -d which is different between inside and outside of the nix shell:

# In the shell
[nix-shell:~/tweag/clodl]$ mktemp -d
/run/user/1000/tmp.AQlPgn2GIW

# Outside of the nix shell
$ mktemp -d
/tmp/tmp.EcwXFl3PpA

I'm using nixos (from nixos-unstable) as my os.

facundominguez commented 5 years ago

I tried it in centos and it seems to work. To produce the following output, I edited bazel-genfiles/clotestbin-cc-pie.sh as follows:

    #!/usr/bin/env bash
-   set -eu
+   set -eux
    tmpdir=$(mktemp -d)
    trap "rm -rf '$tmpdir'" EXIT
    unzip -q "$0" -d "$tmpdir" 2> /dev/null || true
    "$tmpdir/clotestbin-cc-pie-closure_wrapper"
    exit 0
$ bazel-genfiles/clotestbin-cc-pie.sh
+++ mktemp -d
++ tmpdir=/tmp/tmp.LrouJaUrK4
++ trap 'rm -rf '\''/tmp/tmp.LrouJaUrK4'\''' EXIT
++ unzip -q bazel-genfiles/clotestbin-cc-pie.sh -d /tmp/tmp.LrouJaUrK4
++ true
++ /tmp/tmp.LrouJaUrK4/clotestbin-cc-pie-closure_wrapper
hello cc
++ exit 0
++ rm -rf /tmp/tmp.LrouJaUrK4

If the script can't find /tmp/tmp.LrouJaUrK4/clotestbin-cc-pie-closure_wrapper as it says, the most likely cause is that unzip -q bazel-genfiles/clotestbin-cc-pie.sh -d /tmp/tmp.LrouJaUrK4 doesn't output the files where we expect.

guibou commented 5 years ago

You are right. That's just the missing unzip. I'm changing the title to reflact that clodl closures are not working on system without unzip.

facundominguez commented 5 years ago

Sounds good. Do you have any proposals for a fix?

We could check if unzip is present and produce a better error message. Is this what you expect?

guibou commented 5 years ago

It would be better yes.

Why using unzip? Isn't tar or gunzip more portable? I've tested on docker containers, tar and gunzip are always present in ubuntu, debian, arch, centos and nixos. However unzip was not present in any of the docker image I pulled for theses distributions.

mboes commented 5 years ago

See the announcement blog post. Clodl uses zip files because that is the archive format used for JARs. The tools you propose are for different formats.

facundominguez commented 5 years ago

Clodl uses zip, the same as jars. But we never use clodl to build a jar. At least sparkle creates the jars using java_library, not clodl.

@guibou if you need another format, it could be added as an option. If you want to eliminate zip, then we would need to teach sparkle to extract the new format, but it is probably not worth the trouble.

thufschmitt commented 5 years ago

Having clodl output tar archives would be really handy when using it to build containers since the container_image rule from rules_docker can directly take a tgz as input. Atm the way I use it is that I have a genrule which unzip the clodl-generated archive and re-packs it with tar, but that's far from optimal