fix(value): don't assume that zero-terminated array is of type gchar**

[peat-psuwit]

The array's element type can be arbitrary, so we need to check the value
with the correct type. The long switch and a template function is used
to do this job.

We can't just memcmp() with item_size, because one of the type is float,
and we can't assume a 0.0 float is the same as 0 integer.

A test, based on how I discovered this, is added.

[romgrk]

This seems a bit hard to read. Can we clearly separate zero-terminated and non-zero terminated cases?

[peat-psuwit]

Does the current version look better? My other alternative is:

    if (length >= 0) {
        for (int i = 0; i < length; i++) {
            void** pointer = (void**)((ulong)data + i * item_size);
            memcpy(&value, pointer, item_size);

            Nan::Set(array, i, GIArgumentToV8(item_type_info, &value));
        }
    } else {
        for (int i = 0; ; i++) {
            void** pointer = (void**)((ulong)data + i * item_size);
            memcpy(&value, pointer, item_size);

            if (isZero(value, item_type_info))
                break;

            Nan::Set(array, i, GIArgumentToV8(item_type_info, &value));
        }
    }

But this feels silly.

[romgrk]

No, current version still feels off :|
The alternative is nice because it avoid unnecessary comparisons, but I don't think it matters enough. So anyway put it back however you want, if you could just assign bool isZeroTerminated = length == -1; so things are clear for whoever reads it.

[peat-psuwit]

I don't understand the test failure on macos.

     Failed: OUT-array (zero-terminated, of guint8)
TypeError: Not enough arguments; expected 2, have 1
    at /Users/runner/work/node-gtk/node-gtk/tests/conversion__array.js:56:23
...

The annotation of the function is:

 * g_spawn_command_line_sync:
 * @command_line: (type filename): a command line
 * @standard_output: (out) (array zero-terminated=1) (element-type guint8) (optional): return location for child output
 * @standard_error: (out) (array zero-terminated=1) (element-type guint8) (optional): return location for child errors
 * @wait_status: (out) (optional): return location for child wait status, as returned by waitpid()
 * @error: return location for errors

So the JS version should take only 1 argument, the command_line. Besides, the test works on my machine running Ubuntu 20.04. Maybe it comes from that glib on the macos one is 2.70. I don't have a Mac that can debug this.

[romgrk]

Can't see the tests, can you rebase on master to fetch the updated workflow yaml file?

For the issue, maybe you could detect the OS or glib version, and provide the missing argument depending on that?

[peat-psuwit]

Can't see the tests, can you rebase on master to fetch the updated workflow yaml file?

Done. Please see the result.

For the issue, maybe you could detect the OS or glib version, and provide the missing argument depending on that?

The problem is, I don't know what I'm supposed to pass in as the second argument. Without a Mac, I can't take a look at the Gir file or use a debugger to find out what the second argument is.

[romgrk]

You can add the following line somewhere in the CI script to see the metadata for that function:

node -p "require('./lib/inspect').parseNamespace('GLib').spawn_command_line_sync"

[peat-psuwit]

    ArgInfo {
      infoType: 'arg',
      name: 'exit_status',
      parent: [Circular *1],
      typeName: 'gint32',
      type: [TypeInfo],
      direction: 'IN',
      transfer: 'NOTHING'
    }

Huh?

[peat-psuwit]

So, I got myself a macOS VM, and discovered a problem with Homebrew's gobject-introspection bottle. Put a workaround to rebuild that from source, and now the CI is green.

[romgrk]

Thanks :)

@binyamin: I'm very inconsistent in my presence on github due to lack of time :( As you have write access to the repo, I bless you with the power to merge PRs if you feel like they're complete. I can also give you access to NPM so you can publish in case I'm not around.
@peat-psuwit if you want write access the above also applies to you as you've been doing good contributions to the project.