Skip to content

gdal: Add hdf4 support #26039

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 1, 2017
Merged

gdal: Add hdf4 support #26039

merged 2 commits into from
Jul 1, 2017

Conversation

knedlsepp
Copy link
Member

@knedlsepp knedlsepp commented May 23, 2017

Motivation for this change

I want to add hdf4 support to the gdal library.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@mention-bot
Copy link

@knedlsepp, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vbgl, @Hodapp87 and @MarcWeber to be potential reviewers.

@knedlsepp knedlsepp force-pushed the add-gdal-hdf4-support branch from d4e0377 to ddc340d Compare May 24, 2017 06:45
Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

Perhaps its worth to separate hdf4 into multiple outputs.

@@ -41,6 +46,7 @@ composableDerivation.composableDerivation {} (fixed: rec {
"--with-python" # optional
"--with-static-proj4=${proj}" # optional
"--with-geos=${geos}/bin/geos-config"# optional
"--with-hdf4=${hdf4}"
Copy link
Member

Choose a reason for hiding this comment

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

can you add the note that this is optional

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. I'll also look into the multiple outputs issue.

@knedlsepp knedlsepp force-pushed the add-gdal-hdf4-support branch 2 times, most recently from 1abc953 to f364a4c Compare May 29, 2017 17:17
@knedlsepp
Copy link
Member Author

knedlsepp commented May 29, 2017

@FRidh I split hdf4 up into bin/dev/out. I had to manually specify postInstall =''moveToOutput bin "$bin"'' however, because otherwise the build would fail. Not exactly sure if this is the correct way to do it, as the dev output was dealt with automatically.

@knedlsepp
Copy link
Member Author

@FRidh: Would you consider merging?

@FRidh
Copy link
Member

FRidh commented Jun 7, 2017

The following tests FAILED:
        127 - HDF_TEST-testhdf-shared (Failed)
        128 - HDF_TEST-buffer-shared (Failed)
        133 - MFHDF_TEST-cdftest-shared (Failed)
        134 - MFHDF_TEST-hdfnctest-shared (Failed)
Errors while running CTest

builder for ‘/nix/store/cmjc162yw5haxd0qws7r2c12hlbvw3za-hdf-4.2.12.drv’ failed with exit code 8
error: build of ‘/nix/store/cmjc162yw5haxd0qws7r2c12hlbvw3za-hdf-4.2.12.drv’ failed

Some of the errors that I could find in the log:

/build/hdf-4.2.12/build/bin/testhdf-shared: error while loading shared libraries: libhdf.so.4.2.12: cannot open shared object file: No such file or directory
/build/hdf-4.2.12/build/bin/hdfnctest-shared: error while loading shared libraries: libmfhdf.so.4.2.12: cannot open shared object file: No such file or directory

I think you might want to undo the multiple outputs. I tried to use them with hdf5 as well after I suggested it to you, but it didn't work out at all.

@FRidh FRidh self-assigned this Jun 7, 2017
@FRidh
Copy link
Member

FRidh commented Jun 7, 2017

Okay, so I tried myself without multiple outputs, and I still get the errors.

@FRidh
Copy link
Member

FRidh commented Jun 7, 2017

Do you have a NixOS machine with sandbox that you could test on?

@knedlsepp
Copy link
Member Author

Oh. Didn't expect that the NixOS build would fail. I'll spin up a VM and see what's the deal. I suppose it will be the same problem as on the mac, where I need to set $DYLD_LIBRARY_PATH for the checkPhase. The test-binaries don't seem to know that they should look for the library in the build directory.

@knedlsepp
Copy link
Member Author

Adding an "export LD_LIBRARY_PATH=$(pwd)/bin" seemed to help.

knedlsepp added 2 commits June 8, 2017 07:52

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@knedlsepp knedlsepp force-pushed the add-gdal-hdf4-support branch from f364a4c to 7b25d31 Compare June 8, 2017 07:52
@knedlsepp
Copy link
Member Author

@FRidh: Should work by now.

@FRidh FRidh merged commit 9c05499 into NixOS:master Jul 1, 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.

None yet

4 participants