Re: the ScriptingMojo

Hi Kartik,

Taking an early look:

- I think the runCommand method has kind of a misleading name for what it
does, as it only gives you a ProcessBuilder for you to set up for
running your command. Maybe it would be better to just call that one
... processBuilder?

- All the same, I think a runCommand method would also be good to have;
that would be what you call after getting your ProcessBuilder from
processBuilder and configuring it for the command you want.

That method would take care of ProcessBuilder.start() and waiting for
completion and checking the exit status. I think there would be few
times the script code would want to do those things separately (and if
it ever did, it could anyway; no one is forcing it to use runCommand).

Later on, it will be even better to have runCommand, as we will
eventually want not to set the process stdout and stderr to INHERIT,
but to have them as pipes and read them. At that point, runCommand should
have an additional parameter, some JavaScript lambda that will be
called back with messages read from the process, and will classify the
messages as error/warning/info and pass them to the right Maven log
method. That should be done in script because the patterns used to
classify the messages will almost certainly need to be tailored
for each compiler/linker.

That will be a little tricky, because runCommand will need to use
more than one thread, to be able to read both streams from the process
and wait for the process to exit. But the script engine does not
want to be shared between threads. So runCommand might create a
concurrent queue, start threads that will read the process output and
place messages on the queue, while runCommand just reads from the queue
and passes messages to the script callback, until the process is done.
But that can be later; just letting stdout and stderr be INHERIT is
fine for now.

- Once there is processBuilder and runCommand, processBuilder can set a
default for the working directory before it returns the ProcessBuilder,
but I would move the existence test and directory creation into
runCommand; no need to do the work up front when the caller might
change the directory setting.

- I was thinking of simplifying the parameters to processBuilder, to not
need the caller to separately pass the command string and a list of the
rest of the arguments. An idea that just hit me was why not just
processBuilder(Consumer<List<String>>)? A script could use it like:

var pb = processBuilder(function(l) {
l.add(command);
l.addAll(stuff);
l.add(thing);
...
});

so the script would avoid the messiness of allocating its own ArrayList
and so on. On the Java side that's just

pb = new ProcessBuilder();
callback.accept(pb.command());
...
return pb;

Once there is a platform/OS 'dispatcher object' structure, I think the
common methods compile(...), link(...), and so on, need their parameter
lists carefully thought out. One parameter to compile(), for example,
should just be a list of includes, without the -I; the method needs to
know the right syntax for its compiler. Another parameter should be a
simple map <name,value> of defines; again the method needs to know if
-Dname=value is right for its compiler. Similar considerations for link().

It might be worthwhile to define an abstract class in Java that has
the necessary methods; it could also supply some other useful methods
or default implementations. I think the javascript could then do something
like:

var platforms = {
'Linux': { compile: function(...) { ... }, link: function... },
'Windows' : { ... },
...
}

var impl = new AbstractPGXS(platforms['Linux']);

which would enforce that the needed methods were all present, and the
ones supplied in Java would be naturally available too. I have not
tried this out to make sure it does what I expect in both nashorn
and graal.

Regards,
-Chap