machine-learning-exchange/mlx

sed command in helper scripts breaks on some platforms

ckadner opened this issue · 2 comments

The sed command is used in many of our helper scripts. However sed preinstalled on macOS is not POSIX compliant (works differently than on most Linux distributions). But testing for the OS on which the script is being run on is not a good proxy to find out which version of sed is going to be available since macOS users can easily reinstall it (Homebrew, symlink, ...). On top of that, some Bash emulators used on Windows do not present as an operating system.

We need to replace this test for specific operating systems (or specific Bash emulator):

if [[ "$OSTYPE" == "linux-gnu"* ]]; then # Linux
alias gsed="sed -i"
elif [[ "$OSTYPE" == "darwin"* ]]; then # macOS
alias gsed="sed -i ''"
elif [[ "$OSTYPE" == "cygwin" ]]; then # POSIX compatible emulation for Windows
alias gsed="sed -i"
elif [[ "$OSTYPE" == "msys"* ]]; then # Git Bash (Windows)
alias gsed="sed -i"
else
echo "FAILED. OS not compatible with script '${BASH_SOURCE[0]}'"
exit 1
fi
export gsed

With this more generic test:

if ! sed -i '1s/^/test/' $(mktemp) 2> /dev/null; then
    # macOS (BSD) version of sed
    alias gsed="sed -i ''" 
else
    # POSIX compliant version of sed 
    alias gsed="sed -i"
fi

sed is used in various script in the mlx and katalog repo where the platform check needs to be replaced:

  • in the mlx repo:
    • mlx/api/codegen.sh
    • mlx/tools/bash/add_license_headers.sh
  • in the katalog repo:
    • katalog/tools/bash/upgrade_notebook_requirements.sh
    • katalog/tools/bash/add_license_headers.sh

@ckadner I can look into this..

Thanks @krishnakumar27 -- let me know if you need more information