Bug 281215 - www/elinks: fails to build with CSS option on
Summary: www/elinks: fails to build with CSS option on
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: Robert Clausecker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-09-03 02:14 UTC by John Hein
Modified: 2024-09-16 20:25 UTC (History)
3 users (show)

See Also:
jailbird: maintainer-feedback+
fuz: merge-quarterly+


Attachments
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (1.06 KB, patch)
2024-09-03 13:11 UTC, John Hein
no flags Details | Diff
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (v2) (1.08 KB, patch)
2024-09-03 13:30 UTC, John Hein
jailbird: maintainer-approval+
Details | Diff
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (v3) (2.36 KB, patch)
2024-09-15 19:56 UTC, John Hein
jailbird: maintainer-approval-
Details | Diff
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (v4) (2.46 KB, patch)
2024-09-16 18:06 UTC, John Hein
jailbird: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Hein 2024-09-03 02:14:47 UTC
www/elinks is currently 0.17.0, and if the CSS option is on, it fails to build today [[1]].

I don't know if the specific versions of dependent libraries matter or not, but libcss is currently at 0.9.2 and libdom is 0.4.2.  I didn't try to roll back to older versions when the CSS option was introduced, but if it used to build back then (Jan 2024 when elinks was updated to 0.17.0), it doesn't seem to now.

Here's the config I used...

% make showconfig
===> The following configuration options are available for elinks-0.17.0_1:
     256COLORS=on: 256 color support
     88COLORS=on: 88 color support
     BITTORRENT=on: BitTorrent file sharing support
     BROTLI=on: Brotli compression support
     CSS=on: Cascading Style Sheets support (via libcss & libdom)
     CURL=on: Data transfer support via cURL
     EXMODE=on: Exmode (CLI) support
     FASTMEM=on: Fast memory allocation functions
     FINGER=off: finger(1) user information support
     FSP=off: FSP protocol support (via fsplib)
     FTP=on: FTP protocol support
     GOPHER=off: Gopher protocol support
     GUILE=off: Guile extension language support
     HIGHLIGHT=on: HTML highlighting using DOM engine
     IDN=off: International Domain Names support
     LOCAL_CGI=off: Local CGI support
     LUA=on: Lua scripting language support
     LZMA=on: LZMA compression support
     MOUSE=on: Mouse support
     NLS=on: Native Language Support
     NNTP=off: NNTP (News) support
     NOROOT=off: Prevention of usage by root
     SMB=on: SMB network protocol support
     SPIDERMONKEY=on: ECMAScript support (via SpiderMonkey)
     TRE=on: TRE regex search support
     TRUECOLORS=off: True color support
     XBELMARKS=off: XBEL bookmarks (via expat)
     ZSTD=off: Zstandard compression support


[[1]]

FAILED: src/elinks.p/document_libdom_css.c.o
cc -Isrc/elinks.p -Isrc -I../src -I. -I.. -I/usr/local/include -I/usr/local/include/nspr -I/usr/local/include/lua53 -I/usr/local/lib/perl5/5.36/mach/CORE -I/usr/local/include/samba4 -I/usr/local/include/js-102 -fdiagnostics-color=never -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch '-DGETTEXT_PACKAGE="elinks"' '-DBUILD_ID=""' -O2 -pipe -fstack-protector-strong -fno-strict-aliasing -DLIBICONV_PLUG -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -DUSE_THREAD_SAFE_LOCALE -fno-strict-aliasing -fstack-protector-strong -isystem /usr/local/include/mozjs-102 -isystem /usr/local/include -DHAVE_CONFIG_H -fno-strict-aliasing -Wno-address -MD -MQ src/elinks.p/document_libdom_css.c.o -MF src/elinks.p/document_libdom_css.c.o.d -o src/elinks.p/document_libdom_css.c.o -c ../src/document/libdom/css.c
../src/document/libdom/css.c:225:2: error: incompatible function pointer types initializing 'css_error (*)(void *, void *, void *)' (aka 'enum css_error (*)(void *, void *, void *)') with an expression of type 'css_error (void *, const css_hint *, css_hint *)' (aka 'enum css_error (void *, const struct css_hint *, struct css_hint *)') [-Wincompatible-function-pointer-types]
  225 |         compute_font_size,
      |         ^~~~~~~~~~~~~~~~~
../src/document/libdom/css.c:226:2: error: incompatible function pointer types initializing 'css_error (*)(void *, void *, void **)' (aka 'enum css_error (*)(void *, void *, void **)') with an expression of type 'css_error (void *, void *, void *)' (aka 'enum css_error (void *, void *, void *)') [-Wincompatible-function-pointer-types]
  226 |         set_libcss_node_data,
      |         ^~~~~~~~~~~~~~~~~~~~
../src/document/libdom/css.c:227:2: warning: excess elements in struct initializer [-Wexcess-initializers]
  227 |         get_libcss_node_data
      |         ^~~~~~~~~~~~~~~~~~~~
../src/document/libdom/css.c:299:36: error: too few arguments to function call, expected 8, have 7
  298 |         error = css_select_style(ctx->ctx, n, media, inline_style,
      |                 ~~~~~~~~~~~~~~~~
  299 |                         &selection_handler, ctx, &styles);
      |                                                         ^
/usr/local/include/libcss/select.h:221:11: note: 'css_select_style' declared here
  221 | css_error css_select_style(css_select_ctx *ctx, void *node,
      |           ^                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  222 |                 const css_unit_ctx *unit_ctx,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  223 |                 const css_media *media, const css_stylesheet *inline_style,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  224 |                 css_select_handler *handler, void *pw,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  225 |                 css_select_results **result);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/document/libdom/css.c:314:5: error: too many arguments to function call, expected 4, have 5
  311 |                 error = css_computed_style_compose(ctx->parent_style,
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~
  312 |                                 styles->styles[CSS_PSEUDO_ELEMENT_NONE],
  313 |                                 compute_font_size, ctx,
  314 |                                 &composed);
      |                                 ^~~~~~~~~
/usr/local/include/libcss/computed.h:82:11: note: 'css_computed_style_compose' declared here
   82 | css_error css_computed_style_compose(
      |           ^
   83 |                 const css_computed_style *restrict parent,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   84 |                 const css_computed_style *restrict child,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   85 |                 const css_unit_ctx *unit_ctx,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   86 |                 css_computed_style **restrict result);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/document/libdom/css.c:346:5: error: too many arguments to function call, expected 4, have 5
  342 |                 error = css_computed_style_compose(
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~
  343 |                                 styles->styles[CSS_PSEUDO_ELEMENT_NONE],
  344 |                                 styles->styles[pseudo_element],
  345 |                                 compute_font_size, ctx,
  346 |                                 &composed);
      |                                 ^~~~~~~~~
/usr/local/include/libcss/computed.h:82:11: note: 'css_computed_style_compose' declared here
   82 | css_error css_computed_style_compose(
      |           ^
   83 |                 const css_computed_style *restrict parent,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   84 |                 const css_computed_style *restrict child,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   85 |                 const css_unit_ctx *unit_ctx,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   86 |                 css_computed_style **restrict result);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/document/libdom/css.c:384:28: error: too many arguments to function call, expected 4, have 5
  383 |         error = css_computed_style_compose(parent, partial,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~
  384 |                         compute_font_size, ctx, &composed);
      |                                                 ^~~~~~~~~
/usr/local/include/libcss/computed.h:82:11: note: 'css_computed_style_compose' declared here
   82 | css_error css_computed_style_compose(
      |           ^
   83 |                 const css_computed_style *restrict parent,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   84 |                 const css_computed_style *restrict child,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   85 |                 const css_unit_ctx *unit_ctx,
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   86 |                 css_computed_style **restrict result);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning and 6 errors generated.
ninja: build stopped: subcommand failed.
Comment 1 John Hein 2024-09-03 13:11:46 UTC
Created attachment 253296 [details]
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0

Until a new upstream elinks version is released (0.17.1?), pull in an upstream patch that supports the current libcss (0.9.2) and libdom (0.4.2) that were pulled in to FreeBSD ports in Feb 2024.

No need to bump PORTREVISION. No one could build elinks with the CSS option successfully without this, so no such package can exist that will need to be updated.
Comment 2 John Hein 2024-09-03 13:30:48 UTC
Created attachment 253297 [details]
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (v2)
Comment 3 John Hein 2024-09-03 13:32:17 UTC
Comment on attachment 253297 [details]
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (v2)

previous patch (now obsoleted) was missing ".patch" for the patch file specification.
Comment 4 Dustin Marquess 2024-09-04 01:25:27 UTC
Looks good to me! Thanks for the patch!
Comment 5 Robert Clausecker freebsd_committer freebsd_triage 2024-09-12 10:00:22 UTC
Would you like for this patch to be merged into the quarterly branch?

I'll bump PORTREVISION on commit.
Comment 6 Robert Clausecker freebsd_committer freebsd_triage 2024-09-12 10:06:27 UTC
A test build yields the following dependency error:

====> Running Q/A tests (stage-qa)
Error: /usr/local/bin/elinks is linked to /usr/local/lib/libwapcaplet.so.0 from textproc/libwapcaplet but it is not declared as a dependency

Please check what causes the dependency and add appropriate LIB_DEPENDS.
Comment 7 John Hein 2024-09-15 19:27:24 UTC
(In reply to Robert Clausecker from comment #6)
libwapcaplet is an indirect dependency for elinks.  It is a direct dependency of libcss.  So stage-qa is wrong in declaring this an error, IMO - it could be smarter and determine when a shared lib is a direct dependency (although that can be hard to do simply in an automated way).

The reason for the check in stage-qa is to try to avoid a situation where a user could delete a package (A) that is needed at run time by another package (B).  If there is no dependency recorded, the user may not know that deleting package A could leave package B broken.

But in this case, if a user tries to delete libwapcaplet, he will have to delete libcss which is a direct dependency of elinks (when CSS is on).  Thus that deletion attempt will cause elinks to be deleted as well.  (Unless the user forces the deletion with pkg delete -f, but having libwapcaplet in the elinks LIB_DEPENDS won't matter in that case).

Hmmm... I take it back.  It looks like there a few direct dependencies in the elinks source code, such as this one:

% grep lwc_intern_st work/elinks-0.17.0/src/document/libdom/css.c
        lerror = lwc_intern_string(url, strlen(url), abs);

But it uses a few lwc_*() functions BECAUSE of libcss and solely due to the elinks code that is using the libcss functions.  So there's a bit of a gray area here.  There's no reason for the lwc_*() calls without the code that calls the css library.  elinks has no reason to make lwc_*() calls by itself when the css code is not in use.

But since the lwc usage is not fully encapsulated in just libcss code, I suppose it doesn't hurt to add libwapcaplet in LIB_DEPENDS.  We don't want to grow the *_DEPENDS lists to include all dependencies of dependencies as that would quickly get unmanageable.  But this particular "dep of dep" usage is not completely encapsulated in the first dependency level, so to appease stage-qa, we could just add it even though the libcss dependency is likely sufficient to list.  I'll update the patch, and let Dustin weigh in.
Comment 8 John Hein 2024-09-15 19:56:49 UTC
Created attachment 253589 [details]
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (v3)

Changes from the v2 patch concerning stage-qa findings regarding some LIB_DEPENDS [[1]]:

- appease stage-qa and add libwapcaplet to LIB_DEPENDS (probably not strictly needed, but see previous comment 6 and comment 7)

- also address some LIB_DEPENDS needed or no longer needed (also found by stage-qa):

 - libintl.so is not directly used by elinks, so change USES=gettext to USES=gettext-runtime:build.  gettext-tools is also needed (only a build time dependency by default) as msgfmt is invoked during the build.

 - nspr is no longer needed by elinks in 0.17.0 (or perhaps earlier)

 - libxml++ is no longer needed by elinks in 0.17.0 (or perhaps earlier)

 - libmozjs should have been a LIB_DEPENDS item (with SPIDERMONKEY on) - add it



[[1]]
====> Running Q/A tests (stage-qa)
Error: /usr/local/bin/elinks is linked to /usr/local/lib/libmozjs-102.so from lang/spidermonkey102 but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libmozjs-102.so:lang/spidermonkey102
Error: /usr/local/bin/elinks is linked to /usr/local/lib/libwapcaplet.so.0 from textproc/libwapcaplet but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libwapcaplet.so:textproc/libwapcaplet
Warning: you might not need LIB_DEPENDS on libnspr4.so
Warning: you might not need LIB_DEPENDS on libxml++-5.0.so
Warning: you might not need LIB_DEPENDS on libintl.so
Comment 9 Robert Clausecker freebsd_committer freebsd_triage 2024-09-15 20:58:39 UTC
(In reply to John Hein from comment #7)

Even transitive dependencies must be declared in LIB_DEPENDS.  This helps pkg track shared library dependencies so it knows when to reinstall a package if shared library dependencies change (e.g. new soversion).

Please do not try to be smarter than our scripts.

Also if I enable option SPIDERMONKEY but not CSS, the build seems to fail during configuration.  Perhaps you should make the SPIDERMONKEY option dependent on CSS being enabled.
Comment 10 Dustin Marquess 2024-09-15 21:12:58 UTC
Comment on attachment 253589 [details]
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (v3)

We need to fix this with the input from fuz's last comment. Would you like to do it or would you like me to do it? Either way, thank you for this work that you've done so far.
Comment 11 Dustin Marquess 2024-09-15 22:22:19 UTC
The Spidermoney->CSS dependency must be new, because I just added the CSS option this year and the Spidermonkey option has existed for a while now.
Comment 12 Robert Clausecker freebsd_committer freebsd_triage 2024-09-15 23:36:17 UTC
In any case, I'll take this one out of my current batch until there is consensus on what to commit.
Comment 13 John Hein 2024-09-16 01:28:39 UTC
(In reply to Dustin Marquess from comment #11)
Sorry if it wasn't clear - there's no spidermonkey / css interdependency.
The spidermonkey changes are just "while here, fix other stage-qa findings".
Comment 14 Dustin Marquess 2024-09-16 02:40:21 UTC
Fuz in comment #9 says otherwise.
Comment 15 John Hein 2024-09-16 04:13:13 UTC
(In reply to Robert Clausecker from comment #9)
(In reply to Dustin Marquess from comment #14)

I can reproduce the error with default options (where CSS=off by default) except SPIDERMONKEY=on.  'make configure' produces:

 .
 .
Has header "winsock2.h" : NO
Has header "ws2tcpip.h" : NO
Found pkg-config: YES (/usr/local/bin/pkgconf) 2.3.0
Run-time dependency zlib found: YES 1.3.1
Run-time dependency openssl found: YES 3.0.15
Run-time dependency libbrotlidec found: YES 1.1.0
Run-time dependency liblzma found: YES 5.4.5
Run-time dependency mozjs-102 found: YES 102.9.0
Run-time dependency sqlite3 found: YES 3.46.1
Run-time dependency libcurl found: YES 8.9.1
Did not find CMake 'cmake'
Found CMake: NO
Run-time dependency libcss found: NO (tried pkgconfig)

meson.build:499:14: ERROR: Dependency "libcss" not found, tried pkgconfig


I don't know when that started.
But I guess adding SPIDERMONKEY_IMPLIES=CSS would do the trick.
Comment 16 John Hein 2024-09-16 18:06:39 UTC
Created attachment 253609 [details]
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (v4)

(In reply to John Hein from comment #15)
> I don't know when that started.
> But I guess adding SPIDERMONKEY_IMPLIES=CSS would do the trick.

meson.build in 0.17.0 does spell out the requirement for libcss when SPIDERMONKEY=on (or mujs or quickjs).

So v4 of the patch now adds SPIDERMONKEY_IMPLIES=CSS.
Comment 17 John Hein 2024-09-16 18:37:35 UTC
(In reply to John Hein from comment #16)
FYI, the dependency of spidermonkey/other js engines on libcss did not exist in 16.1.1 - it is new for 0.17.0 [[1]]

[[1]] https://github.com/rkd77/elinks/commit/b2ffe855e0f9b832a9c0dd5425e9c15ff98a1a16
Comment 18 Dustin Marquess 2024-09-16 19:21:55 UTC
Comment on attachment 253609 [details]
[patch] support libcss 0.9.2 and libdom 0.4.2 for elinks 0.17.0 (v4)

LGTM!

I really appreciate the help and the submission!
Comment 19 Robert Clausecker freebsd_committer freebsd_triage 2024-09-16 20:09:24 UTC
(In reply to Dustin Marquess from comment #18)

Back on the menu!
Comment 20 commit-hook freebsd_committer freebsd_triage 2024-09-16 20:21:10 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=29a794c2b8900c8a650dc67e8b138d4e9e470a24

commit 29a794c2b8900c8a650dc67e8b138d4e9e470a24
Author:     John Hein <jcfyecrayz@liamekaens.com>
AuthorDate: 2024-09-12 10:07:23 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2024-09-16 20:14:25 +0000

    www/elinks: fix build with CSS option on

     - add upstream patch to support libcss 0.9.2 and libdom 0.4.2
     - appease stage-qa and add libwapcaplet to LIB_DEPENDS
     - also address some LIB_DEPENDS needed or no longer needed (also found by stage-qa):
       - libintl.so is not directly used by elinks, so change USES=gettext to USES=gettext-runtime:build
       - nspr is no longer needed by elinks in 0.17.0 (or perhaps earlier)
       - libxml++ is no longer needed by elinks in 0.17.0 (or perhaps earlier)
       - libmozjs should have been a LIB_DEPENDS item (with SPIDERMONKEY on) - add it
     - gettext-tools is also needed (only a build time dependency by default) as msgfmt is invoked during the build.
     - option SPIDERMONKEY now depends on option CSS

    PR:             281215
    MFH:            2024Q3
    Approved by:    jailbird@fdf.net (maintainer)

 www/elinks/Makefile | 26 +++++++++++++-------------
 www/elinks/distinfo |  4 +++-
 2 files changed, 16 insertions(+), 14 deletions(-)
Comment 21 commit-hook freebsd_committer freebsd_triage 2024-09-16 20:24:11 UTC
A commit in branch 2024Q3 references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=aba551d724952bcc88e3bad19c1d78831a745fa6

commit aba551d724952bcc88e3bad19c1d78831a745fa6
Author:     John Hein <jcfyecrayz@liamekaens.com>
AuthorDate: 2024-09-12 10:07:23 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2024-09-16 20:23:04 +0000

    www/elinks: fix build with CSS option on

     - add upstream patch to support libcss 0.9.2 and libdom 0.4.2
     - appease stage-qa and add libwapcaplet to LIB_DEPENDS
     - also address some LIB_DEPENDS needed or no longer needed (also found by stage-qa):
       - libintl.so is not directly used by elinks, so change USES=gettext to USES=gettext-runtime:build
       - nspr is no longer needed by elinks in 0.17.0 (or perhaps earlier)
       - libxml++ is no longer needed by elinks in 0.17.0 (or perhaps earlier)
       - libmozjs should have been a LIB_DEPENDS item (with SPIDERMONKEY on) - add it
     - gettext-tools is also needed (only a build time dependency by default) as msgfmt is invoked during the build.
     - option SPIDERMONKEY now depends on option CSS

    PR:             281215
    MFH:            2024Q3
    Approved by:    jailbird@fdf.net (maintainer)

    (cherry picked from commit 29a794c2b8900c8a650dc67e8b138d4e9e470a24)

 www/elinks/Makefile | 26 +++++++++++++-------------
 www/elinks/distinfo |  4 +++-
 2 files changed, 16 insertions(+), 14 deletions(-)
Comment 22 Robert Clausecker freebsd_committer freebsd_triage 2024-09-16 20:25:39 UTC
Thank you for the submission.