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

glib: fix g-ir-scanner PATH precedence. #216530

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ewtoombs
Copy link

The staging g-ir-scanner must be preferred over the system's g-ir-scanner. A mismatch in the versions, for example, will cause the build to fail.

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

The staging g-ir-scanner must be preferred over the system's g-ir-scanner.
A mismatch in the versions, for example, will cause the build to fail.
@github-actions github-actions bot added CI-linux-self-hosted-deps Test dependents on Linux self-hosted runner long dependent tests Set a long timeout for dependent testing labels Mar 25, 2025
Copy link
Contributor

Thanks for contributing to Homebrew! 🎉 It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request.

@carlocab
Copy link
Member

carlocab commented Mar 26, 2025

The staging g-ir-scanner must be preferred over the system's g-ir-scanner. A mismatch in the versions, for example, will cause the build to fail.

The system's g-ir-scanner should already not be on PATH in the build environment. How did you install the conflicting g-ir-scanner?

@carlocab carlocab requested a review from cho-m March 26, 2025 07:08
@ewtoombs
Copy link
Author

I don't know why g-ir-scanner is sometimes in the PATH already, but it can be.
This is what caused the build to fail for me.
The fix is simple enough.
I see no reason why it can't be applied.

@carlocab
Copy link
Member

I see no reason why it can't be applied.

One reason is that the choice to append rather than prepend to PATH in 19b6bb1 could've been deliberate.

Another is that it appears, from your description so far, that there is an underlying bug that's causing your build failure, and applying this change just masks that bug. We should fix it instead of try to hide the symptoms.

How did you install the conflicting g-ir-scanner? What is the output of your brew config?

@cho-m cho-m added CI-skip-dependents Pass --skip-dependents to brew test-bot. CI-no-bottles Merge without publishing bottles labels Mar 26, 2025
@cho-m
Copy link
Member

cho-m commented Mar 26, 2025

There does seem to be a scenario where Linux can use a system copy as the sanitized PATH includes /usr/bin. On macOS, I don't think this would happen.

I'm okay with modification as the staging bin directory should only have g-ir-* executables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-linux-self-hosted-deps Test dependents on Linux self-hosted runner CI-no-bottles Merge without publishing bottles CI-skip-dependents Pass --skip-dependents to brew test-bot. long dependent tests Set a long timeout for dependent testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants