osmcode / libosmium

Fast and flexible C++ library for working with OpenStreetMap data.
https://osmcode.org/libosmium/
Boost Software License 1.0
472 stars 114 forks source link

Check for bad file descriptors when opening bzip2-compressed files #354

Closed brawer closed 2 years ago

brawer commented 2 years ago

After this change, all tests pass on Alpine Linux. Fixes https://github.com/osmcode/libosmium/issues/353.

brawer commented 2 years ago

The AppVeyor build failure seems unrelated to the change. https://github.com/osmcode/libosmium/issues/355

joto commented 2 years ago

Fails on Windows builds.

brawer commented 2 years ago

Fails on Windows builds.

@joto, mind approving running GitHub workflows for this change? Should be fixed on Windows, but as a first-time committer to this repo, I can’t start the workflow myself.

joto commented 2 years ago

I am not too happy about that solution. Having to fix a minor problem that will never show up in real use on a fringe system is bad enough and now we need a fix for the fix (ie the ifdef) because it doesn't work on Windows. Maybe we need a different approach here. Presumably using a broken file descriptor with musl will still fail, but instead of in the fdopen somewhere down the line when we try to read some data. Maybe the test should be rewritten to accept failure then.

brawer commented 2 years ago

Agree. See https://github.com/osmcode/libosmium/pull/357 for an alternative fix.

brawer commented 2 years ago

Closing in favor of #357.