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 structseq.c to extra sources. #432

Merged
merged 4 commits into from
May 11, 2023
Merged

Conversation

fangerer
Copy link
Contributor

This is an important bug fix for HPy 0.9.0rc1 (and I will create rc2 after this is merged).

When I've added the helper functions HPyStructSequence_New/NewType in PR #415 , I obviously forgot to add it to the list of extra sources in hpy.devel. This may result in an undefined symbol error when linking.

It doesn't show up in the tests because we build a static lib containing the helpers and I did add the new source file to there (in setup.py).

In order to avoid such problems in future, I plan to have the list of helper sources just in one place.

@fangerer fangerer requested review from hodgestar and mattip May 11, 2023 08:20
% DEFAULT_HPY_ABI),
('hpy-use-static-libs', None, 'Use static library containing context '
'and helper functions for building '
'extensions (default: False)')
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a typo fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I first called it hpy-no-static-libs but then we changed it to semantically do not use static libs by default. So, I renamed the attribute but didn't rename the option 😞 .
This just shows that everything that is not tested will eventually fail. I will also add some tests for this as soon as I find the time to do so.

@mattip
Copy link
Contributor

mattip commented May 11, 2023

LGTM, just trying to understand the hpy-no-static-libs change.

@fangerer fangerer merged commit 5ddcbc8 into master May 11, 2023
@fangerer fangerer deleted the fa/fix_missing_structseq branch May 11, 2023 11:08
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