Wordpress improvements #96910

mohe2015 · 2020-09-01T14:50:55Z

Motivation for this change

Improve the wordpress service:

Add separate package that removes built-in themes and plugins as these can't be updated independently and also can't be removed otherwise.
Package auto-update script
Separate databases to allow multiple wordpress installations on one host by default
Add option for mutable wp-content

(edit: updated)

Things done

nixos/modules/services/web-servers/apache-httpd/default.nix

ajs124 · 2020-09-02T01:21:51Z

I haven't gotten a chance to look at this, but wordpress can actually be served mostly read-only, from the nix store. One component that comes in handy for that is this ~~abomination~~beautiful piece of technology. It gives you each and every wordpress theme and plugin as a nix derivation.

mohe2015 · 2020-10-02T12:48:04Z

@ajs124 Would it be possible for you to review my PR? I would really appreciate it.
Also regarding wp4nix I'm only using a few themes and plugins so I think my approach may also be fine (I think yours downloads all themes and plugins doesn't it?). I have to work on the translations in the future though.

ajs124 · 2020-10-02T14:04:56Z

Sure, I'll try and give some useful feedback.
But first, wp4nix works by crawling the WordPress plugin directory API and SVN. It outputs a bunch of JSON files, in total roughly 25MB. These JSON files contain data like plugin/theme/... name, version, svn path and svn revision. They get parsed by the wp4nix default.nix, which exposes a bunch of attrsets, so you can use things like wordpressPackages.plugins.woocommerce.

The reason why we're using SVN is that with the setup they have there you can republish versions and people do. So the zip you download today does not necessarily have the same hash as the one you download tomorrow.
As a user of it, you'd download all the metadata (so 25MB JSON), but you only build and install the plugins and themes you actually use.

As for this PR:

I can't really comment on the Apache2 config changes, because I don't remember when I last configured that software. Although it looks like they're just configuring a virtualhost as opposed to… whatever it's doing now? Just using the default host? What?
The update scripts should probably moved up a folder, because you only need one for all themes and one for all plugins, right? You'd need to pass the name, but you can probably just do that with writeScript or something like that. That and they're "flawed" for the reasons outlined above, but you might get away with this approach here, since you would always substitute from the binary cache, once it's in there, anways.
The database changes seem sensible enough, but they definitely need some kind of migration instructions. I know WordPress doesn't like if you change that kind of thing, so this needs to be tested well.
You might run into issues with setting WP_HOME and WP_SITEURL the way you are doing here, if you ever change them. The reason for this is because WordPress (themes and plugins, idk if core as well) manage to take this and put it into the database. Which is why there are plugins like this.
Does the nixos/tests test still pass without any changes?

That said, this still looks like an overall improvement 👍

mohe2015 · 2020-10-06T17:03:57Z

@ajs124 Thanks a lot for the review.

I'm assuming you are referring to this commit: 39ed207?w=1. As far as I understand nix it only changes the service name from the services.wordpress.$name to the current host name the wordpress instance is reachable under, prepended with "wordpress-". This prevents service name clashes if you want to move another service to the old host name.
Edit: There are some issues with naming: hostName usually actually means siteName (so not changeable) but I didn't want to further complicate this PR with variable name changes.
I decided to remove the plugins and themes completely in favor of your approach.
I added instructions to the theme path change.
I think wordpress core stores the old url in the database but these variables always override the value. But you are right that some/most plugins and themes are incompatible. I removed setting WP_HOME and WP_SITEURL but still added instructions on how to change the hostName as these seem useful.
yes

I added an option to allow a mutable wp-content directory but I don't know if this is wanted. I usually use this for trying out new themes and plugins.

I also added a mention to wp4nix if that is fine with you.

I also added a basic phpmyadmin implementation. Maybe I should put it into another pull request?

Honestly I assume that almost nobody seriously uses this service unmodified because theme updating is currently impossible without loosing settings.

P.S. It would be great if you could add a license to https://git.helsinki.tools/helsinki-systems/wp4nix. Also hosting on github may be beneficial so contributions are possible (If you want to deal with them).
I got a panic when running wp4nix with removed *.json and *.log files for wordpress 5.5: https://gist.github.com/mohe2015/27fe1bf6db92f3ad6a3a72fba8f596c8 If you want to you can look into it.

I would appreciate it if you could do a second round of reviewing.

Also I don't know how the stateVersion thing should be handled as I don't want to break unstable users' systems but it seems like this is the current way to go. I hope they are watching the repository closely. (I would need to change the stateVersion changes to 20.09 if this actually manages to get merged in the current release cycle, right?)

pkgs/servers/web-apps/wordpress/wordpress-core.nix

ajs124 · 2020-10-13T16:52:33Z

As far as I understand nix it only changes the service name from the services.wordpress.$name to the current host name the wordpress instance is reachable under, prepended with "wordpress-". This prevents service name clashes if you want to move another service to the old host name.

Goes to show how much I know about the apache nixos module.

I added an option to allow a mutable wp-content directory but I don't know if this is wanted. I usually use this for trying out new themes and plugins.

AFAIR mutable content is often needed, because that's also where uploaded images and other files are stored.

I also added a basic phpmyadmin implementation. Maybe I should put it into another pull request?

Yeah, that should probably go in a separate PR.

Honestly I assume that almost nobody seriously uses this service unmodified because theme updating is currently impossible without loosing settings.

I kind of doubt anybody uses this, as well.

wp4nix is now licensed under the EUPL-1.2, but given that all code is either by me or @SlothOfAnarchy, we can also re-license it if anyone would prefer to have it under any other license.
Moving it to github is possible but not trivial, as we use gitlab ci and it's integrated into our deployment.

I got a panic when running wp4nix with removed *.json and *.log files for wordpress 5.5: https://gist.github.com/mohe2015/27fe1bf6db92f3ad6a3a72fba8f596c8 If you want to you can look into it.

If you want to regenerate the json files from scratch, you probably need to create empty (aka containing {}) json files, as it barely does any error handling.
Seeing as this was quite literally my first go project, I'm surprised it didn't set your computer on fire ^^
As far as I remember, it also doesn't actually do any version checking with the wordpress version you give it, because… well, because it wasn't necessary for the plugins and themes we use, probably. Maybe it gets used for the languages? We pull those from subversion as well, though.

Also I don't know how the stateVersion thing should be handled as I don't want to break unstable users' systems but it seems like this is the current way to go. I hope they are watching the repository closely. (I would need to change the stateVersion changes to 20.09 if this actually manages to get merged in the current release cycle, right?)

I haven't used stateVersion so far, maybe @aanderse has some insight on that.

aanderse · 2020-10-13T17:02:48Z

You really would think no one uses this module... but I was shocked to learn that a number of people used the httpd submodules options before I deprecated/removed them. It goes to show you really don't know 🤷‍♂️

I only wrote this module to get rid of the httpd submodule options (hint: these options were bad) and don't have any personal interest in maintaining/using this module. I know nginx is much more popular compared to httpd (especially in NixOS) so maybe it makes sense to do a hard break and rewrite the module for nginx the way you would like it, if you're willing to maintain this module in NixOS/nixpkgs... what do you think?

mohe2015 · 2020-10-13T20:49:45Z

@aanderse The changes itself would be backwards compatible if the thing with the stateVersion is discussed.

I'm sorry but I won't rewrite this module. Also the better way would be to get a webserver-agnostic interface but I know it's gonna take a while until that's going to happen.

I would still like to get this merged though. The only issue left is what do do with the stateVersion because I don't want to break the system of unstable users. How should I handle that?

mohe2015 · 2021-08-03T14:15:42Z

I have rebased and dropped some commits that are not necessary in my opinion. I haven't tested again though yet.

The thing that really is necessary is 319e9b2. But the current migration is not a good way to manage this as this could prevent people from upgrading if they don't want to make these changes. I would propose adding some option like services.wordpress.usePluginAndThemeDirectoriesWithoutVersion = true; or so and documenting migration instructions there. Then maybe also adding a note to the release notes that this is recommended. What do you think?

