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

Fix build on NixOS #70

Merged
merged 2 commits into from
Oct 14, 2024
Merged

Fix build on NixOS #70

merged 2 commits into from
Oct 14, 2024

Conversation

leo60228
Copy link
Contributor

Closes #62

@@ -15,7 +15,7 @@ export AR := $(PREFIX)ar
export AS := $(PREFIX)as
endif

SHELL := /bin/bash -o pipefail
SHELL := bash -o pipefail
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A word of caution, this assumes that bash is on the PATH. Reasonably safe assumption, but still worth being aware of.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imho it's as fragile to depend on bash being on the PATH as it is to assume that it's available as /bin/bash

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The makefile already assumes that other tools like rm are on the path. These are likely to be installed in /bin, implying that this change is extremely unlikely to regress behavior.

Even ignoring that, I don't think I've ever run into a realistic build environment where bash is installed but not on the path. On the other hand, there are many reasons why it wouldn't be in /bin (NixOS, but also for example if it was installed in /usr/local).

@RevoSucks RevoSucks merged commit cd0ed12 into pret:master Oct 14, 2024
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

Successfully merging this pull request may close these issues.

Build script fails on NixOS
4 participants