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

add fenics-basix-pybind11-abi output #10

Merged
merged 7 commits into from
Jun 23, 2023
Merged

Conversation

minrk
Copy link
Member

@minrk minrk commented Jun 21, 2023

this output expresses a runtime constraint on the cxx compiler version, which changes the pybind11 ABI, so that downstream packages won't be built with mismatched compilers, which won't work.

e.g. dolfinx build 101 works, but 102 which upgraded the gcc to 12 doesn't.

related to conda-forge/pybind11-feedstock#79, I think

This update of the compilers may also fix later builds of fenics-dolfinx

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • The outputs section contained an unexpected subsection name. summary is not a valid subsection name.

For recipe:

  • It looks like the 'fenics-basix' output doesn't have any tests.
  • It looks like the 'fenics-basix-pybind11-abi' output doesn't have any tests.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • The outputs section contained an unexpected subsection name. summary is not a valid subsection name.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@minrk minrk changed the title add fenics-basics-pybind11-abi output add fenics-basix-pybind11-abi output Jun 21, 2023
@minrk minrk mentioned this pull request Jun 21, 2023
@minrk
Copy link
Member Author

minrk commented Jun 21, 2023

This is a little trickier than I realized, since the constraint needs to be in the build environment, so it shouldn't have any actual runtime dependencies.

@minrk minrk marked this pull request as draft June 21, 2023 11:44
minrk added 2 commits June 21, 2023 14:48

Verified

This commit was signed with the committer’s verified signature.
minrk Min RK
with run constraint on C compiler version, which changes the pybind11 abi

Verified

This commit was signed with the committer’s verified signature.
minrk Min RK
…nda-forge-pinning 2023.06.21.09.36.35
@minrk
Copy link
Member Author

minrk commented Jun 21, 2023

What we really want is an abi output that becomes a host and runtime dependency, and is versioned. It's not just a run constraint on the compiler (which is required at build-time), it's an actual runtime constraint on the build-time compiler version, otherwise there will be an inappropriate lack of conflict between old builds and new ones (e.g. basix built with gcc 11 and dolfinx build with gcc 12 and vice versa both claim to be compatible, but pybind11 fails in both directions).

For example:

  • fenics-basix-pybind11-abi {{ pybind11_abi }}.{{cxx_compiler_version}}

minrk added 2 commits June 22, 2023 08:50

Verified

This commit was signed with the committer’s verified signature.
minrk Min RK

Verified

This commit was signed with the committer’s verified signature.
minrk Min RK
minrk added 2 commits June 22, 2023 11:28

Verified

This commit was signed with the committer’s verified signature.
minrk Min RK
explodes build matrix (faster builds, more duplicated but identical libbasix published)

Verified

This commit was signed with the committer’s verified signature.
minrk Min RK
…nda-forge-pinning 2023.06.22.04.03.15

Verified

This commit was signed with the committer’s verified signature.
minrk Min RK
avoids the possibility of depending on basix-abi and getting a basix package that doesn't have that abi
@minrk
Copy link
Member Author

minrk commented Jun 23, 2023

So I've gone with skipping the compiler constraint, and relying on packaging tests to catch this, because the package really won't work if this is mismatched and it should be easily tested.

As a result, just exporting the abi as a package which:

  • depends on an exact fenics-basix (to ensure that the abi can never bve satisfied by a basix build that doesn't
  • run_exports a dependency on itself

which should make it impossible for any downstream package (dolfinx) with the abi in a host dependency to install in an env with a basix that doesn't provide that ABI

@minrk
Copy link
Member Author

minrk commented Jun 23, 2023

tested locally with fenics-dolfinx and seems to work

@minrk minrk marked this pull request as ready for review June 23, 2023 12:16
@minrk minrk merged commit 3c77086 into conda-forge:main Jun 23, 2023
@minrk minrk deleted the pybind11-bump branch June 23, 2023 12:17
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.

None yet

1 participant