Update cfitsio to v3.43: includes critical bug fix #7274
Conversation
|
Hi there @drdavella 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃. Everything looks good from my point of view! 👍 If there are any issues with this message, please report them here. |
|
LGTM provided the tests pass. Just a question: Why the euv variable in the script? |
|
@MSeifert04 the script wasn't working for me at first, so I added those flags to help me debug it. I forgot to remove them before committing but it seems like there's no reason to remove them at this point: |
|
Happy to see you found my script (and sorry that it didn't work immediately...) |
mhvk
left a comment
There was a problem hiding this comment.
Yikes, buffer overflows in one of the standard forms... Good to have the update. One comment is really just a question.
cextern/trim_cfitsio.sh
Outdated
| rm -f cfitsio/docs/*.doc | ||
| rm -f cfitsio/docs/*.toc | ||
| rm -f cfitsio/[^L]*.* | ||
| rm -rf cfitsio/[^L]*.* |
There was a problem hiding this comment.
I'm surprised by the need for this change - this means that in the new cfitsio library there is a directory with a . in its name. Out of curiosity, what is it?
There was a problem hiding this comment.
I guess I should wait till this is answered before merging?
There was a problem hiding this comment.
For me it was cfitsio/cfitsio.xcodeproj, which appears to be generated when building (on OSX for me). Maybe we should just explicitly remove that file rather than using -rf here?
There was a problem hiding this comment.
So, that must be a new directory. I think I'd prefer removing it explicitly, but honestly I don't really know why...
|
Alright, all passed and approved, so merging now. |
Update cfitsio to v3.43: includes critical bug fix
This fixes #7272. I'll open a separate PR for backporting to 2.0.x.