Re: [NEW] devel/libnjs

Hi Stuart,

thanks very much for your comments.

On Mon, May 22, 2023 at 07:11:19PM +0100, Stuart Henderson wrote:
> On 2023/05/22 17:28, Sergey A. Osokin wrote:
> > On Sat, May 20, 2023 at 06:03:26PM +0200, Landry Breuil wrote:
> > > Le Thu, May 18, 2023 at 09:31:35PM +0000, Sergey A. Osokin a écrit :
> > > > Hi there,
> > > >
> > > > here's the port of nginx javascript static library for OpenBSD
> > > > ports tree.
> > > > Please let me know your thoughts, ask questions, provide comments.
> > >
> > > you should be able to use the various GH_PROJECT/GH_ACCOUNT/GH_TAGNAME
> > > variables instead of handrolling DISTFILES/MASTER_SITES/PKGNAME/WRKDIST.
> >
> > That's exactly what I tried to use as a first step, however that didn't
> > work for me, so I decided to use this way.
>
> Please see attached Makefile (and run make makesum to update distinfo).
> The way your Makefile has it, the distfile is just named 0.7.12.tar.gz
> which is not specific enough.

Thanks, I figured out how to do that.

> > > is only the static lib necessary in libnjs ? for both ports, why the
> > > auto/make patch, afaict, our sed supports s,X,Y, constructs..
> >
> > The static library is necessary, and probably a shared library will
> > be available for the future releases on nginx javascript.
> >
> > About the auto/make patch - the issue has been reported to the upstream,
> > the upstream's updated,
> > https://github.com/nginx/njs/commit/8a03334e27393fc2031f071830f9605f4373b0be
> > So, I'll remove that patch with the next release of nginx javascript.
>
> The patches don't match standard naming conventions, we normally use
> "make update-patches" to generate them. To fix them up, you can "make
> patch" with the existing patches, then mv the patches dir out the way,
> and "make update-patches". (For newly patched files, copy the source
> file to filename.orig.port, modify as needed, and make update-patches
> to pick them up).
>
> PLIST should also be autogenerated (and maybe tweaked afterwards if
> necessary, but that's the starting point). Run "make plist". Here
> the sort order changes, and lib/libnjs.a gets an "@static-lib"
> annotation.

Thanks.

> > > i dont think do-build is necessary, setting ALL_TARGET=libnjs njs should
> > > achieve the same.
> >
> > Good catch, I'm going to re-work this part and let you know how
> > that works, thanks.
> >
> > > why two ports, instead of a single, or eventually a MULTI_PACKAGE ? i
> > > suppose your hidden untold goal is to enable support in nginx, if so why
> > > not saying it upfront ?
> >
> > That's good idea. Since those two (devel/libnjs and lang/njs) from
> > the different categories, not sure what's the best place to create
> > a port for both (similarly to www/unit, I believe).
>
> My preference would be a single port in lang/njs providing library and cli.

I forgot to mention another argument to split the njs library and njs
command line utility into the two ports. In case of specifing multiple
targets, i.e.:

ALL_TARGET= libnjs njs

it's possible to get only first thing, i.e. libnjs in this case, but not both.
In other words, every "final" target has its own configure target, and
so on. So, I believe it's better to keep them splitted.

> They're in the same distfile, I don't think it makes all that much sense
> to split the package into njs and libnjs really (we don't normally do
> that for other software in ports unless there are heavy extra dependencies
> in one or the other).

There's another target available for the distro - nginx javascript module
for nginx. I'll submit a patch for that as soon as I can.

> > Also, I'm going to enable nginx javascript support for www/unit, so
> > I'd like to create that the same way as I did that for FreeBSD and
> > NetBSD.
>
> sounds good to me.

Thanks. And here's the updated version of devel/libnjs port.
I'm going to update lang/njs port as well and submit the updated one
in a separated mail thread.

--
Sergey A. Osokin