r-wasm / rwasm

Build R packages for WebAssembly and create a CRAN-like repo for distribution.
https://r-wasm.github.io/rwasm/
Other
54 stars 4 forks source link

Package build fails for configure script without shebang #7

Closed jeroen closed 9 months ago

jeroen commented 9 months ago

Packages can have a configure script without any shebang, in this case it should just execute it with the default shell (the safest option is probably bash). Currently it seems to error, for example the openssl package:

> rwasm::build('./openssl_2.1.1.tar.gz')
v Updated metadata database: 6.28 MB in 13 files.
v Updating metadata database ... done

> Will install 2 packages.
> Will download 3 packages with unknown size.
+ askpass   1.2.0 [dl]
+ sys       3.4.2 [dl]
v All system requirements are already installed.

i Getting 2 pkgs with unknown sizes
v Got askpass 1.2.0 (x86_64-pc-linux-gnu-ubuntu-22.04) (21.57 kB)
v Got sys 3.4.2 (x86_64-pc-linux-gnu-ubuntu-22.04) (39.83 kB)
v Downloaded 2 packages (61.40 kB) in 782ms
v Installed askpass 1.2.0  (48ms)
v Installed sys 3.4.2  (69ms)
v 2 deps: added 2, dld 2 (61.40 kB) [16.1s]
* installing *source* package 'openssl' ...
file 'configure' has the wrong MD5 checksum
** using non-staged installation
configure: ./configure.orig --host=wasm32-unknown-emscripten
emconfigure: error: './configure.orig --host=wasm32-unknown-emscripten' failed: [Errno 8] Exec format error: './configure.orig'
ERROR: configuration failed for package 'openssl'
* removing '/tmp/Rtmp8yvnLV/filec1a054c45/openssl'
georgestagg commented 9 months ago

Thanks, I think this comes from Emscripten's emconfigure launching the process using subprocess.run without shell = True or similar.

We might be able to work around it by tweaking rwasm to specify the shell when wrapping configure scripts, with

emconfigure sh ./configure.orig --host=wasm32-unknown-emscripten
jeroen commented 9 months ago

Yes, that works. I think using bash if possible is probably more robust (or we could check $SHELL).

georgestagg commented 9 months ago

I'd prefer sh, but it's probably fine. It just means we'd be relying on bash being installed and on the PATH, which is likely true in almost all cases on Unix anyway and assumed by R already.

My thought process was that if a particular script wants to use e.g. csh- or bash-specific features it could have its own shebang which will take over, otherwise scripts without a shebang would be restricted to strictly POSIX compliant sh, but if R already assumes configure is a bash-compatible scripts we should support that here.

jeroen commented 9 months ago

Yes you are probably right. Cran does not allow for a non standard shebang such as bash, but i have certainly encountered packages that would only work well with bash. But maybe a better solution for this is to make bash the default shell in my docker image.

Feel free to change it 🙂

Op di 12 dec. 2023 16:12 schreef George Stagg @.***>:

I'd prefer sh, but it's probably fine. It just means we'd be relying on bash being installed and on the PATH, which is likely true in almost all cases on Unix anyway and assumed by R already.

My thought process was that if a particular script wants to use e.g. csh- or bash-specific features it could have its own shebang which will take over, otherwise scripts without a shebang would be restricted to strictly POSIX compliant sh.

— Reply to this email directly, view it on GitHub https://github.com/r-wasm/rwasm/issues/7#issuecomment-1852234337, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABUZ73DL26TF5CQUU7Q653YJBX7DAVCNFSM6AAAAABARR2TFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJSGIZTIMZTG4 . You are receiving this because you authored the thread.Message ID: @.***>

georgestagg commented 9 months ago

Okay, I'll switch to sh. We can always put bash back if it causes any serious problems.

jeroen commented 9 months ago

Sounds good, thanks for the quick fix.