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

Testcases 1057 and 1129 fail when built against HDF5 1.10 configured with --with-default-api-version=v18 #41

Closed
pini-gh opened this issue Jul 22, 2016 · 42 comments

Comments

@pini-gh
Copy link

pini-gh commented Jul 22, 2016

Hi,

In the process of preparing the HDF5 1.10 transition in Debian, i've tried a build of Matio 1.5.8 against the HDF5 1.10 packages currently in Debian experimental. Matio builds fine but the test suite reports 2 failures :

  • 1057: Delete variables FAILED (mat73_readwrite.at:40)
  • 1129: Delete variables FAILED (mat73_compressed_readwrite.at:42)
    Please find attached an archive of the resulting testsuite.dir.

Note: this instance of HDF5 1.10 was configured with --with-default-api-version=v18.

Thanks.
testsuite-dir.tar.gz

@tbeu
Copy link
Owner

tbeu commented Jul 22, 2016

Thanks for reporting and providing the files. Need to check and update (Travis) CI to also build against HDF5 1.10. Unfortunately I'll be offline for the next week, hence it will take some more week to fix the issue (if possible).

@tbeu
Copy link
Owner

tbeu commented Jul 30, 2016

Can you please reproduce the problem with HDF5 v1.10.0-patch1 and current HEAD of matio? https://drone.io/github.com/tbeu/matio/547 gives different test failure (of same two tests though).

@pini-gh
Copy link
Author

pini-gh commented Jul 31, 2016

Same failure as https://drone.io/github.com/tbeu/matio/547. New testsuite.dir attached.
testsuite-dir.tar.gz

@tbeu
Copy link
Owner

tbeu commented Aug 2, 2016

I could reproduce on CentOS and investigated the issue. It looks like a problem of the new HDF5 1.10 lib, hence I contacted the HDF Group by mail and kindly asked for their support.

Hi,

I am the maintainer of C library matio (https://github.com/tbeu/matio) which has a dependency on libhdf5.

We noticed a problem on Linux when switching from HDF5 1.8.17 to 1.10.0-patch1 where suddenly two of the matio test cases fail (#41):

The problem is in function Mat_VarDelete where a v7.3 MAT file (which basically is a HDF5 file) is rewritten with one group less. The newly created temporary file (https://github.com/tbeu/matio/blob/master/src/mat.c#L886-L897) is all fine. But after copying the temporary file to the previous location (https://github.com/tbeu/matio/blob/master/src/mat.c#L915) using mat_copy (https://github.com/tbeu/matio/blob/master/src/mat.c#L816-L848), opening the new file (https://github.com/tbeu/matio/blob/master/src/mat.c#L921) and copying the structure members (https://github.com/tbeu/matio/blob/master/src/mat.c#L929), especially the file identifier of the opened file, it looks like this file is an invalid HDF5 file after closing. Please note that I introduced mat_copy by 647bb66 in order to not break any hard links to the orginal file.

To reproduce build hdf5 and matio and run the failing test in directory matio/test:
$ ./test_mat copy -v 7.3 datasets/matio_test_cases_hdf_le.mat
$ ./test_mat delete test_mat_copy.mat var2

I provide the invalid file test_mat_copy.mat (http://tbeu.de/test_mat_copy.mat) and the valid temporary file tmp.mat (http://tbeu.de/tmp.mat) for your convenience. The difference is only two bytes at 0x0228 and 0x0229.

Can you please investigate and confirm. Thank you for your support.

Best regards,
tbeu

@pini-gh
Copy link
Author

pini-gh commented Aug 12, 2016

Hi,
Has the HDF Group replied with a HDFFV issue number?
Thanks.

@tbeu
Copy link
Owner

tbeu commented Aug 12, 2016

They did (w/o issue no) and wanted to investigate. Need to ask them about progress or update. Where do you see the issue numbers?

@tbeu
Copy link
Owner

tbeu commented Aug 22, 2016

The HDF5 group asked for a stand-alone test case merely depending on the HDF5 library. This was my reply from today:

Hi,

thank you for your feedback.

I created a stand-alone application test41 that just simply uses the HDF5 library. Please copy https://gist.github.com/tbeu/b31c50fa11587a21675c211571116ca4 to a source file test41.c and build it with

$ gcc -DHAVE_HDF5=1 test41.c -o test41 -lm -lhdf5

You probably need to specify include and lib pathes for the HDF5 library.

Now copy matio_test_cases_hdf_le.mat from https://sourceforge.net/p/matio/matio_test_datasets/ci/master/tree/ to your test41 directory and run

$ ./test41 matio_test_cases_hdf_le.mat var2

to first copy matio_test_cases_hdf_le.mat to test_mat_copy.mat and then delete variable var2 from the copied file.

You can inspect the resulting MAT file test_mat_copy.mat by a debug tool like h5dump and you will see that using HDF5 1.8.17 results in a valid MAT file test_mat_copy_18.mat (http://tbeu.de/test_mat_copy_18.mat) and using HDF5 1.10.0 results in an invalid MAT file test_mat_copy_110.mat (http://tbeu.de/test_mat_copy_110.mat).

HDF5 1.8.17 was configured with

$ ./configure --quiet --enable-shared --enable-production --enable-debug=no --disable-deprecated-symbols --disable-hl --with-pic --with-default-api-version=v18 CFLAGS="-w"

and HDF5 1.10.0 was configured with

$ ./configure --quiet --enable-shared --enable-build-mode=production --enable-deprecated-symbols=yes --disable-hl --with-pic --with-default-api-version=v110 CFLAGS="-w"

If you need some more details just let me know.

By the way, running test41 on Windows using the pre-built libraries from https://www.hdfgroup.org/ftp/HDF5/releases/hdf5-1.10/hdf5-1.10.0-patch1/bin/windows/extra/ and using Visual Studio 2015 also gives errors, but different ones. Thus I believe there is some new issue apparent in HDF5 1.10.0.

Thank you for caring.

Best regards,
tbeu

@tbeu
Copy link
Owner

tbeu commented Aug 23, 2016

And this is the command to build test41 using Visual Studio 2015 x86 command prompt

cl /MD /DHAVE_HDF5=1 /DH5_BUILT_AS_DYNAMIC_LIB /I"c:\Program Files (x86)\HDF_Group\HDF5\1.10.0\include" test41.c /link /DEFAULTLIB:"c:\Program Files (x86)\HDF_Group\HDF5\1.10.0\lib\hdf5.lib"

@tbeu
Copy link
Owner

tbeu commented Aug 24, 2016

One more answer

Hi,

I know that there a many additional calls beside the HDF5 API calls. I already spent several hours on this problem an can give you the following clues again.

Now I am out of any more ideas and do hope that you will have another look on this case.

Best regards,
tbeu

@tbeu
Copy link
Owner

tbeu commented Sep 2, 2016

HDFFV-9983 is the (non-public) bug number.

@pini-gh
Copy link
Author

pini-gh commented Sep 25, 2016

I've pinged the HDF Group support desk. No news, but they promised to have a look as soon as they can.

@tbeu
Copy link
Owner

tbeu commented Sep 25, 2016

Yes, I did the same in the last weeks. So far nobody did investigate it.

@pini-gh
Copy link
Author

pini-gh commented Oct 11, 2016

Hi Thomas,
What is the status of this ticket? Have you find a worlkround using h5repack(), as suggested by the HDF Group support desk? I'm worried because the transition freeze for Debian Stretch is very soon now: 5th November 2016.
Thx.

@tbeu
Copy link
Owner

tbeu commented Oct 17, 2016

I wondered why I could no longer reproduce the issue (neither on CentOS nor on Win) until I figured out that 38f50fd for #44 also resolves this issue.

@pini-gh
Copy link
Author

pini-gh commented Oct 18, 2016

That's very good news indeed :)
Will you release a new version soon?
Thanks.

@tbeu
Copy link
Owner

tbeu commented Oct 18, 2016

Hope so (before your deadline). But need to work on two more other issues before making the new release.

@pini-gh
Copy link
Author

pini-gh commented Oct 18, 2016

My deadline is somewhat reached already: I have a lot of work to do before requesting a transition slot from Debian Release Team, and ideally this request should be made very soon, in the next few days.
What do you think about cherry-picking 38f50fd for the current Matio release in Debian unstable? It might be the best option I have...
Thanks.

@tbeu
Copy link
Owner

tbeu commented Oct 18, 2016

So what is your exact deadline for matio? There is more than this critical commit.

@pini-gh
Copy link
Author

pini-gh commented Oct 18, 2016

I don't know exactly. The transition freeze is the 5th of november. But if I submit my request the 4th, it is very likely it will be dismissed, so close of the deadline. HDF5 has 65+ reverse dependencies. The sooner I submit, the more chance I have the request will be accepted.
If I start rebuilding all the reverse depends this evening, I can be done by Sunday. Would you have something by then?
Thinking about it, a patch against the current matio release in Debian is the best option: a new matio release would have to transition before hdf5.

@tbeu
Copy link
Owner

tbeu commented Oct 18, 2016

Ok, I'll hurry up such that you have some new release v1.5.9 the next 2 or 3 days. (Just fixed cc03271 and only #40 left open for read-write mode.)

By the way, previously I was in contact with @sebastien-villemot for some Debian related lib maintenance - basically only when it failed.

@svillemot
Copy link
Contributor

@pini-gh I am indeed the maintainer of libmatio in Debian. Feel free to open a bug against the package with the patch, I'll try to handle it quickly. Or even better, don't hesitate to do a team upload directly if you're member of the Debian Science Team.

@tbeu
Copy link
Owner

tbeu commented Oct 18, 2016

From the NEWS:
Changes in upcoming 1.5.9 (xx October 2016)

  1. Fixed resource leak when reading character data from HDF5 MAT file
  2. Fixed bug writing struct to HDF5 MAT file (Mat_VarWrite never returned 0 on success)
  3. Fixed bug writing sparse logical array to HDF5 MAT file
  4. Fixed bug calculating array sizes of structs, cells, complex and sparse arrays in Mat_VarGetSize
  5. Fixed bug duplicating sparse array with empty data in Mat_VarDuplicate
  6. Fixed segmentation fault when reading compressed v5 MAT file with opaque class
  7. Updated support of HDF5 v1.10.x (no longer depend on deprecated v1.8.x symbols)

If you cannot wait I recommend to patch at least 7. (e387271) beside 1. (38f50fd).

@tbeu
Copy link
Owner

tbeu commented Oct 19, 2016

FYI: I plan to release v1.5.9 tonight.

@tbeu tbeu closed this as completed Oct 19, 2016
@tbeu tbeu added the duplicate label Oct 19, 2016
@pini-gh
Copy link
Author

pini-gh commented Oct 20, 2016

Thanks Thomas!

@tbeu
Copy link
Owner

tbeu commented Oct 22, 2016

v1.5.9 is now (that GitHub and Travis CI are back to stable) released.

@svillemot
Copy link
Contributor

@pini-gh I have uploaded version 1.5.9 to Debian. However the package is not yet accepted, because of the ongoing maintenance operation on ftp-master.

@tbeu
Copy link
Owner

tbeu commented Oct 22, 2016

@sebastien-villemot Make sure it is the latest v1.5.9 (needed to re-tag about 20 min ago).

@svillemot
Copy link
Contributor

svillemot commented Oct 22, 2016

@tbeu Oh no… Here is the one I uploaded:
$ md5sum matio-1.5.9.tar.gz
6cdb2f1ff549129068182d09ea5bb2c4 matio-1.5.9.tar.gz

@svillemot
Copy link
Contributor

I'm trying to cancel my upload, maybe it's not too late. Otherwise I'll have to upload the new 1.5.9 tarball under a different version number (like 1.5.9.1 or 1.5.9+real or something like this).

@tbeu
Copy link
Owner

tbeu commented Oct 22, 2016

That is the one.
aab5b4219a3c0262afe7eeb7bdd2f463 *matio-1.5.9.tar.gz
Sorry for that.

@svillemot
Copy link
Contributor

Fortunately, thanks to the temporary disruption of Debian services, I was able to cancel my previous upload.

The right libmatio 1.5.9 has now been uploaded and accepted in Debian.

@tbeu
Copy link
Owner

tbeu commented Oct 24, 2016

Thanks for letting me know.

@pini-gh
Copy link
Author

pini-gh commented Oct 24, 2016

Thanks Sébastien.

@tbeu
Copy link
Owner

tbeu commented Nov 7, 2016

The transition freeze is the 5th of november.

@pini-gh Being curious, did you manage to submit your hdf5 1.10 update?

@pini-gh
Copy link
Author

pini-gh commented Nov 7, 2016

@tbeu It's on good track. I've got an ACK from the release team and they told me I should have a GO in a few days. They want to finalize some other ongoing transitions first.

@tbeu
Copy link
Owner

tbeu commented Nov 7, 2016

Sound good! Fingers crossed!

@pini-gh
Copy link
Author

pini-gh commented Nov 29, 2016

@tbeu: hdf5 1.10 has landed into Debian unstable yesterday. The transition ongoing.

@tbeu
Copy link
Owner

tbeu commented Nov 29, 2016

I was told that new hdf5 1.10 release is expected for Feb.

@pini-gh
Copy link
Author

pini-gh commented Nov 29, 2016

The HDF Group told me 1.10.1 will be released in January. But this will be too late for Debian Stretch.

@tbeu
Copy link
Owner

tbeu commented Mar 6, 2017

Updated hdf5 1.10.1 seems to be delayed to any time later.

@pini-gh
Copy link
Author

pini-gh commented Mar 6, 2017

Indeed. They now say "by the end of March".

@tbeu
Copy link
Owner

tbeu commented May 1, 2017

FYI: hdf5 v1.10.1 was released May 1st 2017.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants