Bug 231663 - www/tdom: update to 0.9.1
Summary: www/tdom: update to 0.9.1
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Mikhail Teterin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-24 13:43 UTC by Pietro Cerutti
Modified: 2019-02-28 12:05 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (mi)


Attachments
Update to 0.9.1 (2.38 KB, patch)
2018-09-24 13:43 UTC, Pietro Cerutti
no flags Details | Diff
Upgrade to 0.9.1 without requiring gmake (3.71 KB, patch)
2018-10-14 14:12 UTC, Mikhail Teterin
no flags Details | Diff
Allow building with Expat from ports OR base (4.67 KB, patch)
2018-10-17 12:06 UTC, Pietro Cerutti
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pietro Cerutti freebsd_committer 2018-09-24 13:43:06 UTC
Created attachment 197433 [details]
Update to 0.9.1
Comment 1 Mikhail Teterin freebsd_committer 2018-10-14 14:12:04 UTC
Created attachment 198136 [details]
Upgrade to 0.9.1 without requiring gmake

Here is the slightly altered patch, which avoids the need for gmake.

However, on my system one of the vendor's self-tests is failing now:

==== dom-2.29 parse not well-formed document with undeclared xml prefix FAILED
==== Contents of test case:

    catch {dom parse {<foo:e/>}} errMsg
    string range $errMsg 0 30

---- Result was:
domDoc0x801897280
---- Result should have been (exact matching):
Namespace prefix is not defined

Have you tried "make test" yourself? Was it clean?
Comment 2 commit-hook freebsd_committer 2018-10-14 14:55:50 UTC
A commit references this bug:

Author: mi
Date: Sun Oct 14 14:55:01 UTC 2018
New revision: 482044
URL: https://svnweb.freebsd.org/changeset/ports/482044

Log:
  Upgrade from 0.9.0 to 0.9.1.

  PR:		231663
  Submitted by:	gahr

Changes:
  head/www/tdom/Makefile
  head/www/tdom/distinfo
  head/www/tdom/files/patch-Makefile.in
  head/www/tdom/files/patch-generic-dom.h
  head/www/tdom/files/patch-tests-tdom
  head/www/tdom/pkg-plist
Comment 3 Mikhail Teterin freebsd_committer 2018-10-14 14:57:41 UTC
(In reply to Mikhail Teterin from comment #1)
> dom-2.29 parse not well-formed document with undeclared xml prefix FAILED

I submitted a ticket with the authors:

http://www.tdom.org/index.html/tktview/2ca7a4547a9820c824289fd20871f94a213d7a23

and marked this test as "knownBug" for the time being.

Thank you!
Comment 4 rolf 2018-10-16 22:20:03 UTC
Turns out, that I have known this. See [1].

In fact, it was my bug report on upstream (expat), that triggered a
fix (see [2]).

So this is indeed due to a different version of expat you are using --
FreeBSD's own "bsdxml" is apparently 2.1.0 (8 releases behind), while
the tdom source distribution comes bundled with 2.2.5 (one release
behind, with trunk already on current upstream), marking the minimum
expected expat version, if dynamic linking with the sysmte library is
preferred.

As long as FreeBSD's system expat (libbsdxml) is behind the minimum
required expat version for tdom you should provide your user an option
to get tdom statically linked with the bundled one.

(Wasn't it you to make that [3] fuss about "stop bundling expat"?)


1) [http://tdom.org/index.html/artifact?ln=523-531&name=e0febcbda4b7d4f1]

2) [https://github.com/libexpat/libexpat/issues/137]

3) [https://github.com/tDOM/tdom/issues/27]
Comment 5 Mikhail Teterin freebsd_committer 2018-10-17 00:42:32 UTC
(In reply to rolf from comment #4)
> FreeBSD's own "bsdxml" is apparently 2.1.0 (8 releases behind)

On my FreeBSD-11, the expat is 2.2.0... But the bug may well still be there...

Well, until FreeBSD's bsdxml is upgraded, "knownBug" is the solution, I guess. Not a bug in the Trf, of course, but known nonetheless.

> Wasn't it you to make that fuss about "stop bundling expat"?

Indeed, that was me -- and I remain of the opinion, you should stop the bundling. The port makes an effort to not even extract the bundled expat, when building Trf. For all the reasons stated on the Github ticket and here:

https://www.freebsd.org/doc/en/books/porters-handbook/bundled-libs.html

Thank you!
Comment 6 rolf 2018-10-17 10:56:16 UTC
(In reply to Mikhail Teterin from comment #5)

> On my FreeBSD-11, the expat is 2.2.0... But the bug may well still
> be there...

So only six releases behind. Great.

Yes, the expat bug in question here is still in 2.2.0, as you could
easily see by following the links I've provided.

> I remain of the opinion, you should stop the bundling.

Well, the world is bigger than FreeBSD (or unix), even if you maybe
think it shouldn't be. As already repeatedly stated in that older
discussion, you're free to link against the system expat; this is a
supported option.

As it is, this means for FreeBSD with respect to expat (and tdom):
more security problems, more bugs, more waste of developer effort
because of reports of already cared about bugs. But at least you did
"the right thing".
Comment 7 Pietro Cerutti freebsd_committer 2018-10-17 12:05:59 UTC
Mikhail, how about allowing tDOM to be built against expat from ports, perhaps behind an OPTION?

I think it's reasonable to assume that most people doing any xml/html-related development will have expat installed anyway. Expat is quite small (expat-2.2.6_1 is 591KiB on my system) so it's not unreasonable to have it as a dependency anyway.
Comment 8 Pietro Cerutti freebsd_committer 2018-10-17 12:06:40 UTC
Created attachment 198264 [details]
Allow building with Expat from ports OR base
Comment 9 commit-hook freebsd_committer 2019-02-28 12:05:46 UTC
A commit references this bug:

Author: gahr
Date: Thu Feb 28 12:05:43 UTC 2019
New revision: 494145
URL: https://svnweb.freebsd.org/changeset/ports/494145

Log:
  www/tdom: add an OPTION (default off) to build against expat from ports

  PR:		231663 (was mentioned in comment #7)
  Submitted by:	gahr
  Approved by:	maintainer timeout

Changes:
  head/www/tdom/Makefile
  head/www/tdom/files/patch-bsdxml
  head/www/tdom/files/patch-generic-dom.h
  head/www/tdom/files/patch-tests-tdom