actions / runner-images

GitHub Actions runner images
MIT License
10.17k stars 3.06k forks source link

Mac OS runner with `bash -el {0}` overwrites existing PATH #10624

Closed hoxbro closed 1 month ago

hoxbro commented 1 month ago

Description

Mac OS runner with bash -el {0} overwrites existing PATH

Platforms affected

Runner images affected

Image version and build link

20240911.3

https://github.com/hoxbro/example-repo/actions/runs/10879060685/job/30183040885

Is it regression?

yes, 20240903.5

Expected behavior

It should only append to PATH

Actual behavior

Using bash -el {0} overwrites existing PATH. This started happening with 20240911.3, because of this line:

PATH="/opt/homebrew/bin:/opt/homebrew/sbin:/Users/runner/.cargo/bin:/usr/local/opt/curl/bin:/usr/local/bin:/usr/local/sbin:/Users/runner/bin:/Users/runner/.yarn/bin:/Users/runner/Library/Android/sdk/tools:/Users/runner/Library/Android/sdk/platform-tools:/Library/Frameworks/Python.framework/Versions/Current/bin:/Library/Frameworks/Mono.framework/Versions/Current/Commands:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin"; export PATH;

Repro steps

I have an MRE repo here: https://github.com/hoxbro/example-repo/actions/runs/10879060685. This used to work on macOS and currently works on Windows and Linux.

pavelzw commented 1 month ago

because of this line

*in ~/.bash_profile

erik-bershel commented 1 month ago

Hey @hoxbro!

I see that one of your steps changed PATH in an uncommon way. Please check PATH before running your actions.

Default macOS-14-arm64 PATH:

PATH=/opt/homebrew/lib/ruby/gems/3.0.0/bin:/opt/homebrew/opt/ruby@3.0/bin:/Users/runner/.local/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/Users/runner/.cargo/bin:/usr/local/opt/curl/bin:/usr/local/bin:/usr/local/sbin:/Users/runner/bin:/Users/runner/.yarn/bin:/Users/runner/Library/Android/sdk/tools:/Users/runner/Library/Android/sdk/platform-tools:/Library/Frameworks/Python.framework/Versions/Current/bin:/Library/Frameworks/Mono.framework/Versions/Current/Commands:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Users/runner/.dotnet/tools

Example PATH from your workflow:

PATH="/opt/homebrew/bin:/opt/homebrew/sbin:/Users/runner/.cargo/bin:/usr/local/opt/curl/bin:/usr/local/bin:/usr/local/sbin:/Users/runner/bin:/Users/runner/.yarn/bin:/Users/runner/Library/Android/sdk/tools:/Users/runner/Library/Android/sdk/platform-tools:/Library/Frameworks/Python.framework/Versions/Current/bin:/Library/Frameworks/Mono.framework/Versions/Current/Commands:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin"; export PATH;
hoxbro commented 1 month ago

The PATH is coming directly from the runner:

https://github.com/hoxbro/example-repo/actions/runs/10888459012/job/30212997311#step:2:44

erik-bershel commented 1 month ago

The PATH is coming directly from the runner:

Yeah, my bad. Found the difference. A bit unexpected exactly.

gcampbell-msft commented 1 month ago

Tracking this, looking forward to a fix, thanks!

erik-bershel commented 1 month ago

šŸ¤” It looks like the problem occurs during the installation of Homebrew. If you look at the following variables in the file, you can see that they belong to Homebrew and were not previously set:

[ -z "${MANPATH-}" ] || export MANPATH=":${MANPATH#:}";
export INFOPATH="/opt/homebrew/share/info:${INFOPATH:-}";

And I see that Homebrew tool was updated in that release:

- - Homebrew 4.3.19
+ - Homebrew 4.3.20

A current update contains the next release:

- - Homebrew 4.3.20
+ - Homebrew 4.3.21

We'll work on the issue. I'll check how it's going on staging.

UPD: Next image update affected by this in the same way.

erik-bershel commented 1 month ago

First version of a workaround was breaking for Homebrew and installed packages, so I decided to remove it

Updated workaround

       - name: Fix PATH in .bashrc
         run: |
           sed -i '' '/; export PATH;/d' ~/.bashrc
           echo 'export PATH="/opt/homebrew/bin:/opt/homebrew/sbin:$PATH"' >> ~/.bashrc
           source ~/.bashrc

cc: @hoxbro @gcampbell-msft

kdaveid commented 1 month ago

I might experience the same issue because I switched from macOS-13 to macOS-14 (without changes in usercode) but the build breaks with:

Tool /usr/bin/xcrun execution started with arguments: actool (...)

Tool /usr/bin/xcrun execution finished (exit code = 72).

xcrun: error: sh -c '/Applications/Xcode_15.4.0.app/Contents/Developer/usr/bin/xcodebuild -sdk /Applications/Xcode_15.4.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk -find actool 2> /dev/null' failed with exit code 16384: (null) (errno=Invalid argument)
xcrun: error: unable to find utility "actool", not a developer tool or in PATH
chadlwilson commented 1 month ago

This might fix the issue as a temporary workaround:

       - name: Try to fix env
         run: |
           sudo sed -i '' '/ export PATH;$/,+2d' ~/.bashrc
           source ~/.bashrc

Example run: https://github.com/erik-bershel/example-repo/actions/runs/10889443100

It would be really nice if third party could check a workaround. Correct me if I wrong please. šŸ™‡

Thanks @erik-bershel - your workaround worked for me at https://github.com/getgauge/gauge-vscode/commit/05f3211848fe2d88ef13beef03615d035b844afa šŸ™

erik-bershel commented 1 month ago

Hey @chadlwilson!

Please pay attention to the fact that the first version of WA was breaking for brew and the packages installed via brew. Until the fix I recommend to use next:

       - name: Fix PATH in .bashrc
         run: |
           sed -i '' '/; export PATH;/d' ~/.bashrc
           echo 'export PATH="/opt/homebrew/bin:/opt/homebrew/sbin:$PATH"' >> ~/.bashrc
           source ~/.bashrc
chadlwilson commented 1 month ago

@erik-bershel sure, thanks. Not a problem for us as dont rely on any brew installed packages. In our case the runner was losing a go binary our workflow builds and adds to the PATH in a JS action that is then needed in later npm driven tests.

This is a bit surprising that this is not yet fixed in live images. This is a pretty bad bug? If the source is homebrew I'd have thought a downgrade to working version to move forward would be safest and easily prioritised?

grahamc commented 1 month ago

This fix has caused a test failure in https://github.com/determinateSystems/nix-installer as well, tracked down to 20240911.3 and 20240915.3.

erik-bershel commented 1 month ago

Rollout with the fix in progress, might be tracked via PR: https://github.com/actions/runner-images/pull/10647

erik-bershel commented 1 month ago

VM rollout finished. .bashrc seems to be fine:

export HOMEBREW_PREFIX="/opt/homebrew";
export HOMEBREW_CELLAR="/opt/homebrew/Cellar";
export HOMEBREW_REPOSITORY="/opt/homebrew";
[ -z "${MANPATH-}" ] || export MANPATH=":${MANPATH#:}";
export INFOPATH="/opt/homebrew/share/info:${INFOPATH:-}";
export PATH="/opt/homebrew/bin:/opt/homebrew/sbin:$PATH"
export ImageVersion=20240918.8
export ImageOS=macos14
wolfv commented 1 month ago

Thanks @erik-bershel !