Bug 247739 - [NEW PORT]: devel/lua-cqueues
Summary: [NEW PORT]: devel/lua-cqueues
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Kurt Jaeger
URL: http://25thandclement.com/~william/pr...
Keywords:
Depends on:
Blocks: 247699
  Show dependency treegraph
 
Reported: 2020-07-03 12:51 UTC by Leo Vandewoestijne
Modified: 2020-07-11 20:51 UTC (History)
2 users (show)

See Also:


Attachments
lua-cqueues (4.16 KB, text/plain)
2020-07-03 12:51 UTC, Leo Vandewoestijne
no flags Details
lua-cqueues (2.58 KB, patch)
2020-07-04 18:19 UTC, Leo Vandewoestijne
no flags Details | Diff
fix cqueues port (2.68 KB, patch)
2020-07-04 21:14 UTC, andrew
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Leo Vandewoestijne 2020-07-03 12:51:54 UTC
Created attachment 216171 [details]
lua-cqueues

When running dns/knot-resolver I encountered missing parts.
This port is meant to solve the missing cqueques in Lua.

It works for me, and does the job,
but I think there is room for improvement when using -soname from _queues.so to liblua-cqueues.so.{LUA_VER}
However I was unable to acomplish that (solve the puzzle with MAKE arg's).
Comment 1 Leo Vandewoestijne 2020-07-03 13:14:29 UTC
In reality I'm able to use it... but these are still bad:

====> Running Q/A tests (stage-qa)
Warning: /wrkdirs/usr/ports/devel/lua-cqueues/work-lua52/stage/usr/local/lib/liblua-cqueues.so.52 doesn't have a SONAME.
Warning: pkg(8) will not register it as being provided by the port.
Warning: If another port depend on it, pkg will not be able to know where it comes from.
Warning: It is directly in /usr/local/lib, it is probably used by other ports.
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: @dir %%LUA_MODLIBDIR%%
Comment 2 Kurt Jaeger freebsd_committer 2020-07-03 18:08:14 UTC
testbuilds are ok
Comment 3 Kurt Jaeger freebsd_committer 2020-07-03 19:40:13 UTC
(In reply to Kurt Jaeger from comment #2)
Adding

LDFLAGS+=       -Wl,-soname,liblua-${PORTNAME}.so.${LUA_VER_STR}

helps for the SONAME warning.
Comment 4 Kurt Jaeger freebsd_committer 2020-07-03 19:46:17 UTC
Committed, thanks!
Comment 5 commit-hook freebsd_committer 2020-07-03 19:46:35 UTC
A commit references this bug:

Author: pi
Date: Fri Jul  3 19:46:04 UTC 2020
New revision: 541131
URL: https://svnweb.freebsd.org/changeset/ports/541131

Log:
  New port: devel/lua-cqueues

  cqueues is a type of event loop for Lua, except it's not a classic
  event loop. It doesn't use callbacks, neither as part of the API nor
  internally, but instead you communicate with an event controller by
  the yielding and resumption of Lua coroutines using objects that
  adhere to a simple interface.

  WWW: http://25thandclement.com/~william/projects/cqueues.html

  PR:		247739
  Submitted by:	Leo Vandewoestijne <freebsd@dns.company>

Changes:
  head/devel/Makefile
  head/devel/lua-cqueues/
  head/devel/lua-cqueues/Makefile
  head/devel/lua-cqueues/distinfo
  head/devel/lua-cqueues/pkg-descr
  head/devel/lua-cqueues/pkg-plist
Comment 6 andrew 2020-07-03 20:01:23 UTC
No, this is broken; a flavored lua port must not install files that aren't specific to its flavor, unless it also declares the conflict. You've made a port that can't be installed under multiple lua versions at once; this needs to be fixed.
Comment 7 Kurt Jaeger freebsd_committer 2020-07-03 20:17:44 UTC
Which file is not lua-flavor-compliant ?

Is it

lib/liblua-cqueues.so

?
Comment 8 andrew 2020-07-03 20:41:51 UTC
Yup.

Also, has anyone actually _tested_ that this port works _as a lua module_, i.e. that requiring it from the lua REPL works? The port isn't installing _cqueues.so to the expected place.

Cqueues is too important a lua module to allow there to be a broken port for it - right now it works fine if you install it from luarocks, even for multiple versions at once. A port install of it needs to work just as well.

What is the purpose of installing a "liblua-cqueues.so" file at all, even with the version number attached? If some other port is trying to use that, then that should probably be fixed instead. (And even if done this way, it should probably be e.g. liblua53-cqueues.so, not liblua-cqueues.so.5.3 since the version number is that of _Lua_, not that of cqueues.)
Comment 9 andrew 2020-07-03 20:43:21 UTC
Oh, I suggest putting this up on Phabricator for review, feel free to add me under my phab username of  andrew_tao173.riddles.org.uk
Comment 10 Leo Vandewoestijne 2020-07-03 23:21:20 UTC
> What is the purpose of installing a "liblua-cqueues.so" file at all, even with the version number attached?
>
When I added it to LIB_DEPENDS it complained the filename should start with lib*
When I googl'ed it I saw similar complaints of users at other OS'es.


> right now it works fine if you install it from luarocks
>
Later on I also would need lua-http and that one already starts with a number other missing lua-modules as dependency. So, yes, sound like a plan.
Still I'm stuck with this:

/!\ knot-resolver-5.1.2: Makefile errors /!\

All LIB_DEPENDS should use the new format and start out with lib. (libfoo.so
vs foo.so)
Comment 11 andrew 2020-07-04 01:16:46 UTC
(In reply to Leo Vandewoestijne from comment #10)

OK, so there's a whole bunch of nasty issues here, some of them involving luajit.

knot-resolver is not using stock Lua but rather the Openresty fork of LuaJIT, which is not supported by the Lua ports framework at this time (I did look into adding it when I did the flavors work, but there are too many luajit forks in play right now, all of which conflict).

I _think_ we can get away with it in this case; cqueues built for Lua 5.1 should run on LuaJIT, even with the limitation to 47-bit lightuserdata values.

I've confirmed that this port as it stands is not functional as a lua module: doing require 'cqueues' fails because of the missing _cqueues.so.

The question is: if this port were fixed to simply install cqueues correctly (without trying to mess with the library name or location), then would it suffice for knot-resolver to simply add

lua51-cqueues>=2020xxxx:devel/lua-cqueues@lua51

to both BUILD_DEPENDS and RUN_DEPENDS ? This should work as long as knot-resolver isn't trying to link directly to the .so (which it should not be doing). Hardcoding lua51 here is correct rather than using LUA_FLAVOR because luajit-openresty only supports the 5.1 language and ABI.

(For knot-resolver itself, you should be extremely wary of both having a dependency on luajit (any version) and also having USES = lua allowing any version other than exactly 5.1, for the reasons just given.)
Comment 12 Leo Vandewoestijne 2020-07-04 18:19:46 UTC
Created attachment 216200 [details]
lua-cqueues

(In reply to andrew from comment #11)
The history of it is that dns/dnsdist switched to luajit-openresty for speed.
I did like the speed argument, and prefer to have a least as software as possible, and made the same switch in dns/knot-resolver.
But indeed, looking at it, I was already pessimistic if they would keep up with newer versions.

I tried the 
lua51-cqueues>=2020xxxx:devel/lua-cqueues@lua51
but still give me the fatal 'lib*.so' naming error.

What do you think about this solution:

Simplify devel/lua-cqueues and add only one copy and one symlink (see patch), and in dns/knot-resolver then have LIB_DEPENDS / RUN_DEPENDS like:

RUN_DEPENDS=    liblua${LUA_VER_STR}-cqueues.so>=2020xxxx:devel/lua-cqueues@{LUA_FLAVOR}

That seems to work for me (also when using lua52).

Still -later on- I would need lua-http which needs luaossl.

I also tried your suggestion to use luarocks, with success, but don't see how I could put that in a port (in particular at deinstall).
Comment 13 andrew 2020-07-04 19:06:09 UTC
(In reply to Leo Vandewoestijne from comment #12)

The error you got is nonsensical - can you please post somewhere the exact patch to knot-resolver that you were testing, so I can try and reproduce it.

There are other issues with your updated version which I will address shortly.
Comment 14 andrew 2020-07-04 21:14:50 UTC
Created attachment 216205 [details]
fix cqueues port
Comment 15 andrew 2020-07-04 21:18:44 UTC
So the attachment I just posted is how I think this should go. Notes:

1. PORTDOCS / PORTEXAMPLES obviate the need to put docs and examples in the plist.

2. There should be no messing with ldconfig and soname, since the .so exists only to be dynamically loaded at runtime, not actually linked to.

3. you had the name in the COMMENT wrong

4. Doesn't build on lua 5.4, so I adjusted that

5. Relatedly, you should always explicitly force the port to build exactly the version it's supposed to build, rather than let it autodetect things. Hence MAKE_ENV and CPPFLAGS

6. shebangfix on the examples is not strictly necessary but is cleaner.
Comment 16 Kurt Jaeger freebsd_committer 2020-07-04 21:21:20 UTC
testbuilds are fine. Leo, can you comment ?
Comment 17 andrew 2020-07-04 22:38:38 UTC
I posted a matching patch for knot-resolver over on that ticket.
Comment 18 Leo Vandewoestijne 2020-07-08 14:22:29 UTC
Did some testing in host, jail and poudriere: looks all fine;
Approved.
Thanks everybody.
Comment 19 commit-hook freebsd_committer 2020-07-11 20:50:49 UTC
A commit references this bug:

Author: pi
Date: Sat Jul 11 20:50:37 UTC 2020
New revision: 542053
URL: https://svnweb.freebsd.org/changeset/ports/542053

Log:
  devel/lua-cqueues: adapt to the needs of dns/knot-resolver

  - PORTDOCS / PORTEXAMPLES obviate the need to put docs and examples in
    the plist.
  - There should be no messing with ldconfig and soname, since the
    .so exists only to be dynamically loaded at runtime, not actually
    linked to.
  - Fixed name in the COMMENT
  - Does not build on lua 5.4, note this in USES
  - Force the port to build exactly the version it's supposed to build,
    rather than let it autodetect things. Hence MAKE_ENV and CPPFLAGS
  - shebangfix on the examples is not strictly necessary but is cleaner.

  PR:		247739
  Submitted by:	andrew@tao11.riddles.org.uk
  Approved by:	Leo Vandewoestijne <freebsd@dns.company> (maintainer)

Changes:
  head/devel/lua-cqueues/Makefile
  head/devel/lua-cqueues/pkg-plist
Comment 20 Kurt Jaeger freebsd_committer 2020-07-11 20:51:06 UTC
Fix committed, thanks!