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

external Pari 2.17 does not work (yet) #38769

Closed
2 tasks done
dimpase opened this issue Oct 4, 2024 · 3 comments · Fixed by #38772
Closed
2 tasks done

external Pari 2.17 does not work (yet) #38769

dimpase opened this issue Oct 4, 2024 · 3 comments · Fixed by #38772
Labels

Comments

@dimpase
Copy link
Member

dimpase commented Oct 4, 2024

Steps To Reproduce

If Pari 2.17 is installed system-wide, it could be picked up, but doesn't work.

Expected Behavior

Sage should not use Pari 2.17

Actual Behavior

it does

Additional Information

it's a 1-line fix in pari's spkg-configure.m4
Ban versions $\geq 135424$

Environment

  • OS: macOS 15.0.1 + Homebrew
  • Sage Version: all

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@dimpase dimpase added the t: bug label Oct 4, 2024
@jhpalmieri
Copy link
Member

jhpalmieri commented Oct 5, 2024

Related: #38749 (upgrade pari to 2.17). It's not clear to me how close that is to being ready, though.

@antonio-rojas
Copy link
Contributor

Related: #38749 (upgrade pari to 2.17). It's not clear to me how close that is to being ready, though.

The sagelib part is mostly ready (I just need to push some fixes for test failures caused by different ideal and unit group generators). However, that's just a small part of the story: most of the breakage is in cypari, which would need a new release including sagemath/cypari2#165 and sagemath/cypari2#166. And, to make things worse, 2.17.0 ships with a bug that breaks cypari build. So 2.17.0 should probably be unconditionally disallowed, and once 2.17.1 is released we should upgrade pari and cypari in sage.

@jhpalmieri
Copy link
Member

it's a 1-line fix in pari's spkg-configure.m4 Ban versions ≥ 135424

Not surprisingly, this works well, fixes the problem for me. I've opened a PR that makes this change.

vbraun pushed a commit to vbraun/sage that referenced this issue Oct 9, 2024
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 9, 2024
    
Do not allow external Pari 2.17 or later

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Sage does not build with Pari 2.17. Homebrew distributes this verison,
so we disallow it.

This fixes sagemath#38769.


### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [X] The title is concise and informative.
- [X] The description explains in detail what this PR is about.
- [X] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38772
Reported by: John H. Palmieri
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 11, 2024
    
Do not allow external Pari 2.17 or later

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Sage does not build with Pari 2.17. Homebrew distributes this verison,
so we disallow it.

This fixes sagemath#38769.


### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [X] The title is concise and informative.
- [X] The description explains in detail what this PR is about.
- [X] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38772
Reported by: John H. Palmieri
Reviewer(s): Dima Pasechnik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants