Skip to content

Fix setup.py with CFLAGS/LDFLAGS #475

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

Merged
merged 3 commits into from
Apr 8, 2025
Merged

Conversation

t20100
Copy link
Collaborator

@t20100 t20100 commented Apr 8, 2025

This PR:

  • fixes a compilation issue if there is 2 spaces in CFLAGS or LDFLAGS env. variables (occuring in new jupyter-slurm conda env). In this case an empty string is passed as an argument to the compiler as "".
  • avoids the use of self.extensions and use ext instead since it is the currently handled Extension
  • avoid use of ext_package and use the fully qualified name in the Extension: Without this, the _cImageD11 is advertised as a "top level" module (I don't know why, it shouldn't). To check: from importlib.metadata import packages_distributions; packages_distributions()['_cImageD11']

@t20100 t20100 requested a review from jonwright April 8, 2025 15:10
@t20100 t20100 changed the title Update setup Fix setup.py with CFLAGS/LDFLAGS Apr 8, 2025
@t20100
Copy link
Collaborator Author

t20100 commented Apr 8, 2025

BTW, I don't think adding CFLAGS and LDFLAGS is needed: Those flags looks to be duplicated in the compiler calls. setuptools looks to already do the job.

Copy link
Member

@jonwright jonwright left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@jonwright jonwright merged commit 71de868 into FABLE-3DXRD:master Apr 8, 2025
2 of 6 checks passed
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.

2 participants