Differential D3784
Make script shebangs POSIX friendly Authored by • jon on Apr 19 2022, 9:55 PM. Tags None Referenced Files
Details
Sourcing bash from /bin/bash doesn't work on NixOS, and on other platforms it ends up using the system-default bash rather than the one sourced by Nix. /usr/bin/env is a utility which will find the related command by searching PATH, this allows for nix to introduce other interpreters. It's also considered a "best practice" as some systems such as android, solaris, and other may also not put their default interpreters in /bin Depends on D3274 This shouldn't invalidate previous behavior of scripts, as most invocations will resolve to /bin/bash
Diff Detail
Event TimelineComment Actions This seems uncontroversial, so I'll accept. But it would've been good to provide more context about why this change is necessary. I Googled and found this and this, which explain that #!/usr/bin/env bash uses whatever bash is in $PATH. But there is still some missing context here. I assume the issue is that #!/bin/bash circumvents Nix by using a direct path to something outside of Nix. Ideally I wouldn't need to Google here at all, and ideally I wouldn't have to make assumptions about the purpose of the diff. As a diff author, your job should be to make diffs as easy as possible to review. Work should be shifted away from the reviewer, to the author. The fact that I wrote multiple sentences here, compared to <10 words in the diff description and test plan... it should be a strong signal to you that you aren't doing enough to explain your changes, and forcing a lot of work on your reviewer. Ideally, I accept without any comments at all because you've done the work to anticipate my questions ahead of time. Comment Actions (Separately, I should never be a first-pass reviewer like this – @varun would've probably been the right choice here, as he is working with you to help on the Nix stuff) |