Bug 215919 - www/hiawatha [MAINTAINER] update to 10.4
Summary: www/hiawatha [MAINTAINER] update to 10.4
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: Nikolai Lifanov
URL:
Keywords: easy, patch, patch-ready
Depends on:
Blocks:
 
Reported: 2017-01-09 23:26 UTC by Chris Hutchinson
Modified: 2017-03-02 21:13 UTC (History)
4 users (show)

See Also:
portmaster: maintainer-feedback+


Attachments
svn diff (patch) for www/hiawatha (2.32 KB, patch)
2017-01-09 23:26 UTC, Chris Hutchinson
no flags Details | Diff
svn diff version 2, for www/hiawatha (2.08 KB, patch)
2017-01-12 01:06 UTC, Chris Hutchinson
portmaster: maintainer-approval+
Details | Diff
svn diff (patch) for www/hiawatha (2.21 KB, patch)
2017-02-23 06:14 UTC, Chris Hutchinson
portmaster: maintainer-approval+
Details | Diff
svn diff (patch) for www/hiawatha - adds TLS mbed, upgrades to 10.5... (2.59 KB, patch)
2017-02-23 19:48 UTC, Chris Hutchinson
portmaster: maintainer-approval+
Details | Diff
candidate cleaned up patch (2.68 KB, patch)
2017-03-02 18:09 UTC, Nikolai Lifanov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Hutchinson 2017-01-09 23:26:37 UTC
Created attachment 178684 [details]
svn diff (patch) for www/hiawatha

this pr(1) updates www/hiawatha from 10.3_1 to 10.4

from hiawatha Changelog:

 mbed TLS updated to 2.3.0.
 SkipCacheCookie option added.
 Added Systemd init script to Debian package.
 Small improvements and bugfixes.

Tested on RELENG_11

Please find www-hiawatha.diff attached
Comment 1 Olivier Duchateau freebsd_committer 2017-01-10 18:29:27 UTC
Why did you remove @sample keyword in pkg-plist?

See Porter Handbook for help, https://www.freebsd.org/doc/en/books/porters-handbook/plist-keywords.html#plist-keywords-sample
Comment 2 Nikolai Lifanov freebsd_committer 2017-01-11 14:43:53 UTC
I'm waiting for an answer for why @sample keyword is gone:
the files in the plist still end in .sample.
Comment 3 Chris Hutchinson 2017-01-12 01:06:35 UTC
Created attachment 178767 [details]
svn diff version 2, for www/hiawatha
Comment 4 Chris Hutchinson 2017-01-12 01:13:33 UTC
(In reply to Nikolai Lifanov from comment #2)
> I'm waiting for an answer for why @sample keyword is gone:
> the files in the plist still end in .sample.

Sorry for the delay. My monitor died, which makes
it pretty hard to read anything on this list, or
anything else, for that matter. ;-)

Anyway, to answer the question;
the change in the plist, was the product of
make -DBATCH makeplist
... the results of which worked fine for
my releng_11 install.
However, as a response, I have attached another
svn diff ( www-hiawatha-02.diff ) that I hope
will adequately address your concerns. :)

Thanks for all your time, and attention to this! :)

--Chris
Comment 5 Nikolai Lifanov freebsd_committer 2017-01-12 15:00:55 UTC
Hi, no problem!

I tested the second patch and I find that some files are not installed:

===> Checking for items in pkg-plist which are not in STAGEDIR
Error: Missing: etc/rc.d/hiawatha
Error: Missing: %%DOCSDIR%%/ChangeLog
Error: Missing: %%DOCSDIR%%/README.md
===> Error: Plist issues found.
*** Error code 1

I see that the files are there and they are included in "make makeplist" output. Maybe they are not installed early enough in the process?


There are also some dependencies which are absent:

Error: /usr/local/lib/hiawatha/libmbedtls.so.2.3.0 is linked to /usr/local/lib/libmbedx509.so.0 from security/mbedtls but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libmbedx509.so:security/mbedtls
Error: /usr/local/lib/hiawatha/libmbedtls.so.2.3.0 is linked to /usr/local/lib/libmbedcrypto.so.0 from security/mbedtls but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libmbedcrypto.so:security/mbedtls
Error: /usr/local/sbin/hiawatha is linked to /usr/local/lib/libmbedtls.so.10 from security/mbedtls but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libmbedtls.so:security/mbedtls

You should probably depend on it just once in case SSL is enabled.
Comment 6 Leo Neo 2017-02-07 16:44:16 UTC
Hiawatha 10.5 was released since this bug was opened: https://www.hiawatha-webserver.org/changelog

Considering 10.5 -among other things- contains another update to the aforementioned mbedtls dependency, wouldn't it make sense to skip 10.4 and go straight to 10.5?
Comment 7 Nikolai Lifanov freebsd_committer 2017-02-07 16:47:34 UTC
Yes, it would.
Comment 8 Chris Hutchinson 2017-02-07 18:38:10 UTC
(In reply to Nikolai Lifanov from comment #7)
> Yes, it would.

+1

I'll create a new patch against the new version.

--Chris
Comment 9 Nikolai Lifanov freebsd_committer 2017-02-18 23:39:25 UTC
Hi Chris!

Thanks a ton for your work, and I'd like to move this forward.
Do you have any news about 10.5 update and fixing "make check-orphans" issues?
Comment 10 Chris Hutchinson 2017-02-23 05:22:24 UTC
(In reply to Nikolai Lifanov from comment #9)
> Hi Chris!
> 
> Thanks a ton for your work, and I'd like to move this forward.
> Do you have any news about 10.5 update and fixing "make check-orphans"
> issues?

Sorry for the delay! $WORK$ is getting in my way. In addition to
the upgrade to 10.5, I'm also testing the ability to add the
additional TLS (mbed TLS) support option ( pr211239 ).

I'm working on it, as I write this. Barring any more interruptions
from $WORK$ I should have something early tomorrow (PST).

--Chris
Comment 11 Chris Hutchinson 2017-02-23 06:14:50 UTC
Created attachment 180239 [details]
svn diff (patch) for www/hiawatha

Upgrades to 10.5
adds TLS (mbed TLS) to OPTIONS
Comment 12 Chris Hutchinson 2017-02-23 06:17:23 UTC
(In reply to Chris Hutchinson from comment #11)
> Created attachment 180239 [details]
> svn diff (patch) for www/hiawatha
> 
> Upgrades to 10.5
> adds TLS (mbed TLS) to OPTIONS

OK this one adds TLS (mbed TLS) OPTION
as well as upgrades to 10.5

I think we're done here -- finally! :-)

Builds like a dream on RELENG_11

--Chris out...
Comment 13 Leo Neo 2017-02-23 15:52:07 UTC
Great! Considering Hiawatha is heavily geared towards security I assume the new option for mbedTLS is on by default?

Also, considering Hiawatha doesn't support SSL any more (the default minimum is TLS 1.1) that SSL_CMAKE_BOOL= is redundant now. Would removing it create compatibility issues? If so it might makes sense to map SSL_CMAKE_BOOL= to TLS_CMAKE_BOOL=	and make both options do the same.
Comment 14 Chris Hutchinson 2017-02-23 19:31:58 UTC
(In reply to Leo Neo from comment #13)
> Great! Considering Hiawatha is heavily geared towards security I assume the
> new option for mbedTLS is on by default?
> 
> Also, considering Hiawatha doesn't support SSL any more (the default minimum
> is TLS 1.1) that SSL_CMAKE_BOOL= is redundant now. Would removing it create
> compatibility issues? If so it might makes sense to map SSL_CMAKE_BOOL= to
> TLS_CMAKE_BOOL=	and make both options do the same.

And here I thought I was FINALLY done. :-P ;-)

OK you bring up some good points here. Simply removing
the SSL option /should/ be adequate
(TLS /implies/ SSL, anyway). In fact, that's what
I'm going to do. I'll also tweak the "TLS (mbed)"
OPTION as a default.

Result svn(1) diff(1) coming in just a few short minutes...

--Chris out...
Comment 15 Chris Hutchinson 2017-02-23 19:48:28 UTC
Created attachment 180247 [details]
svn diff (patch) for www/hiawatha - adds TLS mbed, upgrades to 10.5...

OK this should be the LAST one (fingers crossed).
Anyway;
this patch adds TLS mbed, and upgrades hiawatha
to 10.5.
This patch also removes the SSL option (TLS already
implies SSL)

Patch tested on RELENG_11

Thanks for your input, Leo!

--Chris
Comment 16 Nikolai Lifanov freebsd_committer 2017-02-23 19:52:29 UTC
OK, back to my lab :)
Comment 17 Nikolai Lifanov freebsd_committer 2017-02-23 20:22:48 UTC
Other TLS tookits have a shared option description.
POLARSSL had one. I'm trying to replace it with one for MBEDTLS.

Can this wait a bit while I get this approved?
Comment 18 Chris Hutchinson 2017-02-23 23:28:39 UTC
(In reply to Nikolai Lifanov from comment #17)
> Other TLS tookits have a shared option description.
> POLARSSL had one. I'm trying to replace it with one for MBEDTLS.
> 
> Can this wait a bit while I get this approved?

I'm not exactly sure what you're trying to say;
The description via the config? If so; you don't
care for the one I used?
No matter, take what ever time you need, Nikolai. :-)

--Chris
Comment 19 Nikolai Lifanov freebsd_committer 2017-02-24 00:42:31 UTC
Hi!

So, a bit of background:

We have Mk/bsd.options.desc.mk, which is a file that has default descriptions for common options. That's why you are able to have OPTIONS=OSS and not specify a description for what this means. It results in consistent language describing same options throughout the tree.

All TLS implementations, from OPENSSL, to POLARSSL, WOLFSSL, GNUTLS, NSS, etc. have a description in this file. The mbed TLS software used to be PolarSSL, which had the POLARSSL description. I'm trying to add an MBEDTLS description to it.

The revision that's up for review is here:
https://reviews.freebsd.org/D9772
Comment 20 Chris Hutchinson 2017-02-24 05:00:07 UTC
(In reply to Nikolai Lifanov from comment #19)
> Hi!
> 
> So, a bit of background:
> 
> We have Mk/bsd.options.desc.mk, which is a file that has default
> descriptions for common options. That's why you are able to have OPTIONS=OSS
> and not specify a description for what this means. It results in consistent
> language describing same options throughout the tree.
> 
> All TLS implementations, from OPENSSL, to POLARSSL, WOLFSSL, GNUTLS, NSS,
> etc. have a description in this file. The mbed TLS software used to be
> PolarSSL, which had the POLARSSL description. I'm trying to add an MBEDTLS
> description to it.
> 
> The revision that's up for review is here:
> https://reviews.freebsd.org/D9772

Ah, sure. Thanks for the clarification, and all your
time on this, Nikolai. Greatly appreciated!

--Chris
Comment 21 Nikolai Lifanov freebsd_committer 2017-03-02 17:39:35 UTC
We're getting close!

I'm still seeing some unreported dependencies:
====> Running Q/A tests (stage-qa)
Warning: setuid files in the stage directory (are these necessary?):
2140736 -rwsr-xr-x  1 lifanov  wheel  28464 Mar  2 12:38:21 2017 /usr/home/lifanov/src/svn/freebsd/ports/head/www/hiawatha/work/stage/usr/local/sbin/cgi-wrapper
Error: /usr/local/lib/hiawatha/libmbedtls.so.2.4.0 is linked to /usr/local/lib/libmbedx509.so.0 from security/mbedtls but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libmbedx509.so:security/mbedtls
Error: /usr/local/lib/hiawatha/libmbedtls.so.2.4.0 is linked to /usr/local/lib/libmbedcrypto.so.0 from security/mbedtls but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libmbedcrypto.so:security/mbedtls
Error: /usr/local/sbin/hiawatha is linked to /usr/local/lib/libmbedtls.so.10 from security/mbedtls but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libmbedtls.so:security/mbedtls

Also, there is no "%doc" keyword. Perhaps you meant to do %%DOCS%%?
Comment 22 Nikolai Lifanov freebsd_committer 2017-03-02 18:08:32 UTC
OK, I think I got this cleaned up a bit:

o move work directory to /var/db/hiawatha:
   FreeBSD doesn't use /var/lib. It's a Linux thing.

o switch to (new) global MBEDTLS option

o unbundle mbed TLS

   Bundling libraries is generally frowned upon, but in this case it's a security-sensitive library, so when so@ updates the system version, the possibly vulnerable bundled version can't be updated. Also, the port ended up linking to the system version of mbedtls when it was installed, which creates a different package depending on the user's system, which is also frowned upon. There was also a soup of incorrect and fragile linking in this case, where /usr/local/lib/hiawatha/libmbedtls.so.2.4.0 was linked to, for example, /usr/local/lib/libmbedx509.so.0 from mbedtls package. This would have resulted in runtime crashes when mbedtls port was updated.

o clean up plist

   There is no "@doc" macro. Simply marking something %%PORTDOCS%% is sufficient. But this is made redundant since "PORTDOCS= ChangeLog README.md" was listed in the main Makefile, so it should just be removed from pkg-plist.


This version passes all tests and works correctly.
Please review the candidate patch and let me know what you think.
Comment 23 Nikolai Lifanov freebsd_committer 2017-03-02 18:09:04 UTC
Created attachment 180443 [details]
candidate cleaned up patch
Comment 24 Leo Neo 2017-03-02 18:36:15 UTC
With regards to 'unbundling' mbedTLS, the Hiawatha source tarball comes with its own source for mbedTLS included. That is also why an updated version of mbedTLS is mentioned in the Hiawatha changelog.

I take it that this means that a specific version Hiawatha had been developed and tested with that specific version of mbedTLS.

Can't the port just use the version supplied with Hiawatha irrespective of which version (if any) of mbedTLS is already installed? Or am I missing things?
Comment 25 Nikolai Lifanov freebsd_committer 2017-03-02 18:50:21 UTC
Re: Leo

First, we really don't like bundled libraries. There is a bit of background as to why here: https://www.freebsd.org/doc/en/books/porters-handbook/bundled-libs.html
These reasons are doubly valid for TLS toolkits.

If we must use the bundled version, at the minimum we would have to set BUNDLE_LIBS so that "pkg" tool doesn't register these as provided, which would break building other ports depending on mbedtls.

Second, it really doesn't work in this instance. The cmake machinery in Hiawatha doesn't provide an option to hide the system library from the build process if present and ends up with a cross-linked soup where internal libraries are linked with the system ones and an incompatible system library update would break the port in very unpredictable and crashy ways.

I understand the concern, but normally when the library ABI provided by a dependent port changes, the soname is bumped and optionally a new port is created to satisfy the dependency for software that really is not compatible with the new library.
Comment 26 Chris Hutchinson 2017-03-02 21:07:42 UTC
Comment on attachment 180443 [details]
candidate cleaned up patch

Approved!

A *massive* thank you, for all the work you put into this, Nikolai!
And apologies. I'm much better at trapping such things. But my
build box is lagging. I bought a nice new replacement, only to
discover that the graphics embedded in the APU (AMD/ATI) isn't
(yet) supported on FreeBSD.
So I shelled out another ~$150 on a new nVidia card, that should
arrive tomorrow. Which will enable me to pullup to CURRENT (12)
with a newer ports framework.

Thanks again, Nikolai!

--Chris
Comment 27 commit-hook freebsd_committer 2017-03-02 21:12:27 UTC
A commit references this bug:

Author: lifanov
Date: Thu Mar  2 21:11:31 UTC 2017
New revision: 435296
URL: https://svnweb.freebsd.org/changeset/ports/435296

Log:
  update www/hiawatha to 10.5

  o switch to mbed TLS
  o SkipCacheCookie option added
  o small improvements and bugfixes

  PR:		215919
  Submitted by:	Chris Hutchinson <portmaster@bsdforge.com> (maintainer)

Changes:
  head/www/hiawatha/Makefile
  head/www/hiawatha/distinfo
  head/www/hiawatha/pkg-plist
Comment 28 Nikolai Lifanov freebsd_committer 2017-03-02 21:13:42 UTC
Committed with changes, thanks!

Also, no problem and thanks for your work!