Skip to content

Avoid calling gnubin executables during macos build#20614

Open
lefth wants to merge 2 commits intodarktable-org:masterfrom
lefth:fix-mac-executables
Open

Avoid calling gnubin executables during macos build#20614
lefth wants to merge 2 commits intodarktable-org:masterfrom
lefth:fix-mac-executables

Conversation

@lefth
Copy link
Contributor

@lefth lefth commented Mar 21, 2026

This fixes #20613.

@zisoft
Copy link
Collaborator

zisoft commented Mar 21, 2026

See my comment here: #20613 (comment)

The darktable build scripts don't require or install gnubin, so please fix your system setup.

@zisoft
Copy link
Collaborator

zisoft commented Mar 21, 2026

I am still not convinced about these changes.

These changes are necessary for YOU, because YOU have changed the standard behavior of YOUR system.

Sure, we can add /usr/bin to the tools to make sure the system tools are used, but then we should be complete here: cut, grep, ...

# Handle library relative paths
oToolLDependencies=$(echo "$oToolLDependencies" | sed "s#@loader_path#${absolutePath}#")
oToolLDependencies=$(echo "$oToolLDependencies" | sed "s#@rpath#${absolutePath}#")
oToolLDependencies=$(echo "$oToolLDependencies" | /usr/bin/sed "s#@loader_path#${absolutePath}#")
Copy link
Member

Choose a reason for hiding this comment

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

That's wrong, you need to use PATH and never use full path in a script.

@TurboGit TurboGit added the controversial this raises concerns, don't move on the technical work before reaching a consensus label Mar 22, 2026
@lefth
Copy link
Contributor Author

lefth commented Mar 22, 2026

I'll see if I can tweak the calls to utility binaries to be cross platform. And yes, I know that's silly. It's Mac. It's not cross platform. Yet this is a homebrew installation accept, and homebrew sanctions putting the GNU utilities in PATH.

@zisoft
Copy link
Collaborator

zisoft commented Mar 22, 2026

and homebrew sanctions putting the GNU utilities in PATH

No, homebew does not overwrite the standard system utilities, it installs the GNU utilities with the prefix "g":

All commands have been installed with the prefix "g".
If you need to use these commands with their normal names, you
can add a "gnubin" directory to your PATH from your bashrc like:
PATH="$HOMEBREW_PREFIX/opt/findutils/libexec/gnubin:$PATH"

YOU have changed the PATH to overwrite the standard system utilities.

How should darktable take into account the personal preferences of countless users? The scripts are written to support the minimal standard system installation.

As I already wrote in your issue report, it would be an easy step to write a simple shell script which restores the system PATH and calls the darktable build scripts.

@lefth
Copy link
Contributor Author

lefth commented Mar 23, 2026

@zisoft Those directions you quoted are the directions I followed. I didn't do anything with the system configuration, nor install files in inappropriate places.

My impression is that lowering friction for newbies is important. How easy a project is to build influences whether new contributors join the project. Most of the time, when people fail to build a project, you don't hear about it--they just give up. I'd like to correct one of those small roadblocks. And no, it doesn't affect everybody. It probably affects everyone that shell scripts on both Mac and Linux, though pure Mac users won't have this setup. (That said, I know darktable has a very dedicated team and would not make technically bad compromises for the dubious benefit of some few people being able to compile more easily.)

I didn't understand what you said about assuming a minimal system to support the build agents. Is it important to not change this script?

Failing that, as I said on the issue page, there are other ways to prevent this problem: invoke the commands in a portable way, print a specific error, or add a sentence to the build instructions.

@zisoft
Copy link
Collaborator

zisoft commented Mar 23, 2026

@lefth :
I agree with @TurboGit comment above, and my personal opinion is to not use absolute paths in install scripts.
But that does not mean that I am not open to suggestions.

So let's ask what a coding AI agent tells us in this modern times of software development. Let me quote the answer here:

Based on the analysis of packaging/macosx/3_make_hb_darktable_package.sh, it is recommended to rely on the PATH rather than using absolute paths for system binary tools like sed, find, and grep.
Here is the reasoning for this recommendation:

  1. Portability across macOS Environments:
    • While system tools usually live in /usr/bin or /bin, users (or CI environments) might use package managers like Homebrew or MacPorts that install tools in /usr/local/bin or /opt/homebrew/bin.
    • Relying on PATH allows the user to control which version of the tools is prioritized.
  2. Tool Specificity (BSD vs. GNU):
    • The script uses BSD-style syntax for sed (specifically sed -i '', seen on lines 209, 210, 285, etc.). This syntax is native to macOS's default sed.
    • If you hardcoded /usr/bin/sed, you would enforce the system version. However, if a user has configured their environment to use GNU tools (e.g., gnubin in path), this script might actually fail because GNU sed handles -i differently. Relying on PATH implies "use the tool configured for this environment," which is generally the expected behavior for build scripts.
  3. Readability and Maintenance:
    • The script is significantly cleaner without full paths cluttering the logic.
    • The script already relies on PATH for brew, git, otool, install_name_tool, and pkg-config. Being inconsistent by hardcoding only sed or grep would add confusion.
      Exception / nuance:
      The only strong argument for absolute paths is security (avoiding path hijacking), but for a build script intended to be run by a developer or a CI runner in a controlled environment, this risk is minimal compared to the loss of flexibility.
      Proposed Action:
      Keep the script as is regarding tool invocation. Do not change sed to /usr/bin/sed.

Based on this answer and since I am the maintainer of this script, I would really like to end the discussion here.

@TurboGit : I vote for closing this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

controversial this raises concerns, don't move on the technical work before reaching a consensus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mac build fails if /opt/local/libexec/gnubin is in PATH

3 participants