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

rainloop: 1.13.0 -> 1.14.0 #80910

Merged
merged 1 commit into from Apr 12, 2020
Merged

rainloop: 1.13.0 -> 1.14.0 #80910

merged 1 commit into from Apr 12, 2020

Conversation

haskelious
Copy link
Contributor

  • update version
  • change mechanism to set the data folder outside the nix store
Motivation for this change

Update rainloop to latest available version, and also fix a problem on NixOS where the application would not work by simply creating a symlink for the <derivation>/data folder, as rainloop will complain the data directory is not writable.

The better way to set a data folder outside the rainloop folder is by creating a short include.php that is used as an override to data location (reference: https://github.com/RainLoop/rainloop-webmail/blob/master/_include.php).

The new location for the data folder will have to be created separately (e.g. through a module in NixOS or otherwise) with write access to the user under which the PHP scripts are executed.

Note that on NixOS, setting the rainloop data path under /etc will also cause errors, hence the change of the default to /var/lib/rainloop

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

installPhase = ''
mkdir $out
cp -r rainloop/* $out
rm -rf $out/data
ln -s ${dataPath} $out/data
echo -n "$includeScript" > $out/include.php
Copy link
Member

Choose a reason for hiding this comment

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

Probably better to pkgs.writeText the include script and copy it here. This way, we don't have to think about shell escaping.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dasJ, I changed the derivation according to your advice. I used cp to copy the file. ln -s was also an option but not sure if it was best practice.

- update version
- change mechanism to set the data folder outside the nix store
@ofborg ofborg bot requested a review from dasJ March 2, 2020 21:06
@haskelious
Copy link
Contributor Author

What is missing for this to be merged ?

@dasJ
Copy link
Member

dasJ commented Apr 12, 2020

My merge permissions 👀

@dasJ
Copy link
Member

dasJ commented Apr 12, 2020

cc @Ma27 can you merge this? :)

@Ma27 Ma27 merged commit 6a1a6b9 into NixOS:master Apr 12, 2020
@Ma27
Copy link
Member

Ma27 commented Apr 12, 2020

briefly tested locally, LGTM 👍

@dasJ
Copy link
Member

dasJ commented Apr 12, 2020

Thanks 👍

@haskelious haskelious deleted the fix/rainloop-update branch April 13, 2020 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants