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

sundials 6.5.0 #119907

Closed
wants to merge 2 commits into from
Closed

sundials 6.5.0 #119907

wants to merge 2 commits into from

Conversation

cho-m
Copy link
Member

@cho-m cho-m commented Jan 6, 2023

  • 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 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 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

Will need rebase if we merge #118915.

-DBUILD_SHARED_LIBS=ON
-DKLU_ENABLE=ON
-DKLU_LIBRARY_DIR=#{Formula["suite-sparse"].opt_lib}
-DKLU_INCLUDE_DIR=#{Formula["suite-sparse"].opt_include}
-DLAPACK_ENABLE=ON
-DBLA_VENDOR=OpenBLAS
Copy link
Member Author

Choose a reason for hiding this comment

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

CMake Warning:
  Manually-specified variables were not used by the project:

    BLA_VENDOR

@cho-m cho-m mentioned this pull request Jan 6, 2023
6 tasks
p-linnane
p-linnane previously approved these changes Jan 6, 2023
@cho-m cho-m added long build Set a long timeout for formula testing CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. and removed do not merge labels Jan 7, 2023
@cho-m cho-m force-pushed the bump-sundials-6.5.0 branch 2 times, most recently from c333f5d to d830b3b Compare January 7, 2023 08:21
@cho-m cho-m added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Jan 7, 2023
@cho-m cho-m force-pushed the bump-sundials-6.5.0 branch from d830b3b to 37376c9 Compare January 7, 2023 16:37
@cho-m cho-m removed the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Jan 7, 2023
@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@@ -82,6 +82,9 @@ def install
/%OCTAVE_CONF_OCT(AVE)?_LINK_(DEPS|OPTS)%/,
'""'

# SUNDIALS 6.4.0 and later needs C++14 for C++ based features
ENV.append "CXXFLAGS", "-std=c++14"

Choose a reason for hiding this comment

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

It would be better to set it to -std=gnu++14 because Octave uses some GNU extensions (and only falls back to some inferior workarounds if they aren't available).

Also, setting this only in CXXFLAGS might cause some preprocessor tests to get different results during configuration than what the compiler would see when building.
It might be better to add this flag to CXX.

Copy link

@mmuetzel mmuetzel Jan 13, 2023

Choose a reason for hiding this comment

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

@cho-m: I hope it is ok to comment on a closed PR. But this should definitely be fixed in the build rules for the packaged Octave.
I.e.:

Suggested change
ENV.append "CXXFLAGS", "-std=c++14"
ENV.append "CXX", "-std=gnu++14"

@github-actions github-actions bot added the outdated PR was locked due to age label Feb 13, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 13, 2023
@cho-m cho-m deleted the bump-sundials-6.5.0 branch October 30, 2023 22:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. long build Set a long timeout for formula testing outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants