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

recent commit break bang rewriting for several formulas #12826

Closed
1 of 2 tasks
joshgoebel opened this issue Feb 2, 2022 · 8 comments
Closed
1 of 2 tasks

recent commit break bang rewriting for several formulas #12826

joshgoebel opened this issue Feb 2, 2022 · 8 comments
Labels
bug Reproducible Homebrew/brew bug outdated PR was locked due to age

Comments

@joshgoebel
Copy link

joshgoebel commented Feb 2, 2022

7e6be5e makes a change that is simply not compatible with several formula, including glib.rb. It breaks any packages that pass a large list of paths (one at a time) to rewrite_shebang and do not expect them all to succeed. Such as glib, gobject-introspection, gupnp, simgrid, spades, etc...

The relevant formula code that triggers this looks like:

      bin.find { |f|
        rewrite_shebang detected_python_shebang, f
      }

Lets just take glib as an example and bin.find returns:

/usr/local/Cellar/glib/2.70.3/bin
/usr/local/Cellar/glib/2.70.3/bin/gdbus
/usr/local/Cellar/glib/2.70.3/bin/gdbus-codegen
/usr/local/Cellar/glib/2.70.3/bin/gio
/usr/local/Cellar/glib/2.70.3/bin/gio-querymodules
/usr/local/Cellar/glib/2.70.3/bin/glib-compile-resources
/usr/local/Cellar/glib/2.70.3/bin/glib-compile-schemas
/usr/local/Cellar/glib/2.70.3/bin/glib-genmarshal
/usr/local/Cellar/glib/2.70.3/bin/glib-gettextize
/usr/local/Cellar/glib/2.70.3/bin/glib-mkenums
/usr/local/Cellar/glib/2.70.3/bin/gobject-query
/usr/local/Cellar/glib/2.70.3/bin/gresource
/usr/local/Cellar/glib/2.70.3/bin/gsettings
/usr/local/Cellar/glib/2.70.3/bin/gtester
/usr/local/Cellar/glib/2.70.3/bin/gtester-report

This will fail on the very first item because it's a path, not a file...

brew doctor output

Not relevant.

Verification

  • I ran brew update and am still able to reproduce my issue.
  • I have resolved all warnings from brew doctor and that did not fix my problem.

What were you trying to do (and why)?

Install glib.

What happened (include all command output)?

RuntimeError (No matching files found to rewrite shebang)

Sorry don't have full output, but I've already triaged this to a bug with the commit listed above.

What did you expect to happen?

glib to be installed.

Step-by-step reproduction instructions (by running brew commands)

`brew install glib` 

(and whatever flags you'd need to tell it to build it not just download a binary)

@joshgoebel joshgoebel added the bug Reproducible Homebrew/brew bug label Feb 2, 2022
@carlocab
Copy link
Member

carlocab commented Feb 2, 2022

brew config and brew doctor absolutely are relevant here for reproduction purposes.

Even if they weren't:

Please note we will close your issue without comment if you do not fill out the issue checklist below and provide ALL the requested information.

Please provide the requested information and we can reopen this. (Or open a new issue with the requested information.)

@carlocab carlocab closed this as completed Feb 2, 2022
@joshgoebel
Copy link
Author

joshgoebel commented Feb 2, 2022

@carlocab Very frustrating. I literally just spent an hour debugging this issue for you and pointed you to the relevant commit and showed exactly how it breaks formula if you had spent just a few minutes to look at it. It has nothing to do with my local config - it's a commit that just went into master in the past 24 hours.

CC @MikeMcQuaid Perhaps you could take a quick look at this.

commit 955292d66666c1aaafb1243fb5653f030e78fceb
Merge: 7a31b3beb 7e6be5eb4
Author: Mike McQuaid <mike@mikemcquaid.com>
Date:   Tue Feb 1 08:37:34 2022 +0000

    Merge pull request #12820 from branchvincent/shebang

    shebang: raise error if no rewriting

@joshgoebel
Copy link
Author

joshgoebel commented Feb 2, 2022

@carlocab Would a revert PR be looked at any more receptively? I'm not sure what this commit was even designed to resolve (and sadly don't have time to figure that out) so I'm not the best person to try to actually rewrite it (to accord to whatever the original intent was)... but I'd be happy to open a PR reverting it.

As soon as I reverted it locally my glib build succeeded without any issue.

@carlocab
Copy link
Member

carlocab commented Feb 2, 2022

I appreciate the time you spent in debugging this issue, but bear in mind that we very regularly get non-actionable bug reports here. Many of these reports are very, very frustrating and can waste many, many hours of time when not dealt with appropriately.

Typically these come from users who don't understand what an actionable report is, therefore it isn't that simple to explain to them that we didn't require the requested information from another bug report because that other report was actionable and theirs isn't. Explaining that we require the requested information from all bug reports or else they get closed, however, is.

PRs are always welcome.

@joshgoebel
Copy link
Author

but bear in mind that we very regularly get non-actionable bug reports here.

Yes, I maintain a popular OSS library, Highlight.js, I know how issue reporting can be... and I even understand why the policy exists... but yet when I run into it it's still frustrating. :-) I wasn't frustrated at you personally, just more the "state of things" in general that force things to be this way... This system is still on Mohave, and doctor says all sorts of stuff - yet none of that is relevant - I've personally debugged the issue (and confirmed the fix).

I raised this point on the original PR and I'll see if the original contributor wants to pick it up and handle it. Let's see where that goes first.

@iMichka
Copy link
Member

iMichka commented Feb 2, 2022

brew doctor output
Not relevant.

I think it is.
You are compiling from source? We clearly state that we provide limited support for that, and that things might break. We ask to not open issues about this but rather open a pull request. If you are compiling from source you are on the "expert" side of homebrew, so on your own.
This is written in the brew doctor message.

Also: please don't cross post your issue at 3 different places to get things fixed faster. It just filles all the maintainers notification queue for not much more effect, except making everyone grumpy.

@MikeMcQuaid
Copy link
Member

This system is still on Mohave, and doctor says all sorts of stuff - yet none of that is relevant - I've personally debugged the issue (and confirmed the fix).

@joshgoebel That's up to us to decide. In this case, as @iMichka points out, it seems that you've omitted critically important information that you're running a configuration that's not supported. That doesn't mean we're not going to fix things but it seems you've very intentionally decided to omit that information.

Given you run a large project yourself: come on, pal, you really know better.

Also: please don't cross post your issue at 3 different places to get things fixed faster.

Similarly: if you do this again: you'll be blocked. It's really inconsiderate behaviour.

@joshgoebel
Copy link
Author

joshgoebel commented Feb 2, 2022

...to get things fixed faster ... please don't cross post your issue at 3 different places

I had a fix already, I wasn't in a hurry. 🤦🏻‍♂️ I've seen commit comments get entirely lost [multiple times] before (people finding them months later) so I opened an issue for better visibility. I did not realize the comment had landed on a PR thread until afterward... I'm sorry this created 3 notifications for some.

...but rather open a pull request.

I will definitely do this next time and see how that goes. 👍🏼 It's good to see carlocab was able to confirm my fix this morning via #12828.

@github-actions github-actions bot added the outdated PR was locked due to age label Mar 5, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Reproducible Homebrew/brew bug outdated PR was locked due to age
Projects
None yet
Development

No branches or pull requests

4 participants