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

sextractor: Support newer compilers by changing finite to isfinite #181246

Merged
merged 1 commit into from
Sep 21, 2024

Conversation

Chekov2k
Copy link

  • [ x] Have you followed the guidelines for contributing?
  • [ x] Have you ensured that your commits follow the commit style guide?
  • [ x] Have you checked that there aren't other open pull requests for the same formula update/change?
  • [ x] 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?
  • [ x] Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • [ x] 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>?

Works around the following issue:

homebrew-core % brew install sextractor -s
==> Downloading https://formulae.brew.sh/api/formula.jws.json
##O#-  #
==> Downloading https://formulae.brew.sh/api/cask.jws.json
##O#-  #
Warning: sextractor 2.28.0 is already installed and up-to-date.
To reinstall 2.28.0, run:
  brew reinstall sextractor
balzer@akb homebrew-core % brew reinstall sextractor -s
Warning: building from source is not supported!
You're on your own. Failures are expected so don't create any issues, please!
==> Fetching sextractor
==> Downloading https://raw.githubusercontent.com/Homebrew/homebrew-core/8a081cafac89674690eb3f7fcd2c1ee6a24b64ef/Formula/s/se
####################################################################################################################### 100.0%
==> Downloading https://github.com/astromatic/sextractor/archive/refs/tags/2.28.0.tar.gz
==> Downloading from https://codeload.github.com/astromatic/sextractor/tar.gz/refs/tags/2.28.0
# -#O=#    #
==> Reinstalling sextractor
==> ./autogen.sh
==> ./configure --disable-silent-rules --enable-openblas --with-openblas-libdir=/opt/homebrew/opt/openblas/lib --with-openblas
==> make install
Last 15 lines from /Users/balzer/Library/Logs/Homebrew/sextractor/03.make:
#define LM_FINITE finite // ICC, GCC
                  ^
#define LM_FINITE finite // ICC, GCC
                  ^
./lmbc_core.c:324:13: note: include the header <math.h> or explicitly provide a declaration for 'finite'
./compiler.h:63:19: note: expanded from macro 'LM_FINITE'
#define LM_FINITE finite // ICC, GCC
                  ^
1 error generated.
1 error generated.
make[2]: *** [lm.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: *** [lmbc.o] Error 1
make[1]: *** [install-recursive] Error 1
make: *** [install-recursive] Error 1

READ THIS: https://docs.brew.sh/Troubleshooting

@github-actions github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Aug 15, 2024
@SMillerDev SMillerDev changed the title Support newer compilers by changing finite to isfinite sextractor: Support newer compilers by changing finite to isfinite Aug 15, 2024
@SMillerDev
Copy link
Member

Has this been submitted upstream?

@Chekov2k
Copy link
Author

Upstream hasn't released a version in years and would need checking against a whole range of old/ancient compilers. I felt it would be easier to patch it here since the range of compilers that need supporting is fairly limited compared to upstream.

@Chekov2k
Copy link
Author

There is an upstream bug report opened in 2022 as well with no traction: astromatic/sextractor#27

@SMillerDev
Copy link
Member

If upstream stopped responding we should also deprecate this.

@Chekov2k
Copy link
Author

Bit harsh ;) Gonna raise an MR on upstream as well and see if they respond

@SMillerDev
Copy link
Member

Bit harsh ;) Gonna raise an MR on upstream as well and see if they respond

It's either that or we keep putting bandaids on a corpse hoping it'll live.

@Chekov2k
Copy link
Author

Don't get me wrong, I get your point of view. I'll try and get that fixed upstream.

@cho-m cho-m added the upstream issue An upstream issue report is needed label Aug 15, 2024
Chekov2k pushed a commit to Chekov2k/sextractor that referenced this pull request Aug 15, 2024
@cho-m cho-m mentioned this pull request Aug 28, 2024
3 tasks
Copy link
Contributor

github-actions bot commented Sep 5, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Sep 5, 2024
@github-actions github-actions bot closed this Sep 12, 2024
@chenrui333 chenrui333 reopened this Sep 15, 2024
@chenrui333 chenrui333 closed this Sep 15, 2024
@chenrui333 chenrui333 reopened this Sep 15, 2024
@chenrui333 chenrui333 added sequoia-bottling CI-no-bottles Merge without publishing bottles labels Sep 15, 2024
@github-actions github-actions bot removed the autosquash Automatically squash pull request commits according to Homebrew style. label Sep 15, 2024
@chenrui333 chenrui333 removed upstream issue An upstream issue report is needed stale No recent activity labels Sep 15, 2024
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.

@chenrui333
Copy link
Member

  compiler.h:63:10: fatal error: cmath: No such file or directory
     63 | #include <cmath>
        |          ^~~~~~~
  compilation terminated.
  In file included from lm.c:59:
  compiler.h:63:10: fatal error: cmath: No such file or directory
     63 | #include <cmath>
        |          ^~~~~~~
  compilation terminated.

@Chekov2k
Copy link
Author

Well that took a few attempts but works now

Chekov2k pushed a commit to Chekov2k/sextractor that referenced this pull request Sep 16, 2024
@Chekov2k
Copy link
Author

Added an Apple check in the sextractor patch so now it works for Linux like before

@cho-m
Copy link
Member

cho-m commented Sep 16, 2024

Isn't the upstream code in C (<math.h>) not C++ (<cmath>)?

@Chekov2k
Copy link
Author

I went with the header this page suggested: https://en.cppreference.com/w/cpp/numeric/math/isfinite

@Chekov2k
Copy link
Author

The upstream maintainers replied so going to look at that code hopefully as well. I can try changing to math.h tonight

Chekov2k pushed a commit to Chekov2k/sextractor that referenced this pull request Sep 16, 2024
@Chekov2k
Copy link
Author

Changed it to math.h

@SMillerDev
Copy link
Member

If this applies cleanly, could you make the patch block use the upstream commit instead of a DATA block?

@Chekov2k
Copy link
Author

Let me read up on how to do that!

@Chekov2k
Copy link
Author

Should be done now

Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

One last thing

Formula/s/sextractor.rb Outdated Show resolved Hide resolved
Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

Thanks @Chekov2k ! Without contributions like yours it'd be impossible to keep homebrew going with the high standards that users have come to expect from the project. You can feel good knowing that you've made the world a tiny bit better for homebrew users around the world! 👍 🎉

@Chekov2k
Copy link
Author

No problem :)

@SMillerDev SMillerDev added this pull request to the merge queue Sep 21, 2024
Merged via the queue into Homebrew:master with commit 57aecc6 Sep 21, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-no-bottles Merge without publishing bottles CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. sequoia-bottling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants