Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

closures does not work in system without unzip #31

Open
guibou opened this issue Feb 28, 2019 · 7 comments
Open

closures does not work in system without unzip #31

guibou opened this issue Feb 28, 2019 · 7 comments

Comments

@guibou
Copy link

guibou commented Feb 28, 2019

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
Copy link
Member

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
Copy link
Author

guibou commented Mar 1, 2019

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.

@guibou guibou changed the title closures does not work out of nix-shell closures does not work in system without unzip Mar 1, 2019
@facundominguez
Copy link
Member

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
Copy link
Author

guibou commented Mar 1, 2019

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
Copy link
Member

mboes commented Mar 2, 2019 via email

@facundominguez
Copy link
Member

facundominguez commented Mar 3, 2019

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants