Skip to content

Bump version to 1.8.18 to resolve multiple CVEs#68

Merged
ocefpaf merged 1 commit intoconda-forge:masterfrom
qwhelan:v1.8.18
May 31, 2017
Merged

Bump version to 1.8.18 to resolve multiple CVEs#68
ocefpaf merged 1 commit intoconda-forge:masterfrom
qwhelan:v1.8.18

Conversation

@qwhelan
Copy link
Copy Markdown
Contributor

@qwhelan qwhelan commented May 3, 2017

A few CVEs were reported and fixed in hdf5 back in November 2016: http://blog.talosintelligence.com/2016/11/hdf5-vulns.html

The hdf5 release notes for that release are available here: https://support.hdfgroup.org/ftp/HDF5/current18/src/hdf5-1.8.18-RELEASE.txt

@conda-forge-linter
Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@qwhelan qwhelan force-pushed the v1.8.18 branch 2 times, most recently from 9d12319 to b4bfe7c Compare May 6, 2017 18:06
@qwhelan
Copy link
Copy Markdown
Contributor Author

qwhelan commented May 6, 2017

Just updated the PR, so believe this is ready for merging once builds complete.

recipe/meta.yaml Outdated

build:
number: 10
number: 1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please reset to 0.

recipe/meta.yaml Outdated
build:
number: 10
number: 1
skip: True # [py==36]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please change py==36 to win and py==36 and then re-render with the latest conda-smithy.

@qwhelan qwhelan force-pushed the v1.8.18 branch 5 times, most recently from b22aa52 to 08258a4 Compare May 7, 2017 05:48
@conda-forge-linter
Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Selectors are suggested to take a <two spaces>#<one space>[<expression>] form.

@conda-forge-linter
Copy link
Copy Markdown

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@qwhelan qwhelan force-pushed the v1.8.18 branch 3 times, most recently from 5ddd791 to 1720f90 Compare May 8, 2017 22:27
recipe/build.sh Outdated
export LDFLAGS="${LDFLAGS} -Wl,-rpath,$PREFIX/lib"
else
# Shared Fortran libs on OSX unsupported as of 1.8.18
FORTRAN_FLAGS=("--enable-fortran" "--enable-fortran2003")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm...this may be a problem. @astrofrog, care to comment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jakirkham While digging into why the Fortran tests were failing on OSX, I came across a log message saying the following:

checking if shared Fortran libraries are supported... no
configure: WARNING: Shared Fortran libraries not currently supported on Mac.
configure: WARNING: Allowing unsupported Fortran shared libraries due to use of --enable-unsupported flag

The --enable-unsupported flag is needed to build the high-level API and enable thread-safe (which is not supported by the library authors), so we can't turn that off.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also, to clarify - the lack of support for shared fortran libs on OSX goes back to at least 1.8.14.

Copy link
Copy Markdown
Member

@jakirkham jakirkham May 9, 2017

Choose a reason for hiding this comment

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

I see. Thanks for clarifying. What if we do a hack and try to delete the shared libgfortran libraries on OS X during the build instead? This way it will be forced to link statically to libgfortran.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't have a Mac, so I've been testing through Travis here - my impression is that Apple ships those with OSX. We could try to delete the shared libraries that are built, but we'd need to delete between make and make check.

Given the fortran test suite is totally failing on OSX, I'd be skeptical of whether it's safe to rely on.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No we use a gcc package to handle that case. Considering this built fine before the --enable-unsupported flag was added and it is complaining about shared libraries on Mac, I think it would be worth trying to see if forcing static libraries would work. Would recommend borrowing this commit and changing libbz2 to libgfortran.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jakirkham Just passing the --disable-shared flag seems to have done the trick. Would you like me to throw in something like libbz2 to nuke the old shared lib when users upgrade?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice! Thanks for figuring that out. I think removing libgfortran is unnecessary then as we know it will fail if it does that.

@jakirkham
Copy link
Copy Markdown
Member

Could you please add a separate PR that includes just those changes to fix Mac (e.g. re-rendering, skip changes, and the --disable-shared flag) for the current version? Also please document in there info about what we learned with Mac.

Once that is merged we can clean up this PR to be a simple version bump and merge it as well.

Does that all sound good?

@qwhelan
Copy link
Copy Markdown
Contributor Author

qwhelan commented May 16, 2017

@jakirkham Sounds good, splitting it out now

@qwhelan qwhelan force-pushed the v1.8.18 branch 2 times, most recently from a1c1e9d to 54ed4af Compare May 16, 2017 05:03
@jakirkham
Copy link
Copy Markdown
Member

jakirkham commented May 16, 2017

Thanks for doing that. For reference, this is split off as PR ( #69 ).

@jakirkham
Copy link
Copy Markdown
Member

Now that PR ( #69 ) is in. Could we please rebase this?

recipe/meta.yaml Outdated
fn: hdf5-{{version}}.tar.gz
url: https://support.hdfgroup.org/ftp/HDF5/releases/hdf5-{{ version }}/src/hdf5-{{ version }}.tar.gz
md5: 7d572f8f3b798a628b8245af0391a0ca
url: https://support.hdfgroup.org/ftp/HDF5/releases/hdf5-1.8/hdf5-{{ version }}/src/hdf5-{{ version }}.tar.gz
Copy link
Copy Markdown
Member

@jakirkham jakirkham May 29, 2017

Choose a reason for hiding this comment

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

Pulling out this URL tweak into PR ( #70 ). So this will be a bit cleaner to manage (i.e. not hard coding version numbers in the URL).

Copy link
Copy Markdown
Member

@jakirkham jakirkham May 29, 2017

Choose a reason for hiding this comment

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

Have addressed this in master. So will need another rebase of this PR to clean this stuff out.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for making the URL change. Rebased and pushed a minute ago.

@qwhelan
Copy link
Copy Markdown
Contributor Author

qwhelan commented May 31, 2017

@jakirkham I believe this is ready to merge.

This was referenced Jun 2, 2017
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.

4 participants