scilab: hdf5 is broken in Scilab 6.0 on bionic

BUG DESCRIPTION:
----------------
Not able to save or load a variable

ERROR LOG:
----------
HDF5-DIAG: Error detected in HDF5 (1.10.0-patch1) thread 140383204820864:
  #000: ../../../src/H5G.c line 553 in H5Gget_info(): invalid argument
    major: Invalid arguments to routine
    minor: Bad value
HDF5-DIAG: Error detected in HDF5 (1.10.0-patch1) thread 140383204820864:
  #000: ../../../src/H5D.c line 121 in H5Dcreate2(): not a location ID
    major: Invalid arguments to routine
    minor: Inappropriate type
  #001: ../../../src/H5Gloc.c line 253 in H5G_loc(): invalid object ID
    major: Invalid arguments to routine
    minor: Bad value
HDF5-DIAG: Error detected in HDF5 (1.10.0-patch1) thread 140383204820864:
  #000: ../../../src/H5F.c line 749 in H5Fclose(): not a file ID
    major: Invalid arguments to routine
    minor: Inappropriate type
failed to close file

HOW TO REPRODUCE THE BUG:
-------------------------
 - install a brand new debian testing
 - install scilab 6.0.0 package and dependencies
 - create any variable (e.g. "stack")
 - save it with:
         save("stack.sod","stack")

OTHER INFORMATION:
------------------
Scilab also fails to load the ATOMS manager, and it seems to be bound to that problem.

*** Bug 15738 has been marked as a duplicate of this bug. ***

I confirm that this bug is also present under Ubuntu 18.04 64bits.

Antoine

Antoine, could you please confirm here that this bug is now fixed on the 6.0-branch and on the master one, for this OS, as you did on users@?
Thanks

PhiRG,
Could you please try on Debian with the nightly build: http://www.scilab.org/en/development/nightly_builds/branch60
Thanks

This bug is resolved scilab-branch-6.0-1534494461 and scilab-master-1534776009, at least for Ubuntu 18.04 64bits.

Antoine

Hello Samuel,

Well done, it works with scilab-branch-6.0-1534494461 now.

The atoms manager opens too now.

Thank you very much for your work!

Philippe

There are plenty of users who install distribution packages believing they are "official" packages that still fail with the same problem. We cannot always say them to install the .tar.gz

@Stéphane: I don't understand your rationale for reopening this report.
Do you mean that there are some distributions packaging the official Scilab 6.0.2, with which the reported problem still occur?

Currently, no mainstream dist (Ubuntu, Fedora, ...) packaged Scilab 6.0.2 because the packaged 6.0.1 version does not work already. Just try the following pair of keywords on Google:

Scilab Ubuntu
Scilab Fedora
...

In fact, Scilab 6.0.2 and the master can be built w.r.t hdf5 1.10.n, but at runtime the problem of bug #15394 still occurs.
The only version that is supported in Scilab is hdf5 1.8. The problem is that in recent distributions, the matio lib is too recent and absolutely needs hdf5 1.10.n (I have this problem also in OSX when I use brew as a packaging system).

The problem is that package maintainers are not informed of this strict dependency :-(

(In reply to Stéphane MOTTELET from comment #9)
> Currently, no mainstream dist (Ubuntu, Fedora, ...) packaged Scilab 6.0.2 because the packaged 6.0.1 version does not work already. Just try the
> following pair of keywords on Google:
>
> Scilab Ubuntu
> Scilab Fedora
> ...

Google is not always up to date :) ; "6.0.1" was mostly reported as buggy due to its startup and search results are also cluttered by it. Scilab 6.0.2 is available as a Fedora package and atomsSystemUpdate works (I just checked) cause Fedora did not update to hdf5 1.10 yet.

Mixing packaging issues with upstream bugs seems a bad idea to me, it will make us responsible for packaging Scilab in systems that are free to update/patch/deprecates/etc... If a distribution package wished to upgrade to hdf5 1.10 for some reason, that's ok but when issues arise we could only try to help them fix things on their side and work together to find the root cause (is it hdf5, is it Scilab, is it some obscure compilation mismatch)

> In fact, Scilab 6.0.2 and the master can be built w.r.t hdf5 1.10.n, but at runtime the problem of bug #15394 still occurs.
> The only version that is supported in Scilab is hdf5 1.8. The problem is that in recent distributions, the matio lib is too recent and absolutely
> needs hdf5 1.10.n (I have this problem also in OSX when I use brew as a packaging system).

And that's the whole point, hdf5 1.10 seems to have an issue at runtime and matio started to require it. I clarified the summary and let's work on that :)

