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

Update to GMP 6.2.0 #36343

Merged
merged 1 commit into from
Jul 8, 2020
Merged

Update to GMP 6.2.0 #36343

merged 1 commit into from
Jul 8, 2020

Conversation

fingolfin
Copy link
Contributor

@fingolfin fingolfin commented Jun 18, 2020

This fixes a few bugs and brings performance improvements; for the full release notes, see https://gmplib.org/gmp6.2.

Removed deps/patches/gmp-config-ldflags.patch as it is no longer needed.

Adjusted deps/patches/gmp-exception.patch (in principle it could go, but then on Windows division by zero of GMP integers would not lead to a DivideError but instead a generic ErrorException)

This is the same as an improved version of #36309 which was reverted in #36328 as it was failing the pre-merge CI tests on Win32 and Win64. Moreover, there were other issues (see JuliaPackaging/Yggdrasil#1159 and JuliaRegistries/General#16517), so naturally this PR here shouldn't be merged hastily. But I wanted to submit it so that I can see what the failures on Win32 and Win64 are so that hopefully they can be debugged and resolved.

UPDATE: I added back a variant of the GMP exception pass to ensure behavior for division by zero of GMP bigints stayed the same on Windows, too. See also JuliaPackaging/Yggdrasil#1188

@fingolfin
Copy link
Contributor Author

The Win32 precheck now works, but the main test fails -- I am pretty sure this is because JuliaPackaging/Yggdrasil#1188 isn't merged, so of course we are using the "old" GMP_jll 6.2 which had the old exceptions patch.

This fixes a few bugs and brings performance improvements; for the full
release notes, see <https://gmplib.org/gmp6.2>.

Removed `deps/patches/gmp-config-ldflags.patch` as it is no longer needed.

Adjusted `deps/patches/gmp-exception.patch` (in principle it could go, but
then on Windows division by zero of GMP integers would not lead to a
`DivideError` but instead a generic `ErrorException`)
@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Jul 8, 2020

@StefanKarpinski Should this get into 1.5 RC2 as it has bugfixes and "major speedup" (and supposedly tested already, as a separate project)? [And maybe your "active project" PR today as it's a small change (and at least safe for users; only for developers)?] [EDIT: I suppose it can be risky (I just thought 1.5 is a major update anyway), as you say it can go into 1.5.1, but not "active project" PR?]

@fingolfin fingolfin deleted the mh/GMP branch July 9, 2020 10:37
@StefanKarpinski
Copy link
Member

No, upgrading dependencies is quite risky and not something to do in a release candidate. It could go into 1.5.1 if it's unproblematic on master for a while.

@fingolfin
Copy link
Contributor Author

This is indeed quite delicate. For now, GMP_jll 6.2.0 is set to require Julia >= 1.6 in the general registry; that would have to be adapted if it was put into Julia 1.5.1.

All in all, though, this might be a breaking change for other packages using GMP, so personally I would recommend to not put this into 1.5 but rather leave it for 1.6 (in the hope that 1.6 in turn won't be that far off ;-).

simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants