Bug 248601

Summary: www/tt-rss: Missing dependencies, wrong filenames and default permissions
Product: Ports & Packages Reporter: Ulrich Spörlein <uqs>
Component: Individual Port(s)Assignee: Ulrich Spörlein <uqs>
Status: Closed FIXED    
Severity: Affects Many People CC: dereks
Priority: --- Keywords: needs-qa
Version: LatestFlags: koobs: maintainer-feedback+
koobs: merge-quarterly?
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D26061
Attachments:
Description Flags
First pass at fixing issues with a fresh install
none
Current patch from phab diff
none
v3 from Phab diff. Fixing pkg-message and changing base directory permissions
none
v4 final Patch from D26061 dereks: maintainer-approval+

Description Ulrich Spörlein freebsd_committer freebsd_triage 2020-08-11 13:44:40 UTC 
I've already sent this to the maintainer per email. A recent setup of tt-rss in a a fresh jail ran into several problems:

First of all, once installed, it complains that php72-iconv isn't
installed, so I had to add that manually. Seems like the port needs a
patch for that and it has grown a new dependency.

Now, when trying the "install" it errors out, as it want to see a
config.php-dist, but the port Makefile actually renames that. The error
I'm getting is

<br />
<b>Warning</b>:  file_get_contents(../config.php-dist): failed to open stream: No such file or directory in <b>/usr/local/www/tt-rss/install/index.php</b> on line <b>154</b><br />

Once I moved the file to the old name, I get a permission error in it :)

I've chmod 0600 the resulting config.php, I guess one would have to
check whether the permissions come out right.


After that, it loads and runs into the next problem again:
Image cache is not writable (chmod -R 777 cache/images)

The directory exists, not sure who created it though, it looks a bit out
of place:

root@ttrss:/usr/local/www/tt-rss # ll cache
total 2
drwxr-xr-x  2 www   www    3 Jul  1 16:48 export/
drwxr-xr-x  2 www   www    3 Jul  1 16:48 feeds/
drwxr-xr-x  2 root  wheel  3 Jul 31 13:25 images/
drwxr-xr-x  2 www   www    3 Jul  1 16:48 upload/

In fact, there are some files and dirs owned by root. Is this
intentional?

root@ttrss:/usr/local/www/tt-rss # find . -user root|sort
./cache/images
./config.php-dist
./js/form
./lib/flat-ttrss
./lib/flat-ttrss/fonts
./lib/flat-ttrss/images
./lib/iconfont
./locale/eo
./locale/eo/LC_MESSAGES
./locale/id
./locale/id/LC_MESSAGES
./locale/uk_UA
./locale/uk_UA/LC_MESSAGES
./plugins/af_proxy_http
./plugins/af_readability/vendor
./plugins/af_readability/vendor/andreskrey
./plugins/af_readability/vendor/andreskrey/Readability
./plugins/af_readability/vendor/andreskrey/Readability/Nodes
./plugins/af_readability/vendor/andreskrey/Readability/Nodes/DOM
./plugins/hotkeys_force_top
./plugins/hotkeys_noscroll
./plugins/hotkeys_swap_jk
./templates.local
./themes/light
./vendor
./vendor/Psr


It looks like a strange mix to me.

Do you want me to commit a patch that
1. add php72-iconv
2. names the file config.php-dist
3. double-checks permissions in the resulting pkg and potentially change
   all (?) of them to www:www ?

Cheers
Uli
Comment 1 Derek Schrock 2020-08-12 00:52:30 UTC 
I believe the original intent was to not use the web interface to init. the database and write out the config.php which is where you're running in to issues with config.php-dist.

You can see this via the pkg-message.  Edit config.php as needed and init. the database via psql command as the postgres user with the proper schema file, database, user.

So is there a need to support the web interface here?  If not the existing config.php.sample and config.php can remain as is.

Yep, I was looking at include/sanity_check.php for requirements didn't realize there's a couple other places to look and my existing installation had iconv for other reasons.   So yep we need to add iconv to USE_PHP.

We'll need an @dir( ... cache/images) here too.. oops.

Yes it appears some of those root owned dirs/files are from mkdir -p created locations maybe?

I see a couple other changes that semi-break the reconfig of pgsql/mysql on the fly in the dist/sample and it appears the apache sample config is a little dated.

See patch for resolving 1 and 3.  Should we ignore 2?
Comment 2 Derek Schrock 2020-08-12 00:52:59 UTC 
Created attachment 217163 [details]
First pass at fixing issues with a fresh install
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2020-08-13 12:37:03 UTC 
^Triage: Reporter is committer, assign accordingly.

Any committer may commit to any repository with an accepted review from any committer with existing access to that repository.

Committers may obtain review via a Differential in Phabricator, adding the "Contributor Reviewers ($Repository)" group as a Reviewer, reaching out to other committers; directly or via mailing lists, or setting the attachment flag to: maintainer-approval ? <person-youd-like-to-review>
Comment 4 Ulrich Spörlein freebsd_committer freebsd_triage 2020-08-13 13:04:25 UTC 
Hi Derek, thanks for looking at this. The patch looks good to me for (1) and (3) (I haven't used the apache config, as I'm running this under nginx).

As for (2), I think we should fix it, especially as:
- the fix is trivial, just don't rename the file, and
- the web-based setup seems more documented online and apparently is what the developers of tt-rss support, and
- that web-based setup correctly flagged the missing iconv module, so it's a good thing :)

It was also easier to setup and init the database. I rarely need to use the psql or mysql CLIs so not having to deal with them is a plus :)

Is there a reason the file is being renamed in the first place? That seems a bit gratuitous. If the changed name is really required, I would suggest to copy/link instead of renaming it, so both dist or sample files are present (and updated).

What do you think?
Comment 5 Derek Schrock 2020-08-13 21:58:29 UTC 
(In reply to Ulrich Spörlein from comment #4)

I'll create a review for this and we'll take the discussion there.
Comment 6 Derek Schrock 2020-08-15 08:16:45 UTC 
Created attachment 217224 [details]
Current patch from phab diff

- Default permissions for cache/images broken on new installations
- Permissions on misc dirs not set to www:www
- apache sample updated with 2.4 syntax
- Add missing PHP extension iconv
- patch web isntaller to make config.php 0400
- Use modern port settings mechinisms
- Don't copy .empty files
- Clean up pkg-plist to be root:www and www:www where write is needed
- Don't try to edit config.php-dist on the fly. Leave that to admin post
  install or use the web installer.
- Keep config.php-dist around for the web installer
- Rewrite inscrutions to support the web installer by opt-in by
  deleting config.php created by @sample
Comment 7 Derek Schrock 2020-08-16 06:02:43 UTC 
Created attachment 217250 [details]
v3 from Phab diff.  Fixing pkg-message and changing base directory permissions

- Add missing % in pkg-message.in from rm line.
- Make the base directory root:www 0755.
  Since the web installer can't create config.php now remove the patch to make it 0400
Comment 8 Derek Schrock 2020-08-19 00:59:29 UTC 
Created attachment 217330 [details]
v4 final Patch from D26061

- apache sample updated with 2.4 syntax
- Add missing PHP extension iconv
- Use alt. options for MYSQL/PGSQL, CURL, GD
- Don't copy .empty files from extracted dist
- Update pkg-message for web installer support
- root:www all files, www:www user rw where needed
Comment 9 commit-hook freebsd_committer freebsd_triage 2020-08-21 13:56:13 UTC 
A commit references this bug:

Author: uqs
Date: Fri Aug 21 13:55:17 UTC 2020
New revision: 545598
URL: https://svnweb.freebsd.org/changeset/ports/545598

Log:
  www/tt-rss: Fix default permissions, apache sample, add missing iconv dep.

  PR:		248601
  Submitted by:	maintainer
  Reported by:	uqs
  Differential Revision:	https://reviews.freebsd.org/D26061

Changes:
  head/www/tt-rss/Makefile
  head/www/tt-rss/files/httpd-tt-rss.conf.in
  head/www/tt-rss/files/pkg-message.in
  head/www/tt-rss/pkg-plist
Comment 10 Ulrich Spörlein freebsd_committer freebsd_triage 2020-08-21 14:04:27 UTC 
Thanks for the fix!