Skip to content

Conversation

@provokateurin
Copy link
Member

@provokateurin provokateurin commented Sep 14, 2025

Replaces #354035

Instead of trying to comes as close as possible to the official release tarballs, I tried to build a minimal working version instead. The official release tarballs apparently contain quite some junk that doesn't belong there (e.g. README.md and vendor-bin for shipped apps which is not useful for admins or end users). I also didn't include the composer.json, composer.lock, package.json and package-lock.json files, as they are not necessary at runtime. I'm actually not sure why they are included, but as long as Nextcloud works correctly I would just skip them. Also there are a bunch of license files I skipped, as it should just be enough to have them in the source and not in the output.

One major difference to the official release tarballs is that all files necessary for the integrity check are not there, since those are signed with a private key and are not reproducible. Given that the integrity check has been disabled in NixOS a few months ago, it shouldn't really matter.

There are probably still some issues and I haven't done any user testing so far, as I wanted to gather feedback on everything first. Adding some missing files in the build process should be a trivial fix.

One downside of this approach is that updating all the hashes takes a lot of time, but I don't think there is a way around that. It just takes a bit of time 🤷‍♀️

TODOs:

  • Add documentation
  • Test that all apps work: Probably impossible to achieve, but it should be tried at least

Potential TODOs:

  • Build and include the updater: It's not used within NixOS, since the updates are done via the nix store, however it might still be useful to come closer to the official release tarballs.
  • Run make latexpdf in the documentation like the official release tarballs: It contains the same user manual, but just looks a bit nicer. I looked into it, but got stuck early on some font issues and decided to postpone it for now.
  • Restructure everything so that it's possible to patch the server (including JS) and shipped apps
  • Probably something I forgot already

I definitely didn't spent way too much time on this and had to fight a lot with npm /s

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@provokateurin provokateurin mentioned this pull request Sep 14, 2025
13 tasks
@onny
Copy link
Contributor

onny commented Sep 15, 2025

Really great work, thank you!

What do you think about moving the derivation to pkgs/by-name/ne/nextcloudXX, build one version from source and just use an override for the other versions?

Instead of building all the Nextcloud apps from source here too, what about putting them into nextcloudApps subpackages first and include the default set into services.nextcloud.extraApps

@Ma27
Copy link
Member

Ma27 commented Sep 15, 2025

What do you think about moving the derivation to pkgs/by-name/ne/nextcloudXX, build one version from source and just use an override for the other versions?

I'm pretty sure that this won't save asset rebuilds? Especially because it's not guaranteed that the sources are the same?

Also, how would moving a single major version to by-name help with this? This really seems like another attempt to move stuff to by-name just for the sake of it (some packages don't fit into that structure and that's fine).

Instead of building all the Nextcloud apps from source here too, what about putting them into nextcloudApps subpackages first and include the default set into services.nextcloud.extraApps

Can you give a rationale for doing that, please?

This seems like needlessly overcomplicating stuff.

@Ma27
Copy link
Member

Ma27 commented Sep 15, 2025

Thanks! I need a few quiet hours for a closer review. While this looks pretty nice and promising, there are a few aspects (like the ad-hoc check if certain dependencies are required) that I'm afraid will cause major maintenance pain on upgrades and I don't have the time & spoons currently to think of suggestion on how to make this a little better.

@onny
Copy link
Contributor

onny commented Sep 16, 2025

Also, how would moving a single major version to by-name help with this? This really seems like another attempt to move stuff to by-name just for the sake of it (some packages don't fit into that structure and that's fine).

Maybe only have the newest Nextcloud version in pkgs/by-name such as any other web app like InvoicePlane, WordPress or DokuWiki and then just overwrite the version tag and hash in pkgs/top-level

Instead of building all the Nextcloud apps from source here too, what about putting them into nextcloudApps subpackages first and include the default set into services.nextcloud.extraApps

Can you give a rationale for doing that, please?
This seems like needlessly overcomplicating stuff.

In my opinion the Nextcloud derivation in this PR is too big and complex for now. As a NixOS newbie I wouldn't knew where to start. So building just Nextcloud core/server and pulling all default required apps from pkgs.nextcloudApps would be a simpler and more straightforward approach

@provokateurin
Copy link
Member Author

Maybe only have the newest Nextcloud version in pkgs/by-name

The problem is that Nextcloud only allows upgrading from one major version to the next without skipping one.
It can happen that two major Nextcloud versions are released within one NixOS release cycle, leaving everyone who upgrades with an annoying upgrade path. They might need to manually update the used package multiple times, so I don't see a benefit in this.

@onny
Copy link
Contributor

onny commented Sep 16, 2025

Maybe only have the newest Nextcloud version in pkgs/by-name