# Intro

The text below is not related to official binary builds of Scilab (6.0.2 and latest from https://build.scilab.org/view/Scilab%20master/job/scilab-master-linux-64/ ).
They are not affected by the problem as they continue to use HDF 1.8.14 (see `lib/thirdparty/libhdf5.settings` for confirmation).

# Problem

The rpm- and deb- packages in GNU/Linux distributions have broken `save` and `load` functionality.

Currently many Linux distributions switched to HDF 1.10.x branch:

| Distibution | Scilab | HDF5 | `save`/ `load` |
| ------------------- | ------ | ------ | ------------------------ |
| Debian 10 Buster | 6.0.1 | 1.10.4 | !!! bug !!! |
| Ubuntu 18.04 LTS | 6.0.1 | 1.10.0 | !!! bug !!! (LP#1765503) |
| Ubuntu 19.04 | 6.0.2 | 1.10.4 | !!! bug !!! (LP#1765503) |
| Fedora RawHide | 6.0.2 | 1.10.5 | !!! bug !!! |
| Mageia 7 | 6.0.2 | 1.10.5 | !!! bug !!! |
| OpenSuSe Leap 15.1 | 6.0.2 | 1.10.4 | !!! bug !!! |
| OpenSuSe Tumbleweed | 6.0.2 | 1.10.4 | !!! bug !!! |

You can see Scilab packaged version in distributions on Repology - https://repology.org/project/scilab/packages .

Below is a text output of `test_run('hdf5')` from Ubuntu 18.04.2 LTS without Details section as example:

```
--> test_run('hdf5')
   TMPDIR = /tmp/SCI_TMP_213_5dIgF3

 Module 001/001 - [hdf5] :

   001/046 - [hdf5] binary_bit-for-bit..........................failed: Slave Scilab exited with error code 1
   002/046 - [hdf5] h5attr......................................failed: Slave Scilab exited with error code 1
   003/046 - [hdf5] h5close.....................................failed: Slave Scilab exited with error code 1
   004/046 - [hdf5] h5cp........................................failed: error_output not empty
                                                                 Use 'no_check_error_output' option to disable this check.
   005/046 - [hdf5] h5dataset...................................failed: error_output not empty
                                                                 Use 'no_check_error_output' option to disable this check.
   006/046 - [hdf5] h5exists....................................failed: Slave Scilab exited with error code 1
   007/046 - [hdf5] h5flush.....................................failed: error_output not empty
                                                                 Use 'no_check_error_output' option to disable this check.
   008/046 - [hdf5] h5group.....................................failed: error_output not empty
                                                                 Use 'no_check_error_output' option to disable this check.
   009/046 - [hdf5] h5isAttr....................................failed: Slave Scilab exited with error code 1
   010/046 - [hdf5] h5isFile....................................failed: Slave Scilab exited with error code 1
   011/046 - [hdf5] h5isGroup...................................failed: Slave Scilab exited with error code 1
   012/046 - [hdf5] h5isRef.....................................failed: Slave Scilab exited with error code 1
   013/046...

Created attachment 4992
Patches for Ubuntu (18.04 LTS and 19.04) and master branch

# Intro

I have analyzed the source code of the `modules/hdf5` folder.
I have used KDevelop IDE for doing so.
It has full feature *Semantic code* analyzer ( https://www.kdevelop.org/screenshots/c-semantic-information-tooltip ).
Without KDevelop you can detect problems with datatypes after adding `-Wconversion`
to the CFGLAGS and CXXFLAGS variables inside `modules/hdf5/Makefile`.

Usually the warning messages from GCC look like:

```
src/c/h5_writeDataToFile.c: In function 'writeStringMatrix':
src/c/h5_writeDataToFile.c:296:27: warning: conversion to 'int' from 'hid_t {aka long int}' may alter its value [-Wconversion]
     status = addAttribute(dset, g_SCILAB_CLASS, g_SCILAB_CLASS_STRING);
                           ^~~~
...
src/c/h5_fileManagement.c: In function 'openHDF5File':
src/c/h5_fileManagement.c:161:12: warning: conversion to 'int' from 'hid_t {aka long int}' may alter its value [-Wconversion]
     return file;
            ^~~~
...
src/c/h5_fileManagement.c: In function 'createHDF5File':
src/c/h5_fileManagement.c:101:12: warning: conversion to 'int' from 'hid_t {aka long int}' may alter its value [-Wconversion]
     return file;
            ^~~~
sci_gateway/cpp/sci_hdf5_listvar_v3.cpp: In function 'bool read_struct(int, VarInfo6&)':
sci_gateway/cpp/sci_hdf5_listvar_v3.cpp:738:42: warning: conversion to 'int' from 'hid_t {aka long int}' may alter its value [-Wconversion]
                 int data = H5Rdereference(refs,
```

and so on.

# Patching

I attach 3 patches to fix HDF-related problems in Ubuntu (18.04 LTS and 19.04) and in master branch.

## Ubuntu 18.04 LTS

Applying patch is as follows:

```
sudo apt-get build-dep scilab
cd ~/Downloads
apt-get source scilab
cd scilab-6.0.1
dpkg-builpackage -b -j$(nproc) # build to see current state
cd modules/hdf5
patch -p1 < ~/Downloads/scilab-6.0.1-ubuntu-bionic.patch
cd ../..
dpkg-builpackage -b -j$(nproc) # build to see changes
```

As the patch is applied then we can test patched Scilab.
The results from `test_run('hdf5')` are the following:

```
-->test_run('hdf5')
   TMPDIR = /tmp/SCI_TMP_2556_AWXxWF

 Module 001/001 - [hdf5] :

   001/046 - [hdf5] binary_bit-for-bit..........................passed
   002/046 - [hdf5] h5attr......................................passed
   003/046 - [hdf5] h5close.....................................passed
   004/046 - [hdf5] h5cp........................................passed
   005/046 - [hdf5] h5dataset...................................passed
   006/046 - [hdf5] h5exists....................................passed
   007/046 - [hdf5] h5flush.....................................passed
   008/046 - [hdf5] h5group.....................................passed
   009/046 - [hdf5] h5isAttr....................................passed
   010/046 - [hdf5] h5isFile....................................passed
   011/046 - [hdf5] h5isGroup...................................passed
   012/046 - [hdf5] h5isRef.....................................passed
   013/046 - [hdf5] h5isSet.....................................passed
   014/046 - [hdf5] h5isSpace...................................pa...

*** Bug 16287 has been marked as a duplicate of this bug. ***

Nikolay why don't you commit you patches on CR ?

As per the analysis of Nikolay, the impact of this bug means that ATOMS and many other functions are not available to Ubuntu users.

I've tried installing the latest successful nightly build, but that doesn't work as the customization for Ubuntu/Debian is missing and too many environment issues arose. E.g. when starting scilab from the command line, nothing happens, no output, no CPU load. The only think you can do is stop the process (^Z) and then kill the stopped job. I couldn't run the stable 6.0.2. version either.

Next, I've tried building from source, but here I couldn't get past the JOGL and javah issue, also reported by Nikolay (the javah issue I could work around, but I couldn't find a fix for the JOGL issue).

Effectively, this means that I don't have means to run Scilab, and I guess with me also most of the other Ubuntu/Debian users.

@Stepahne, is there something you can do to raise the awareness on this issue and take measures for a big fix release being pushed to the Ubuntu package maintainers?

Thanks and regards, Paul

(In reply to Paul van Haren from comment #15)
> As per the analysis of Nikolay, the impact of this bug means that ATOMS and many other functions are not available to Ubuntu users.
>
> I've tried installing the latest successful nightly build, but that doesn't work as the customization for Ubuntu/Debian is missing and too many
> environment issues arose. E.g. when starting scilab from the command line, nothing happens, no output, no CPU load. The only think you can do is
> stop the process (^Z) and then kill the stopped job. I couldn't run the stable 6.0.2. version either.
>
> Next, I've tried building from source, but here I couldn't get past the JOGL and javah issue, also reported by Nikolay (the javah issue I could
> work around, but I couldn't find a fix for the JOGL issue).
>
> Effectively, this means that I don't have means to run Scilab, and I guess with me also most of the other Ubuntu/Debian users.
>
> @Stepahne, is there something you can do to raise the awareness on this issue and take measures for a big fix release being pushed to the Ubuntu
> package maintainers?
>
> Thanks and regards, Paul

If think there is a simpler fix than patching all relevant files:

https://github.com/HDFGroup/HDF.PInvoke/wiki/Important-Differences-between-HDF5-1.8-and-1.10

(In reply to Paul van Haren from comment #15)
> As per the analysis of Nikolay, the impact of this bug means that ATOMS and many other functions are not available to Ubuntu users.
>
> I've tried installing the latest successful nightly build, but that doesn't work as the customization for Ubuntu/Debian is missing and too many
> environment issues arose. E.g. when starting scilab from the command line, nothing happens, no output, no CPU load. The only think you can do is
> stop the process (^Z) and then kill the stopped job. I couldn't run the stable 6.0.2. version either.
>
> Next, I've tried building from source, but here I couldn't get past the JOGL and javah issue, also reported by Nikolay (the javah issue I could
> work around, but I couldn't find a fix for the JOGL issue).
>
> Effectively, this means that I don't have means to run Scilab, and I guess with me also most of the other Ubuntu/Debian users.
>
> @Stepahne, is there something you can do to raise the awareness on this issue and take measures for a big fix release being pushed to the Ubuntu
> package maintainers?
>
> Thanks and regards, Paul

If think there is a simpler fix than patching all relevant files:

https://github.com/HDFGroup/HDF.PInvoke/wiki/Important-Differences-between-HDF5-1.8-and-1.10(In reply to Stéphane MOTTELET from comment #16)
> (In reply to Paul van Haren from comment #15)
> > As per the analysis of Nikolay, the impact of this bug means that ATOMS and many other functions are not available to Ubuntu users.
> >
> > I've tried installing the latest successful nightly build, but that doesn't work as the customization for Ubuntu/Debian is missing and too many
> > environment issues arose. E.g. when starting scilab from the command line, nothing happens, no output, no CPU load. The only think you can do is
> > stop the process (^Z) and then kill the stopped job. I couldn't run the stable 6.0.2. version either.
> >
> > Next, I've tried building from source, but here I couldn't get past the JOGL and javah issue, also reported by Nikolay (the javah issue I could
> > work around, but I couldn't find a fix for the JOGL issue).
> >
> > Effectively, this means that I don't have means to run Scilab, and I guess with me also most of the other Ubuntu/Debian users.
> >
> > @Stepahne, is there something you can do to raise the awareness on this issue and take measures for a big fix release being pushed to the Ubuntu
> > package maintainers?
> >
> > Thanks and regards, Paul
>
> If think there is a simpler fix than patching all relevant files:
>
> https://github.com/HDFGroup/HDF.PInvoke/wiki/Important-Differences-between-HDF5-1.8-and-1.10

I mean, something similar

Dear Stephane,

I think you're overlooking the observation by Nikolay (close to the bottom of comment 11):

# Brief analysis

I believe that root cause of the problem is change of `hid_t` datatype in HDF library.

It is not an `int` anymore, it is `int64_t`.
But the scilab source code have many places with hardcoded `int`.
See for example `openHDF5File` - http://cgit.scilab.org/scilab/tree/scilab/modules/hdf5/src/c/h5_fileManagement.c which calls `H5Fopen`:

```
int openHDF5File(const char *name, int _iAppendMode)
{
    hid_t file;
...

    file = H5Fopen(filename, H5F_ACC_RDONLY, H5P_DEFAULT);
...
    return file;
}
```
Best regards, Paul

(In reply to Paul van Haren from comment #18)
> Dear Stephane,
>
> I think you're overlooking the observation by Nikolay (close to the bottom of comment 11):
>
> # Brief analysis
>
> I believe that root cause of the problem is change of `hid_t` datatype in HDF library.
>
> It is not an `int` anymore, it is `int64_t`.
> But the scilab source code have many places with hardcoded `int`.
> See for example `openHDF5File` - http://cgit.scilab.org/scilab/tree/scilab/modules/hdf5/src/c/h5_fileManagement.c which calls `H5Fopen`:
>
> ```
> int openHDF5File(const char *name, int _iAppendMode)
> {
> hid_t file;
> ...
>
> file = H5Fopen(filename, H5F_ACC_RDONLY, H5P_DEFAULT);
> ...
> return file;
> }
> ```
> Best regards, Paul

I have understood the problem and one of the possible fixes is to redefine hid_t to int (as in the link I gave) as hdf5 1.10 seems to accept it. But I don't understand the logic of Nikolay's patch. I see for example

- hid_t iFile = openHDF5File(filename.data(), 0);
+ int iFile = openHDF5File(filename.data(), 0);

In the piece of code your gave, the logic would be to define the output type of openHDF5File as hid_t. If you declare the "file" id as an int the destructive conversion will occur in the call "file = H5Fopen..." since H5Fopen() yields an hid_t. So instead of changing type in the (many) calls it is simpler to change the output type of the function.

Definitive fix under review @ https://codereview.scilab.org/#/c/21179/

Merged @ http://gitweb.scilab.org/?p=scilab.git;a=commit;h=23ed74868f447cf4f2f0ed54409d8e2a8f86f48d