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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNT: Update expat 2.2.9 #10038

Merged
merged 3 commits into from Mar 19, 2020
Merged

MNT: Update expat 2.2.9 #10038

merged 3 commits into from Mar 19, 2020

Conversation

pllim
Copy link
Member

@pllim pllim commented Mar 13, 2020

Description

This pull request is to update bundled expat to 2.2.9. It is used in astropy.utils.xml for parsing of VO table.

@saimn , since you updated it the last time in #8343, can you please have a look to make sure I didn't mess anything up? 馃檱鈥嶁檧

@pllim pllim added the external PRs and issues related to external packages vendored with Astropy (astropy.extern) label Mar 13, 2020
@pllim pllim added this to the v4.1 milestone Mar 13, 2020
@pllim pllim requested a review from saimn March 13, 2020 00:28
@pllim
Copy link
Member Author

pllim commented Mar 13, 2020

Travis CI and mpldev failures caused by scientific-python/pytest-doctestplus#94

@pllim

This comment has been minimized.

@saimn
Copy link
Contributor

saimn commented Mar 13, 2020

I had the same issue ;), you need to revert some changes in expat_config.h:
78653ac

--with-getrandom
--without-getrandom
--with-sys-getrandom
--without-sys-getrandom
Copy link
Contributor

Choose a reason for hiding this comment

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

Using these new options with ./configure would simplify the update process.

Copy link
Member Author

Choose a reason for hiding this comment

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

Which one is the correct option? 馃槅

Copy link
Contributor

Choose a reason for hiding this comment

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

I think both --without-getrandom and --without-sys-getrandom, you can check the diff on expat_config.h.

Other changes:
#299 #302 Windows: Replace LoadLibrary hack to access
unofficial API function SystemFunction036 (RtlGenRandom)
by using official API function rand_s (needs WinXP+)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also interesting since I had to include loadlibrary.c for windows:
ef5b44e

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. Does that mean I can remove what you added to setup_package.py?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I think so.

@pllim

This comment has been minimized.

@pllim

This comment has been minimized.

@bsipocz
Copy link
Member

bsipocz commented Mar 19, 2020

@pllim - I leave the rebase to you here.

@pllim
Copy link
Member Author

pllim commented Mar 19, 2020

@saimn , tests passed. Are you okay with this? Thanks for reviewing!

Copy link
Contributor

@saimn saimn 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, at least to my knowledge.

@saimn saimn merged commit 98c3ab2 into astropy:master Mar 19, 2020
@pllim pllim deleted the expat-2.2.9 branch March 19, 2020 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement external PRs and issues related to external packages vendored with Astropy (astropy.extern)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants