Skip to content
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

[hdf5] Update HDF5 to 1.12 #11747

Merged
merged 18 commits into from
Jun 23, 2020
Merged

[hdf5] Update HDF5 to 1.12 #11747

merged 18 commits into from
Jun 23, 2020

Conversation

Neumann-A
Copy link
Contributor

and use gitlab live-clones/hdf5 instead of binary download

closes #11745

@Neumann-A Neumann-A marked this pull request as draft June 3, 2020 15:18
@cenit
Copy link
Contributor

cenit commented Jun 3, 2020

Personally, I am not a fan of unofficial urls.
If you want to download from repo, isn't it better to use vcpkg_from_git and point to https://bitbucket.hdfgroup.org/projects/HDFFV/repos/hdf5/browse ?

@Neumann-A
Copy link
Contributor Author

@cenit: give me a working example for vcpkg_from_git for hdf5 with the tag and I will change it. Otherwise I will not spend time on trying to get vcpkg_from_git to work ;)

ports/hdf5/portfile.cmake Outdated Show resolved Hide resolved
@cenit
Copy link
Contributor

cenit commented Jun 3, 2020

vcpkg_from_git is working, no need to touch it

I put it as a suggestion in the code review panel, copied also here

vcpkg_from_git(
    OUT_SOURCE_PATH SOURCE_PATH
    URL https://bitbucket.hdfgroup.org/scm/hdffv/hdf5.git
    REF 83385326ef1588167f2b37300313a6e51e51ffca
)

(there it's missing the close bracket in order to continue with your PATCHES keyword

@Neumann-A
Copy link
Contributor Author

@cenit: You actually tried the code?

Starting package 3/3: hdf5:x64-windows-static
Building package hdf5[core,szip,zlib]:x64-windows-static...
-- Fetching https://bitbucket.hdfgroup.org/scm/hdffv/hdf5.git...
CMake Error at scripts/cmake/vcpkg_execute_required_process.cmake:72 (message):
    Command failed: "C:/Program Files/Git/mingw64/bin/git.exe" fetch https://bitbucket.hdfgroup.org/scm/hdffv/hdf5.git 83385326ef1588167f2b37300313a6e51e51ffca --depth 1 -n
    Working Directory: D:/vcpkg_common/downloads/git-tmp
    Error code: 128
    See logs for more information:
      D:\qt2\buildtrees\hdf5\git-fetch-x64-windows-static-err.log
fatal: expected shallow/unshallow, got The client mentioned a shallow object that is missing on the server. This may be due to a rebase or other forced update on the server. You should recreate your shallow clone.
fatal: the remote end hung up unexpectedly

@cenit
Copy link
Contributor

cenit commented Jun 3, 2020

Working on my Pc
Anyway, feel free to do what you prefer. Just stating my opinion about unofficial repositories...

@NancyLi1013 NancyLi1013 self-assigned this Jun 4, 2020
@Neumann-A Neumann-A marked this pull request as ready for review June 4, 2020 19:19
@Neumann-A
Copy link
Contributor Author

@NancyLi1013: Can i assume that none of those regressions have anything to do with the changes made in this PR?

@NancyLi1013
Copy link
Contributor

@Neumann-A
Thanks for your kindly reminder. I will rerun this PR and confirm if those regression can be solved.
Currently, I'm not sure how the regression for field3d on x64-linux is caused.

/mnt/vcpkg-ci/buildtrees/field3d/src/v1.7.2-ae08f6d321/src/Field3DFileHDF5.cpp:1138:71: error: too few arguments to function ‘herr_t H5Oget_info_by_name3(hid_t, const char*, H5O_info2_t*, unsigned int, hid_t)’
   status = H5Oget_info_by_name(loc_id, itemName, &infobuf, H5P_DEFAULT);
                                                                       ^
In file included from /mnt/vcpkg-ci/install/x64-linux/include/H5Apublic.h:22:0,
                 from /mnt/vcpkg-ci/install/x64-linux/include/hdf5.h:23,
                 from /mnt/vcpkg-ci/buildtrees/field3d/src/v1.7.2-ae08f6d321/src/Field3DFileHDF5.cpp:50:
/mnt/vcpkg-ci/install/x64-linux/include/H5Opublic.h:188:15: note: declared here
 H5_DLL herr_t H5Oget_info_by_name3(hid_t loc_id, const char *name, H5O_info2_t *oinfo,
               ^~~~~~~~~~~~~~~~~~~~
/mnt/vcpkg-ci/buildtrees/field3d/src/v1.7.2-ae08f6d321/src/Field3DFileHDF5.cpp: In function ‘herr_t Field3D::v1_7::InputFileHDF5::parseLayers(hid_t, const char*, const H5L_info2_t*, void*)’:

   status = H5Oget_info_by_name (loc_id, itemName, &infobuf, H5P_DEFAULT);

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013
Copy link
Contributor

@Neumann-A
Seems the regression is related with this hdf5.
Could you please look into this?

@Neumann-A
Copy link
Contributor Author

@NancyLi1013: TODO for the future: Fix field3d hidden MPI dependency

@NancyLi1013 NancyLi1013 added the category:port-update The issue is with a library, which is requesting update new revision label Jun 8, 2020
@JackBoosY
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@cenit
Copy link
Contributor

cenit commented Jun 9, 2020

@NancyLi1013: TODO for the future: Fix field3d hidden MPI dependency

It’s already almost done in my opencv 4.3 pr

@JackBoosY
Copy link
Contributor

@cenit So, should we waiting for your PR #11130?

@cenit
Copy link
Contributor

cenit commented Jun 10, 2020

yes please. I just pushed the patch for field3d there.
Waiting for CI results, hoping for that PR to be done...

ports/field3d/portfile.cmake Show resolved Hide resolved
ports/hdf5/portfile.cmake Outdated Show resolved Hide resolved
ports/hdf5/portfile.cmake Outdated Show resolved Hide resolved
ports/hdf5/portfile.cmake Outdated Show resolved Hide resolved
@JackBoosY JackBoosY added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jun 11, 2020
@NancyLi1013
Copy link
Contributor

@cenit
Thanks for your attention and kindly reminder. Actually, we always recommend to use official repos except for some specific reasons. And now I noticed that @Neumann-A has updated as official repo for hdf5 in this PR. Really appreciate.

@NancyLi1013
Copy link
Contributor

@Neumann-A
Is there anything else to be done for this PR? If not, I will try to test these features.

@Neumann-A
Copy link
Contributor Author

@NancyLi1013: Nothing to add. Feel free to resolve the conflict with the baseline.

@JackBoosY
Copy link
Contributor

Can you merge the latest changes into this branch and use this to replace ci.baseline.txt?

@Neumann-A
Copy link
Contributor Author

@JackBoosY: I cleaned the baseline. If you need more changes than that those should probably be in another PR.

@JackBoosY JackBoosY added requires:testing Needs tests added before merging and removed requires:author-response labels Jun 16, 2020
@NancyLi1013
Copy link
Contributor

All features have passed with the following triplets:

  • x86-windows
  • x64-windows
  • x64-windows-static

Note: Feature fortran has not been supported yet.

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:testing Needs tests added before merging labels Jun 16, 2020
@Neumann-A
Copy link
Contributor Author

@JackBoosY @NancyLi1013´: Is there something blocking the merge of this PR?

@JackBoosY
Copy link
Contributor

@Neumann-A See #11747 (comment)

Waiting for merge #11130.

@dan-shaw
Copy link
Contributor

LGTM, just waiting for #11130 as suggested above. @Neumann-A Let us know if it makes more sense to merge this PR first.

@Neumann-A
Copy link
Contributor Author

@dan-shaw: seems like #11130 is still not finished so merging this one first seems more reasonable

@dan-shaw dan-shaw merged commit 23eadea into microsoft:master Jun 23, 2020
@Neumann-A Neumann-A deleted the update_hdf5 branch June 23, 2020 20:00
@cenit
Copy link
Contributor

cenit commented Jun 23, 2020

#11130 is finished...

BillyONeal added a commit to JackBoosY/vcpkg that referenced this pull request Jun 25, 2020
@BillyONeal
Copy link
Member

This change broke a lot of stuff :(

@Neumann-A
Copy link
Contributor Author

@BillyONeal. Instead of patching the sources use the following in the CMakeLists.txt:

add_definitions(-UH5_USE_112_API_DEFAULT)
add_definitions(-DH5_USE_110_API_DEFAULT)

there is probably also a way to set those via an environment variable from the portfile or using VCPKG_CXX_FLAGS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision depends:different-pr This PR or Issue depends on a PR which has been filed info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[hdf5] update to 1.12
6 participants