Page MenuHomePhabricator

[services] Change sourcing bash in scripts
ClosedPublic

Authored by karol on May 10 2022, 2:30 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, May 29, 3:21 PM
Unknown Object (File)
Thu, May 23, 1:10 AM
Unknown Object (File)
Sat, May 18, 7:03 AM
Unknown Object (File)
Apr 6 2024, 10:31 AM
Unknown Object (File)
Apr 3 2024, 12:33 PM
Unknown Object (File)
Apr 3 2024, 12:22 PM
Unknown Object (File)
Mar 26 2024, 4:35 AM
Unknown Object (File)
Mar 13 2024, 9:14 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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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)