Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1k2vjL-0004r1-IR for pljava-dev@arkaria.postgresql.org; Tue, 04 Aug 2020 12:07:43 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1k2vjK-0002jb-Ir for pljava-dev@arkaria.postgresql.org; Tue, 04 Aug 2020 12:07:42 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1k2vjK-0002jT-AU for pljava-dev@lists.postgresql.org; Tue, 04 Aug 2020 12:07:42 +0000 Received: from mail-ej1-x631.google.com ([2a00:1450:4864:20::631]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1k2vjC-00079p-RV for pljava-dev@lists.postgresql.org; Tue, 04 Aug 2020 12:07:41 +0000 Received: by mail-ej1-x631.google.com with SMTP id d6so28286313ejr.5 for ; Tue, 04 Aug 2020 05:07:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=A/1WGplxu53gYf9z1MDGJii5vbBuTmrpu2aa8OVbeas=; b=idC+PneWfFOAgUqfboGQRFaGVcmVglcSCt058x2HuQzatByuTJcrbmqirKSGoLqUxc AEe3KTuIav4DucuOjCPdAG2uJkgwBHibirH9YRBaJuvtbr3fqHbFNUisOJmKjMvkxJpD EJb2Y3Uy470xFl7uhdA/cr6uAbXrj29NXSFDP6pibQ6JCd+oOz8xQw94LajXAuroU7Li iiad4V6cHkkGJmsw9dWMlb0fzEBaBKx3K5UA44r4f9pz/6jakOAscas6u1hBt8+B1+Yt Gms8n9yySNds68vYG40iawQBCC+cVQAbk3Q1dBz8Ep8Z/JgxXFeG5j8Uf9PaIaztGTsk 3pxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=A/1WGplxu53gYf9z1MDGJii5vbBuTmrpu2aa8OVbeas=; b=MHz8klf486MUFNLlZHjo3wb9zBm6JocxT76pvPU6qCTTaAI4ItGDiGhefeJ/RGX1qg rQ8EBT2CjgNGuuWWn9OtMH+JRQ6oE982iJXFfeRBsBE7qRbKhbXeByqxZvTbW9WEpTuO kfTL8Yksq/5Qs85y9A92f3JwxDIIrsMVphiiaBuF8KfimYaCRCf2BThGdHbUqXWME1Rk a8E816o7pxYbgwBhUY0jjjT2Hhg/VZIeRV6rrRGrSa/tTlZPXLVJXp+sez6xHKXevwoU onXqCShZ9D0O0ZMw+Iwc2ovpbnYxyG/stuR9k6MEFnXfxjMk5qGE+zUAIRCbGJLLQV8m ia5g== X-Gm-Message-State: AOAM5316zNx6LN0qRRUQbx0C5TvK/wEd6+Mr7sajQ9aKQIida+RmJxSO mdWnmr5aJPZ8tDxSbpWRFMC8LU0cvPcfS1DHBLxb9JpK X-Google-Smtp-Source: ABdhPJzJO2gsi1TGD7wty9IWfyixqS4DyvGlRMxm8VVu4oHslH/3168XnZjlpnN3n2Ncz7r3mSgkfhBSSZFDQ5qaNtU= X-Received: by 2002:a17:907:42a0:: with SMTP id ny24mr21581042ejb.328.1596542853680; Tue, 04 Aug 2020 05:07:33 -0700 (PDT) MIME-Version: 1.0 References: <5F1F88E0.2050305@anastigmatix.net> <5F20E585.6010703@anastigmatix.net> <5F22DF79.2090701@anastigmatix.net> <5F230337.3040406@anastigmatix.net> <5F232EBB.9030208@anastigmatix.net> <5F278CCE.1090006@anastigmatix.net> <5F2897AF.9040909@anastigmatix.net> In-Reply-To: <5F2897AF.9040909@anastigmatix.net> From: Kartik Ohri Date: Tue, 4 Aug 2020 17:37:22 +0530 Message-ID: Subject: Re: PL/Java new build plugin To: Chapman Flack Cc: pljava-dev@lists.postgresql.org Content-Type: multipart/alternative; boundary="000000000000a9917d05ac0c1912" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000a9917d05ac0c1912 Content-Type: text/plain; charset="UTF-8" On Tue, Aug 4, 2020 at 4:33 AM Chapman Flack wrote: > On 08/03/20 15:25, Kartik Ohri wrote: > > I have fixed this. I checked maven javadoc code but there was nothing > much > > obvious from it. I then compared the project-reports.html file for the > > plugins and found what the issue was. Generating the file using sink was > > actually wrong and wouldn't have worked anyways. (Not in the current > setup > > at least.) Further details are in the commit message. > > Ah, so that's what the isExternalReport() method is good for! > > So this looks good, only now the output name and isExternal bit are > hardcoded in Java. This is probably a good time to look again at > defining a small Java interface similar to MavenReport (at least with > methods resembling canGenerateReport, getCategoryName, getDescription, > getName, getOutputName, and isExternalReport, plus the AbstractMavenReport- > specified executeReport method), of which the script engine can create an > implementation. > > I remember you were concerned about the order in which Maven might > call those. I think it might be workable if done like this: the mojo > overrides each of those methods, with a wrapper that first checks whether > the script interface implementation has been created yet; if not, evaluate > the script block and create it. Then call the corresponding method on the > script interface object, and return the result. > > The only way I can think of that not working would be if some of those > methods can be called before Maven has interpolated the ${property} values > into the script block. Otherwise, it sounds workable to me. > I did some digging around how maven generates reports. I found that the maven plugin manager delegates the task to invoke various report plugins to maven-site-plugin. Looking through its source, I found that it calls the executeReport method before anything else. The codebase was a bit large so I may have missed a few subtle details but I tested out my implementation as explained below and it worked. > I think the methods on the script interface may need to be declared with > the 'report' mojo instance as a parameter. That way, the script interface > could have, for example, > > default boolean isExternalReport(ReportScriptingMojo report) > { > return report.isExternalReport(); // ok not quite that simple ;) > } > > so the behavior is only changed if the script block defines an overriding > method. > > It has to be a little more complicated to avoid infinite recursion, > because report.isExternalReport() would just call the interface method > again. Maybe each of those methods should have a corresponding, e.g., > > boolean isExternalReportDefault() { return super.isExternalReport(); } > > which would be what the default method on the script interface calls. > I tried to comprehend how this would work but was not able to comprehend how it would work. I did try to go this route but I felt that it was getting a bit difficult. So, after some thinking I moved the hard coded values to the default values of the interface and utilised it in the mojo. This approach worked well and I have pushed the changes. I have also pushed a working example to test it. Please review it and let me know if changes are required. > ---------------- > > I have been checking out nar maven plugin's codebase to see how to invoke > > the C compiler and so on. It seems that the plugin delegates at least > some > > parts to ant. It creates a custom task and hooks its adapter for > different > > os, architecture and linker configurations into it. Do we want to use ant > > for that part or follow a direct approach ? > > I lean toward a direct approach; I think part of the appeal of ant may > have been that it was born when Java was very young, and handled things > that were harder to do in straight Java then (before ProcessBuilder, > before java.nio.file, etc.), that now might be simple enough to do > directly. > > But that wouldn't make it a bad idea to write up a brief page or so of > what particular operations you see it delegating to ant, and which of those > look like they might be nontrivial enough to keep. > I'll try to figure this out. > ---------------- > When you have a moment for doing something different, could you review > the methods forWindowsCRuntime and asPgCtlInvocation found here: > > > https://github.com/jcflack/pljava/blob/2fc8513/pljava-packaging/src/main/java/Node.java#L1646 > > The first one especially, forWindowsCRuntime, is a part of what I had > originally proposed for your plugin, but I turned out to also need this > simplified subset of it here. So, it should provide some code that you > will be able to reuse in the plugin. (I am not concerned about avoiding > code duplication; the Node.java where I am using it here is intended to > work standalone, just inserted in the installer jar, so it's kind of > special. The plugin can just end up with a similar method included > in PGXSUtils, and perhaps it will need to implement a few more of the > edge cases I was able to reject.) > > Getting these transformations just right is kind of critical to avoid > nutty failures dependent on values of passed arguments. I was looking > at that David Deley document on Windows quoting rules, and the Java > ProcessImpl sources, so long I was going crosseyed. Despite writing > detailed comments for each step and referring back to lines in Java's > source, I still ended up fixing some details in debugging (a344af6). > > So it would be very helpful if you could look those over in some detail > and let me know of parts that do or don't convince you. > Yeah, sure. I'll take a look at it soon. Regards, Kartik --000000000000a9917d05ac0c1912 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Tue, Aug 4, 2020 at 4:33 AM Chapman Fl= ack <chap@anastigmatix.net&= gt; wrote:
On 08/03/20 15:25, Kartik Ohri wrote:
> I have fixed this. I checked maven javadoc code but there was nothing = much
> obvious from it. I then compared the project-reports.html file for the=
> plugins and found what the issue was. Generating the file using sink w= as
> actually wrong and wouldn't have worked anyways. (Not in the curre= nt setup
> at least.) Further details are in the commit message.

Ah, so that's what the isExternalReport() method is good for!

So this looks good, only now the output name and isExternal bit are
hardcoded in Java. This is probably a good time to look again at
defining a small Java interface similar to MavenReport (at least with
methods resembling canGenerateReport, getCategoryName, getDescription,
getName, getOutputName, and isExternalReport, plus the AbstractMavenReport-=
specified executeReport method), of which the script engine can create an implementation.

I remember you were concerned about the order in which Maven might
call those. I think it might be workable if done like this: the mojo
overrides each of those methods, with a wrapper that first checks whether the script interface implementation has been created yet; if not, evaluate<= br> the script block and create it. Then call the corresponding method on the script interface object, and return the result.

The only way I can think of that not working would be if some of those
methods can be called before Maven has interpolated the ${property} values<= br> into the script block. Otherwise, it sounds workable to me.

I did some digging around how maven generates reports.= I found that the maven plugin manager delegates the task to invoke various= report plugins to maven-site-plugin. Looking through its source, I found t= hat it calls the executeReport method before anything else. The codebase wa= s a bit large so I may have missed a few subtle details but I tested out my= implementation as explained below and it worked.
=C2=A0
I think the methods on the script interface may need to be declared with the 'report' mojo instance as a parameter. That way, the script int= erface
could have, for example,

default boolean isExternalReport(ReportScriptingMojo report)
{
=C2=A0 return report.isExternalReport(); // ok not quite that simple ;)
}

so the behavior is only changed if the script block defines an overriding method.

It has to be a little more complicated to avoid infinite recursion,
because report.isExternalReport() would just call the interface method
again. Maybe each of those methods should have a corresponding, e.g.,

boolean isExternalReportDefault() { return super.isExternalReport(); }

which would be what the default method on the script interface calls.

I tried to comprehend how this would work bu= t was not able to comprehend how it would work. I did try to go this route = but I felt that it was getting a bit difficult. So, after some thinking I m= oved the hard coded values to the default values of the interface and utili= sed it in the mojo. This approach worked well and I have pushed the changes= . I have also pushed a working example to test it. Please review it and let= me know if changes are required.

> ----------------
> I have been checking out nar maven plugin's codebase to see how to= invoke
> the C compiler and so on. It seems that the plugin delegates at least = some
> parts to ant. It creates a custom task and hooks its adapter for diffe= rent
> os, architecture and linker configurations into it. Do we want to use = ant
> for that part or follow a direct approach ?

I lean toward a direct approach; I think part of the appeal of ant may
have been that it was born when Java was very young, and handled things
that were harder to do in straight Java then (before ProcessBuilder,
before java.nio.file, etc.), that now might be simple enough to do directly= .

But that wouldn't make it a bad idea to write up a brief page or so of<= br> what particular operations you see it delegating to ant, and which of those=
look like they might be nontrivial enough to keep.
I'll try to figure this out.
=C2=A0
----------------
When you have a moment for doing something different, could you review
the methods forWindowsCRuntime and asPgCtlInvocation found here:

https:/= /github.com/jcflack/pljava/blob/2fc8513/pljava-packaging/src/main/java/Node= .java#L1646

The first one especially, forWindowsCRuntime, is a part of what I had
originally proposed for your plugin, but I turned out to also need this
simplified subset of it here. So, it should provide some code that you
will be able to reuse in the plugin. (I am not concerned about avoiding
code duplication; the Node.java where I am using it here is intended to
work standalone, just inserted in the installer jar, so it's kind of special. The plugin can just end up with a similar method included
in PGXSUtils, and perhaps it will need to implement a few more of the
edge cases I was able to reject.)

Getting these transformations just right is kind of critical to avoid
nutty failures dependent on values of passed arguments. I was looking
at that David Deley document on Windows quoting rules, and the Java
ProcessImpl sources, so long I was going crosseyed. Despite writing
detailed comments for each step and referring back to lines in Java's source, I still ended up fixing some details in debugging (a344af6).

So it would be very helpful if you could look those over in some detail
and let me know of parts that do or don't convince you.

Yeah, sure. I'll take a look at it soon.=C2=A0

Regards,
Kartik
--000000000000a9917d05ac0c1912--