The problem is that Nextcloud only allows upgrading from one major version to the next without skipping one. It can happen that two major Nextcloud versions are released within one NixOS release cycle, leaving everyone who upgrades with an annoying upgrade path. They might need to manually update the used package multiple times, so I don't see a benefit in this.

the nextcloud module can always pin a specific nextcloud version depending on the nixos release version. the nextcloud package in pkgs/by-name could be a kind of "meta-package"

@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 21, 2025
@provokateurin provokateurin marked this pull request as draft September 25, 2025 18:47
@nixpkgs-ci nixpkgs-ci bot added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Sep 25, 2025
@provokateurin
Copy link
Member Author

Some apps ship a .nextcloudignore, it should be used here to remove additional unwanted files.

@provokateurin
Copy link
Member Author

Tried to rebase for 32, but unfortunately there are new apps which have somewhat broken package-lock.json files. I'll try to fix those, but probably not for 32.0.1 due to having too little time right now. I'll also try to fix the root cause of these issues, as otherwise we will probably have the issue over and over with each new patch release (and thus being unable to build without modified package-lock.json files).

@provokateurin
Copy link
Member Author

For the broken package-lock.json files I made nextcloud/.github#620, so it hopefully goes away soon and doesn't need any manual fixes anymore.

Minion3665 added a commit to Minion3665/nixpkgs that referenced this pull request Oct 28, 2025
This builds on work from NixOS#442910, but adds support
for building from specific revisions, which is then used to build
nextcloud33.

Like in that PR, updating hashes is a very painful part of this - and
maybe moreso as there's no script to figure out what versions you need:
you need to manually get the git revisions for all of them and update
the hashes in versions.json... that could be greased with some script
that, say, fetched every "latest" commit at a given time.

If you're trying to update the hashes yourself, I'd suggest running
something like:

  nix-build ./default.nix -A nextcloud33 --keep-going

and picking off the hashes several at a time (rather than either
manually calculating hashes or running many, many builds)

Some further observations:
- The way this is implemented in this commit is a bit of a hack - but
  a surprisingly nice one. It might be worth implementing something
  similar to this upstream (if only because of the documentation not
  being versioned!)
- This feels very not-easy to override - I suspect if you want a custom
  build of nextcloud you really want to be forking nixpkgs and doing
  this sort of process rather than trying to overlay this in your own
  configs
- You can't make the version (dynamic from the name in versions.json)
  anything you like - it has to be accepted by PHP which means things
  like "33.unstable-2025-10-27" aren't acceptable. You want instead to
  use something that roughly looks like semver, I think, so I went with
  33.3665.20251027 (3665 from my username - it's arbitrary, 20251027 is
  today's date)

And some stuff I didn't properly do
- The 'recommendations' and 'files_pdfviewer' apps which had various
  errors each:
  - at least some of that was package-lock.json issues, which might be
    fixed soon judging by talk in the original PR... other bits aren't
- I didn't do the documentation - since as I'm not going to be using it
@provokateurin
Copy link
Member Author

provokateurin commented Oct 29, 2025

So I did a bunch of changes, including a script to generate all the hashes automatically and removing all IFD. Currently nextcloud31 builds as-is, nextcloud32 unfortunately doesn't due to a bunch of problems with package-lock.json.
I think for reviewing it's enough to have nextcloud31 building correctly for now, but of course nextcloud32 must be buildable as well, before merging this.

@nixpkgs-ci nixpkgs-ci bot added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 29, 2025
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would probably be a good idea to extract all the python dependencies and merge them already in another PR to reduce the scope of this PR.

@SuperSandro2000
Copy link
Member

I'm not sure why the eval is failing in CI, locally it's fine. Is this because of the IFD for catching missing hashes? I think I can remove it at this point, because the generate script will do everything correctly and the throws were only there to catch human mistakes.

Yep, sounds plausible to me.

@provokateurin
Copy link
Member Author

I think something that is still missing is compiling and copying core/css from server.

@provokateurin
Copy link
Member Author

@SuperSandro2000 See #463822 for the sphinx packages.

@provokateurin provokateurin force-pushed the nextcloud-src branch 2 times, most recently from 8b02589 to 8c69f30 Compare November 22, 2025 09:27
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 6.topic: python Python is a high-level, general-purpose programming language. labels Nov 22, 2025
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 6.topic: python Python is a high-level, general-purpose programming language. labels Nov 24, 2025
@SuperSandro2000
Copy link
Member

Now we got all the python things out of the way and can focus on Nextcloud things.

@provokateurin
Copy link
Member Author

Just noticed building the user manual PDF fails and an empty file is generated, so it still passes.

@provokateurin
Copy link
Member Author

No more patches 🎉

I still think there are a few thinks to fix, but a general review should be possible now.
Is there anything I could do to make it easier for the reviewers?
@Ma27 @onny maybe it would be possible to do a guided review IRL since you are also based in Karlsruhe AFAIK? Maybe something for next year though ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants