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

Update install.sh to use /usr/bin/stat #849

Merged
merged 1 commit into from
May 18, 2024
Merged

Conversation

DG12
Copy link
Contributor

@DG12 DG12 commented May 17, 2024

Using customized stat in $PATH can be in conflict with standard.

Using customized stat in $PATH can be in conflict with standard.
@@ -160,7 +160,7 @@ then
fi
HOMEBREW_CACHE="${HOME}/Library/Caches/Homebrew"

STAT_PRINTF=("stat" "-f")
STAT_PRINTF=("/usr/bin/stat" "-f")
Copy link
Member

Choose a reason for hiding this comment

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

In practice this should always be right, but I can't find a good POSIX/similar source that asserts that stat is always at /usr/bin/stat. Then again maybe this doesn't matter, since we assume similar things for chown and chgrp below.

Copy link
Member

Choose a reason for hiding this comment

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

I checked the file list for all the distros we advertise support for (Debian, Ubuntu, Fedora, CentOS, Red Hat, Arch). In all cases stat is provided by coreutils and present at /usr/bin/stat. I agree though, we don't necessarily have a guarantee it'll always be provided there in every distro; for example, alpine's busybox lives in /bin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be more compatible perhaps someone could enhance this (and others if necessary ) to include something like:

UTILDIR ='/usr/bin'
if [ ! -x $UTILDIR/stat ]; then UTILDIR='/bin' ; fi
STAT_PRINTF=("$UTILDIR/stat" "-f")
...
STAT_PRINTF=("$UTILDIR/stat" "--printf")

Copy link
Member

Choose a reason for hiding this comment

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

It's fine as-is until someone complains.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks @DG12 for the PR and @mistydemeo for the research!

@MikeMcQuaid MikeMcQuaid merged commit 44c4db5 into Homebrew:master May 18, 2024
3 checks passed
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

4 participants