Page MenuHomePhabricator

[services] Change sourcing bash in scripts
ClosedPublic

Authored by karol on May 10 2022, 2:30 AM.
Tags
None
Referenced Files
F3281745: D3981.id12531.diff
Sat, Nov 16, 10:08 AM
F3281744: D3981.id.diff
Sat, Nov 16, 10:07 AM
Unknown Object (File)
Thu, Nov 14, 9:09 AM
Unknown Object (File)
Thu, Nov 14, 9:09 AM
Unknown Object (File)
Sat, Nov 9, 11:40 AM
Unknown Object (File)
Sat, Nov 9, 11:40 AM
Unknown Object (File)
Sat, Nov 9, 11:40 AM
Unknown Object (File)
Fri, Nov 8, 8:06 AM

Details

Summary
Test Plan
cd services
yarn test-backup-service-dev-mode
yarn test-blob-service-dev-mode
cd ..
git grep "#\!/bin/bash" # should be empty

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D3981
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

karol edited the test plan for this revision. (Show Details)
karol added a reviewer: tomek.

I also spotted a couple of places beyond services where this occurs:

native/android/app/bash/build_openssl.sh:#!/bin/bash
native/android/app/bash/detect_abis.sh:#!/bin/bash
native/scripts/mark-generated.sh:#!/bin/bash

Should we change this as well or is this just for services? @jonringer-comm @ashoat

Yeah I think this is probably always a good idea, since it makes it possible to run these scripts from within Nix. Nix creates it's own path /nix in your system with all of its binaries, and sources its bash into $PATH... so we can use the Nix bash by accessing #!/usr/bin/env bash (which checks $PATH for bash), but if we use #!/bin/bash we are using the older system-default bash on MacOS.

I think the original reason for @jonringer-comm's diff was that NixOS (Linux distro based on Nix) doesn't even have /bin/bash, so the scripts would fail to run there.

This revision is now accepted and ready to land.May 10 2022, 6:15 AM
In D3981#111570, @karol-bisztyga wrote:

I also spotted a couple of places beyond services where this occurs:

native/android/app/bash/build_openssl.sh:#!/bin/bash
native/android/app/bash/detect_abis.sh:#!/bin/bash
native/scripts/mark-generated.sh:#!/bin/bash

Should we change this as well or is this just for services? @jonringer-comm @ashoat

Let's update all of these so they work on NixOS

karol edited the summary of this revision. (Show Details)

add scripts from beyond the services

karol edited the test plan for this revision. (Show Details)