mohe2015 · 2021-08-03T14:30:59Z

@GrahamcOfBorg test wordpress

mohe2015 · 2021-08-16T22:30:58Z

@GrahamcOfBorg test wordpress

mohe2015 · 2021-08-16T22:32:24Z

@onny Would love to get a review from you.

I removed the controversial change for now. I will try this in a separate PR.

Other reviews are also greatly appreciated.

mohe2015 · 2021-08-18T22:26:17Z

nixos/modules/services/databases/mysql.nix

@@ -483,7 +483,7 @@ in
            ''
              ( echo "CREATE USER IF NOT EXISTS '${user.name}'@'localhost' IDENTIFIED WITH ${if isMariaDB then "unix_socket" else "auth_socket"};"
                ${concatStringsSep "\n" (mapAttrsToList (database: permission: ''
-                  echo "GRANT ${permission} ON ${database} TO '${user.name}'@'localhost';"
+                  echo 'GRANT ${permission} ON ${database} TO `${user.name}`@`localhost`;'


I checked (code-wise) that this shouldn't break anything but somebody else should probably also check that. Also I would run some other modules that use this before merging.

mohe2015 · 2021-08-18T22:28:24Z

nixos/modules/services/web-apps/wordpress.nix

@@ -168,7 +191,7 @@ let

          name = mkOption {
            type = types.str;
-            default = "wordpress";
+            default = if lib.versionAtLeast config.system.stateVersion "21.11" then "wordpress-${name}" else "wordpress";


Maybe I should change this to

Suggested change

default = if lib.versionAtLeast config.system.stateVersion "21.11" then "wordpress-${name}" else "wordpress";

default = if lib.versionAtLeast config.system.stateVersion "21.11" then "wordpress-${replaceStrings [ "." ] [ "_" ] name}" else "wordpress";

mohe2015 · 2021-10-13T21:26:47Z

Superseded by #135966. Thank your to everyone who participated.

nixos-discourse · 2021-12-01T17:00:16Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/45

ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package labels Sep 1, 2020

ofborg bot requested a review from basvandijk September 1, 2020 15:10

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Sep 1, 2020

aanderse reviewed Sep 1, 2020

View reviewed changes

nixos/modules/services/web-servers/apache-httpd/default.nix Outdated Show resolved Hide resolved

mohe2015 mentioned this pull request Sep 1, 2020

nixos/httpd: respect hostName when provisioning certificates #96923

Closed

10 tasks

mohe2015 mentioned this pull request Oct 1, 2020

wordpress: 5.4.2 -> 5.5.1 #98302

Merged

10 tasks

mohe2015 force-pushed the wordpress-improvements branch from 1926eb8 to 8222f08 Compare October 2, 2020 12:16

ofborg bot added the 10.rebuild-darwin: 1 label Oct 2, 2020

mohe2015 force-pushed the wordpress-improvements branch from 8222f08 to a7db5db Compare October 6, 2020 17:03

mohe2015 marked this pull request as ready for review October 6, 2020 17:05

ajs124 reviewed Oct 13, 2020

View reviewed changes

pkgs/servers/web-apps/wordpress/wordpress-core.nix Outdated Show resolved Hide resolved

mohe2015 force-pushed the wordpress-improvements branch 5 times, most recently from fe3ef48 to 8a3ce02 Compare October 13, 2020 21:00

SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 18, 2021

mohe2015 mentioned this pull request Feb 2, 2021

nixos/wordpress: nginx support #84446

Merged

10 tasks

mohe2015 mentioned this pull request Apr 13, 2021

Documentation for Wordpress module is misleading #118359

Open

ajs124 mentioned this pull request May 25, 2021

wordpress: packages for plugins and themes #124383

Closed

9 tasks

mohe2015 force-pushed the wordpress-improvements branch from 8a3ce02 to 323e591 Compare June 28, 2021 22:05

mohe2015 requested a review from ryantm as a code owner June 28, 2021 22:05

mohe2015 removed the request for review from ryantm June 28, 2021 22:06

mohe2015 force-pushed the wordpress-improvements branch from 323e591 to 24aba6c Compare June 28, 2021 22:13

mohe2015 force-pushed the wordpress-improvements branch from 24aba6c to b8e7b92 Compare August 3, 2021 14:10

ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 3, 2021

mohe2015 force-pushed the wordpress-improvements branch from b8e7b92 to 0a3cdaa Compare August 3, 2021 14:20

mohe2015 added 6 commits August 17, 2021 00:19

wordpress: Add update script

8ce3255

wordpress-core: init at 5.7.2

5ef625b

nixos/mysql: Fix for databases with special characters

97ed2d3

nixos/wordpress: Use separate database for each wordpress instance

28afe07

nixos/wordpress: Add option to allow mutable wp-content directory

512f643

nixos/wordpress: Mention a theme and plugin packaging tool in the docs.

0e3ae1c

mohe2015 force-pushed the wordpress-improvements branch from 0a3cdaa to 0e3ae1c Compare August 16, 2021 22:29

mohe2015 requested a review from ajs124 August 16, 2021 22:30

mohe2015 requested a review from aanderse August 16, 2021 22:40

mohe2015 commented Aug 18, 2021

View reviewed changes

mohe2015 mentioned this pull request Aug 27, 2021

wordpress: allow updating builtin themes and plugins #135966

Closed

12 tasks

mohe2015 closed this Oct 13, 2021

mohe2015 deleted the wordpress-improvements branch November 28, 2021 20:26

mohe2015 mentioned this pull request Apr 19, 2022

wp4nix: init at 1.0.0 #169183

Merged

13 tasks

Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wordpress improvements #96910

Wordpress improvements #96910

mohe2015 commented Sep 1, 2020 •

edited

Loading

ajs124 commented Sep 2, 2020

mohe2015 commented Oct 2, 2020

ajs124 commented Oct 2, 2020

mohe2015 commented Oct 6, 2020 •

edited

Loading

ajs124 commented Oct 13, 2020

aanderse commented Oct 13, 2020

mohe2015 commented Oct 13, 2020 •

edited

Loading

mohe2015 commented Aug 3, 2021 •

edited

Loading

mohe2015 commented Aug 3, 2021

mohe2015 commented Aug 16, 2021

mohe2015 commented Aug 16, 2021 •

edited

Loading

mohe2015 Aug 18, 2021

mohe2015 Aug 18, 2021

mohe2015 commented Oct 13, 2021

nixos-discourse commented Dec 1, 2021

	default = if lib.versionAtLeast config.system.stateVersion "21.11" then "wordpress-${name}" else "wordpress";
	default = if lib.versionAtLeast config.system.stateVersion "21.11" then "wordpress-${replaceStrings [ "." ] [ "_" ] name}" else "wordpress";

Wordpress improvements #96910

Wordpress improvements #96910

Conversation

mohe2015 commented Sep 1, 2020 • edited Loading

Motivation for this change

Things done

ajs124 commented Sep 2, 2020

mohe2015 commented Oct 2, 2020

ajs124 commented Oct 2, 2020

mohe2015 commented Oct 6, 2020 • edited Loading

ajs124 commented Oct 13, 2020

aanderse commented Oct 13, 2020

mohe2015 commented Oct 13, 2020 • edited Loading

mohe2015 commented Aug 3, 2021 • edited Loading

mohe2015 commented Aug 3, 2021

mohe2015 commented Aug 16, 2021

mohe2015 commented Aug 16, 2021 • edited Loading

mohe2015 Aug 18, 2021

Choose a reason for hiding this comment

mohe2015 Aug 18, 2021

Choose a reason for hiding this comment

mohe2015 commented Oct 13, 2021

nixos-discourse commented Dec 1, 2021

mohe2015 commented Sep 1, 2020 •

edited

Loading

mohe2015 commented Oct 6, 2020 •

edited

Loading

mohe2015 commented Oct 13, 2020 •

edited

Loading

mohe2015 commented Aug 3, 2021 •

edited

Loading

mohe2015 commented Aug 16, 2021 •

edited

Loading