public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kartik Ohri <[email protected]>
To: Chapman Flack <[email protected]>
Cc: [email protected]
Subject: Re: PL/Java new build plugin
Date: Wed, 12 Aug 2020 04:19:53 +0530
Message-ID: <CAASLQ4O3ONhDr-XgwDDD3oW+x2VwCDTmuu-T+fz-jcZ=T23zuw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAASLQ4Mva7dAXiS_c9OLhV4VzNHF=XNKJosuS5gtZTB6b4TW1Q@mail.gmail.com>
<[email protected]>
<CAASLQ4OknEQTpSeoBfyAZR1na-EyL+AtN9t0fPGyv5DfWJOSWA@mail.gmail.com>
<CAASLQ4NHROvOq-rUz6Jn0dUd7oAq3_P655WNeqKksNBqFtAGgw@mail.gmail.com>
<CAASLQ4OCLUYXpkk0tTioz8=zc4UEbAm-Whgp_5K7uy-5hZdjKA@mail.gmail.com>
<[email protected]>
<CAASLQ4M8cN20L7cyVEgks9qdnfsXH4ci7iQbmyi4v_y0W85eLA@mail.gmail.com>
<[email protected]>
<CAASLQ4MLFnD3v_fqrUj+Bwc9MSBG-ATet3J7UXebzXHTH_si3A@mail.gmail.com>
<[email protected]>
<CAASLQ4O5Wfk1hH6nO_1PDUbRcpUywvTxq7e99aCJ4RPeMmssAw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAASLQ4NzHE-YO9XYTQSqHEEz9UQm8dRq1d-jbdAFPVATFiLN2w@mail.gmail.com>
<CAASLQ4OLhgc=MMMOWv2sxt93xC1Te4vwThBgEy7=QBBz0dEn6g@mail.gmail.com>
<[email protected]>
<CAASLQ4NA76safp+fa__9kMo2sG__EiwGdViFECgQ4wfL3=OpXw@mail.gmail.com>
<[email protected]>
On Wed, Aug 12, 2020 at 3:45 AM Chapman Flack <[email protected]> wrote:
> On 08/11/20 14:49, Kartik Ohri wrote:
> > I have currently added this
> >
> https://github.com/amCap1712/pljava/commit/174dd71365986094a500d6dda4c832de78f90783
> .
> > Is this fine? I had to change the way error handling worked to avoid
> > creating a new functional interface.
>
> That looks like some of the right ideas shaping up.
>
> You definitely do NOT want "arguments = String.join(" ", argumentsList)".
> That's exactly how to create a mess that cannot be correctly reparsed
> into the original arguments, depending on what characters they contain.
>
> The proper model to think in always a list of individual arguments. That
> is the model the Java ProcessBuilder uses, and it is also the true native
> model in POSIX (and so in Linux and Mac OS).
>
> The native model in Windows is a single joined command line that the
> receiving program then has to reparse to get the original individual
> arguments correctly back, and the Java runtime itself will create the
> joined string for you (you still pass individual arguments to
> ProcessBuilder), but it needs help to do that correctly; I hope you are
> reviewing the code in Node.java that I mentioned earlier. The point is to
> do this transparently and correctly, so you start with a list of arguments,
> and the invoked program ultimately has a list of arguments, and that's
> the same as the starting list.
>
>
Yes, I agree with you. I used String.join because the ProcessBuilder
documentation said that "here are operating systems where programs are
expected to tokenize command line strings themselves - on such a system a
Java implementation might require commands to contain exactly two elements".
I was not sure if this was relevant but tried to incorporate it.
Regarding the subprocess stdout and stderr, as a way of doing things in
> stages, I would suggest starting as simply as possible: just set them
> both to INHERIT, so they go straight to our own stdout and stderr, and
> we don't have to do anything with them, and can concentrate on getting
> the build working.
>
> As a later step, we could revisit that to capture output and funnel it
> into Maven's logging, probably applying some filtering first, to decide
> what should be logged as info, warn, or error (and ideally do a better
> job of that than nar-maven-plugin does).
>
> Capturing all of the output in a readAllBytes() is not the usual way
> to do that, because it's preferable to see the messages as they happen
> rather than all at the end (and if a process happens to generate a whole
> lot of output, readAllBytes uses a lot of memory).
>
>
Right, this is certainly desirable. I'll look into this and fix this.
> In that way, that makes this task different from getPgConfigProperty,
> where the point really was to just read all of the output from pg_config
> and return it. I see now that you have refactored that method in terms
> of this one, but that may just complicate things. It is ok to have a
> couple of different methods using ProcessBuilder but in different ways,
> as "run pg_config and return its output" and "run this arbitrary compiler
> or linker with its output going to the log" are sufficiently different
> tasks.
>
> Yes, makes sense. I'll revert this change.
Regards,
> -Chap
>
Thanks.
Regards,
Kartik
view thread (183+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: PL/Java new build plugin
In-Reply-To: <CAASLQ4O3ONhDr-XgwDDD3oW+x2VwCDTmuu-T+fz-jcZ=T23zuw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox