Skip to content

Conversation

@tueda
Copy link
Collaborator

@tueda tueda commented Aug 25, 2025

This PR adds checks both at configuration and at runtime to require FLINT 3.2.0 or later. Resolves #679.

Note: Many test jobs are currently failing due to old FLINT versions.

Update: This PR depends on:

@jodavies
Copy link
Collaborator

For sensible runtimes we'd need to cache the flint build, as you do for the macos tests, for the linux jobs.

@tueda
Copy link
Collaborator Author

tueda commented Aug 26, 2025

Rebased onto #622.

@coveralls
Copy link

coveralls commented Aug 26, 2025

Coverage Status

coverage: 54.369% (-0.005%) from 54.374%
when pulling 7813cf6 on tueda:pr/flint-version-check
into 9828301 on form-dev:master.

@tueda tueda force-pushed the pr/flint-version-check branch from 39e2dd5 to db91484 Compare August 26, 2025 10:42
@jodavies
Copy link
Collaborator

jodavies commented Sep 3, 2025

If we require at least v3.2.0, we could also remove the "workaround" here:

form/sources/flintinterface.cc

Lines 1724 to 1751 in e3eeea3

#[ flint::startup_init :
*/
// The purpose of this function is it work around threading issues in flint, reported in
// https://github.com/flintlib/flint/issues/1652 and fixed in
// https://github.com/flintlib/flint/pull/1658 . This implies versions prior to 3.1.0,
// but at least 3.0.0 (when gr was introduced).
void flint::startup_init(void) {
#if __FLINT_RELEASE >= 30000
// Here we initialize some gr contexts so that their method tables are populated. Crashes have
// otherwise been observed due to these two contexts in particular.
fmpz_t dummy_fmpz;
fmpz_init(dummy_fmpz);
fmpz_set_si(dummy_fmpz, 19);
gr_ctx_t dummy_gr_ctx_fmpz_mod;
gr_ctx_init_fmpz_mod(dummy_gr_ctx_fmpz_mod, dummy_fmpz);
gr_ctx_clear(dummy_gr_ctx_fmpz_mod);
gr_ctx_t dummy_gr_ctx_nmod;
gr_ctx_init_nmod(dummy_gr_ctx_nmod, 19);
gr_ctx_clear(dummy_gr_ctx_nmod);
fmpz_clear(dummy_fmpz);
#endif
}
/*
#] flint::startup_init :

@tueda tueda force-pushed the pr/flint-version-check branch from db91484 to ed1328f Compare September 4, 2025 01:50
@tueda
Copy link
Collaborator Author

tueda commented Sep 4, 2025

If we require at least v3.2.0, we could also remove the "workaround" here:

Right. I have pushed a commit that completely removes the workaround.
Do you think we should leave an empty hook, like flint_startup_init in AllocSetups, just in case we need it again in the future?

@jodavies
Copy link
Collaborator

jodavies commented Sep 4, 2025

I don't think so, it would be straightforward to add back in if it becomes necessary in the future.

@jodavies
Copy link
Collaborator

jodavies commented Sep 5, 2025

At this point we should also recommend flint >=3.2.0 (and zstd) in the README.md and INSTALL files.

Disallow FLINT versions earlier than 3.2.0 by performing a
preprocessor-based check during configure.
Add a runtime check that disallows FLINT versions earlier than 3.2.0.
Note that the version check is also performed during the configure step.
A runtime failure typically indicates that an incorrect shared library
was linked.
We now require FLINT >= 3.2.0. The workaround is no longer necessary.
@tueda tueda force-pushed the pr/flint-version-check branch from ce61f51 to 7813cf6 Compare September 30, 2025 09:41
@tueda tueda merged commit 0e45065 into form-dev:master Sep 30, 2025
84 checks passed
@tueda
Copy link
Collaborator Author

tueda commented Sep 30, 2025

TODO: README.md and INSTALL files must be updated.

@tueda tueda deleted the pr/flint-version-check branch September 30, 2025 10:40
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.

Crash in FactArg with Flint

3 participants