Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX Ensure 'which' command detection across Linux distributions #11218

Closed
wants to merge 1 commit into from

Conversation

minimalic
Copy link

@minimalic minimalic commented May 4, 2024

Description

This pull request addresses an issue with the which command detection in the sake script, which failed to accurately detect the presence of the which command across different operating systems. The previous implementation using command -v which functioned properly on Ubuntu-like distributions but not on others like Rocky Linux/RHEL or Fedora, where it sometimes returned only a command name or a function instead of the full path to the executable.

To ensure broader compatibility, especially with non-Debian-based Linux distributions, this fix modifies the command detection logic to use type -P which.

Manual testing steps

On Linux inside Silverstripe project, run vendor/bin/sake dev/build and verify that it executes without the "which command" error.

Issues

Pull request checklist

  • The target branch is correct
  • All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • Code changes follow our coding conventions
  • This change is covered with tests (or tests aren't necessary for this change)
  • Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
  • CI is green

Copy link
Contributor

@michalkleiner michalkleiner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic needs to try two or three commands/approaches if we want to be truly OS-agnostic. type -P which doesn't work on MacOS.

Screenshot 2024-05-05 at 19 05 31

@minimalic
Copy link
Author

Yes, I thought the same on my first try on macOS, but I noticed the type command behaves differently in zsh (current default on macOS).

If you try it with bash (default environment in the sake script) you will notice that type -P which behaves the same across Unix-Like Systems.

You can test it with a simple script (don't forget to make this script itself executable with chmod u+x script.sh):

#!/usr/bin/env bash

[ -x "$(command -v which)" ] && echo "executable" || echo "no"
[ -x "$(type -P which)" ] && echo "executable" || echo "no"

I've tested it on latest macOS, Ubuntu, Rocky Linux/RHEL, Fedora. The second command worked on all systems, while the first (actual sake method) did not.

The problem with command -v which is the output of an "Alias" instead of direct path on some systems - so the check for executability fails. The type -P which method should be most compatible alternative - as long as sake uses bash shell environment.

@GuySartorelli
Copy link
Member

Closed in favour of #11232

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants