Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

3.12.0 (3.12.0-b4 and up) is broken on NixOS #15087

Closed
Kiwi opened this issue Oct 31, 2019 · 5 comments · Fixed by #15106
Closed

3.12.0 (3.12.0-b4 and up) is broken on NixOS #15087

Kiwi opened this issue Oct 31, 2019 · 5 comments · Fixed by #15106

Comments

@Kiwi
Copy link

Kiwi commented Oct 31, 2019 •
edited

I recently (ok, a month ago) updated the NixOS module for Matomo to remove deprecation warnings in it. At the same time as I was doing that I had (re)discovered a bug, which I later found had already been reported, and then fixed in 3.12.0-b3 (or possibly an earlier one, I can't remember; that was the most recent).

That made me decide to add a matomo-beta to NixOS, which I did as part of that PR, to allow for more frequent updates if people wanted the betas.

Here's the original issue.

While going through the review process and cleaning it up to be merged 3.12.0-b4 was released. I bumped the version and then we found out that something changed between b3 and b4 that made it error. (And whatever that change was has persisted)

Here's the error message.

We'd wanted to figure out what the problem/fix was before doing the merge but given time constraints of involved and at least in my case complete unfamiliarity with the code we're still not sure how to fix it. That's why I'm here, I'm hoping someone can help us figure out what was changed and an appropriate fix and hopefully submit a PR here to resolve it.

In the past @florianjacob has worked with you on similar issues and suspects it might be related to using one of these when the other should be used; PIWIK_DOCUMENT_ROOT | PIWIK_USER_PATH Beyond that and the error message and the time range it broke I'm not sure what else there is to go on.

I'm (and I expect florianjacob as well) willing to help in anyway I can to get this fixed. On my own I'm a bit out of my depth at this point. I've already put a lot of time into it and haven't been able to figure it out.

I'm on irc.freenode.net as DigitalKiwi and I'm currently in the #matomo channel. If anyone would like to reach out so that Matomo isn't stuck on 3.11.0 for everyone on NixOS (or 3.12.0-b3...), it would be much appreciated, thanks!

P.S. Amusingly, there are enough people on NixOS that use Matomo that while mine was in the process there were 2 other duplicate/similar PRs that were submitted...

edit: presumably a change done in one of these commits is the cause.
@florianjacob
Copy link
Contributor

florianjacob commented Nov 3, 2019 •
edited

I identified the cause, it is #14866 which broke PIWIK_DOCUMENT_ROOT being a shared, read-only directory and PIWIK_USER_PATH being the directory to relocate writable config and tmp to.

See the documentation for PIWIK_USER_PATH:

PIWIK_USER_PATH – override the default to relocate config and tmp files outside the web document root. This facilitates a “best practice” of preventing direct access to php files. It is also useful with shared hosting to separate shared code from user/account-specific configuration. The default is the same as PIWIK_DOCUMENT_ROOT. Note: open_basedir() restrictions may apply

A case where this was previously broken: #11654
A team member's comment explaining the constant's indended use: #1453 (comment)

EDIT: wrongly copied the docs for PIWIK_INCLUDE_PATH. PIWIK_INCLUDE_PATH is unrelated, this is all about PIWIK_DOCUMENT_ROOT and PIWIK_USER_PATH.

@florianjacob florianjacob mentioned this issue Nov 3, 2019
Merged
@tsteur
Copy link
Member

tsteur commented Nov 3, 2019

@florianjacob could you maybe explain what the issue is with PIWIK_USER_PATH? I'm not quite understanding the problem from above comment and also why you mention PIWIK_INCLUDE_PATH etc?

PIWIK_USER_PATH: There you should have basically your config.ini.php in there. Is this a problem when you install it headless maybe or so? And how / where do you define PIWIK_USER_PATH and when?

@florianjacob
Copy link
Contributor

I'm sorry for not being able to bring the issue across, I'll try again to explain what we're doing with more context:

We maintain the matomo package for the NixOS Linux distribution.
The package installs the contents of the matomo release zip files to /usr/lib/matomo and uses that as PIWIK_DOCUMENT_ROOT. This distribution though enforces a strict policy of /usr/ being read-only. Everything that is variable, like Matomo's config.ini.php and tmp has to go to /var/lib/matomo. The idea is that the files in PIWIK_DOCUMENT_ROOT = /usr/lib/matomo can be shared read-only between multiple Matomo instances. While having shared package files in a read-only /usr is good practice, other distributions don't enforce this, and either allow to install the matomo files to /var, or allow to have writable files in /usr.

With Matomo 3.11 and below, we separated the read-only files that come with the package and the read-write files that are created or modified at runtime by having a bootstrap.php in /usr/lib/matomo, i.e. the PIWIK_DOCUMENT_ROOT, that sets PIWIK_USER_PATH to /var/lib/matomo. Matomo upgrades are handled by the operating system, so Matomo did not require write access to its PIWIK_DOCUMENT_ROOT. Everything worked out.

#14866 changed the behaviour of PIWIK_USER_PATH that it doesn't move all read-write files and folders from PIWIK_DOCUMENT_ROOT anymore, i.e. it tries to write to ${PIWIK_DOCUMENT_ROOT}/tmp (and config/global.php & config/global.ini.php? I'm not sure if those files are written if matomo upgrades are handled by the operating system and not matomo itself).

I hope this comprehensible, thanks for your patience.

(For NixOS-proficient readers: With /usr/lib/matomo I mean /nix/store, but the NixOS specifics don't matter here)

@tsteur
Copy link
Member

tsteur commented Nov 4, 2019

I understand now. I didn't understand why config be an issue but now it's clear that only tmp is an issue. The solution be to change this line back: https://github.com/matomo-org/matomo/pull/14866/files#diff-56421c8d6bc21777119c147c3c6aa589R11 to 'path.root' => PIWIK_USER_PATH,. I'm not sure this worked before by intention as path.root did not quite sound like it should be user related. I'll have a think and create a PR

tsteur added a commit that referenced this issue Nov 4, 2019
@tsteur
tmp directory should use user path
a4c2f0c 
fix #15087

See eg #15087 (comment) and #15087 (comment)
@tsteur tsteur mentioned this issue Nov 4, 2019
Merged
@tsteur
Copy link
Member

tsteur commented Nov 4, 2019

Create a PR. Here's the diff https://github.com/matomo-org/matomo/pull/15106/files

diosmosis pushed a commit that referenced this issue Nov 5, 2019
@tsteur @diosmosis
tmp directory should use user path (#15106)
dd39a1c 
fix #15087

See eg #15087 (comment) and #15087 (comment)
diosmosis pushed a commit that referenced this issue Nov 6, 2019
@tsteur @diosmosis
tmp directory should use user path (#15106)
7570b8b 
fix #15087

See eg #15087 (comment) and #15087 (comment)
@Kiwi Kiwi mentioned this issue Nov 18, 2019
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tmp directory should use user path matomo-org/matomo
3 participants
@Kiwi @tsteur @florianjacob