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

change shebang to be more compatible #2583

Closed
wucke13 opened this issue Aug 20, 2023 · 3 comments · Fixed by #2587
Closed

change shebang to be more compatible #2583

wucke13 opened this issue Aug 20, 2023 · 3 comments · Fixed by #2587

Comments

@wucke13
Copy link
Contributor

wucke13 commented Aug 20, 2023

This

#!/bin/bash

should be

#!/usr/bin/env bash

in order to maximize compatibility. On many more nieche systems /bin/bash does not exist (/bin/sh would be fine!), but /usr/bin/env and env would be able to find bash on most if not all systems if bash is present at all.

@oli-obk
Copy link
Member

oli-obk commented Aug 20, 2023

Good catch. I'd merge a PR that changes this

@wucke13
Copy link
Contributor Author

wucke13 commented Aug 20, 2023

No problem. Another friction point with the script is that it only works with a rustup cargo, not with native cargo (due to reliance on +nightly). I don't know how to robustly detect if cargo is infact rustup:

+ cargo +nightly build --manifest-path bin/Cargo.toml --bin serde_derive --profile precompiled -Z unstable-options -Z build-std=std,panic_abort -Z build-std-features=panic_immediate_abort --target x86_64-unknown-linux-musl --out-dir serde_derive
error: no such command: `+nightly`

	Cargo does not handle `+toolchain` directives.
	Did you mean to invoke `cargo` through `rustup` instead?

wucke13 added a commit to wucke13/serde that referenced this issue Aug 20, 2023
@oli-obk
Copy link
Member

oli-obk commented Aug 20, 2023

I think the rustup issue will be fixed by whatever resolves #2575

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

Successfully merging a pull request may close this issue.

2 participants