Skip to content

Update bundled expat version #7594

Closed

Description

@bsipocz

We have a very old version bundled in cextern (v2.1 back from 2013), I wonder whether we want to update it to a more recent one for v3.1?

Their chancelog includes several mentions of "security fixes" but I don't have enough experience with our xml utilities to be able to make an informed, quick judgement whether any of it affects us or not.

Activity

  1. added this to the v3.1 milestone on Jun 24, 2018
  2. pllim commented on Oct 26, 2018

    @pllim
    Member

    Given it is already freeze time, I would say "no" to 3.1?

  3. modified the milestones: v3.1, v3.2 on Oct 26, 2018
  4. bsipocz commented on Oct 28, 2018

    @bsipocz
    MemberAuthor

    I was looking into it, but it's like a black box for me. So ideally it would be nice to be updated by someone who actually knows something about libexpat, xmls, etc.

  5. pllim commented on Jan 10, 2019

    @pllim
    Member

    I did look into it recently semi-accidentally and went into a 🐰 🕳 . It has obviously been refactored significantly upstream, so we need to re-discover how to build that C library. An obvious place is expat_config.h in astropy/utils/xml/src. There might be other less obvious places but I think this library is only used in astropy/utils/xml, so anyone who wants to work on this should look into:

    • setup scripts in top level
    • astropy/utils/xml
    • the bundled expat (which theoretically is a blind copy-and-paste from upstream?)

    I am trying my luck here by pinging @mdboom .

  6. mhvk commented on Jan 10, 2019

    @mhvk
    Contributor

    I noticed in iterparser.py that there is an option to do slower parsing just using python, so libexpat is not essential for things to work. The question then is whether speedy xml parsing sufficiently important that it has to be bundled? Or can it be something we recommend people to install if they deal with lots of votable files?

  7. bsipocz commented on Jan 10, 2019

    @bsipocz
    MemberAuthor

    cc @tomdonaldson for answer for the votable question

  8. tomdonaldson commented on Jan 10, 2019

    @tomdonaldson
    Contributor

    I'd be curious to know how much slower the Python xml parser is. That said, my gut feeling is that unless Python xml parsing is really terrible, it would perform well enough for most votable users.

  9. mhvk commented on Jan 10, 2019

    @mhvk
    Contributor

    Sadly, unless I'm mistaken it doesn't seem we currently have any benchmarks in https://github.com/astropy/astropy-benchmarks - maybe the first goal would be to do a few quick tests and add some.

    Also cc @taldcroft since he may have more sense of speed.

  10. saimn commented on Jan 10, 2019

    @saimn
    Contributor

    Distributions build against the current version of expat, so my guess is that it should not be too hard to update ;). I did that in #8343, not sure about some changes in expat_config.h but I guess that it should work!

  11. taldcroft commented on Jan 11, 2019

    @taldcroft
    Member

    @mhvk - sorry, I have no sense of speed for xml parsing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions