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

engelsystem: init at 3.1.0 #82753

Merged
merged 4 commits into from May 24, 2020
Merged

engelsystem: init at 3.1.0 #82753

merged 4 commits into from May 24, 2020

Conversation

Kloenk
Copy link
Member

@Kloenk Kloenk commented Mar 16, 2020

Motivation for this change

provide engelsystem as a module, so it can be easily hosted via nixos, without the use of docker

Things done

init engelsystem.
I'm still unsure of the way of handling the storage directory and the config file

  • 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.

maintainers/maintainer-list.nix Outdated Show resolved Hide resolved
pkgs/servers/web-apps/engelsystem/default.nix Outdated Show resolved Hide resolved
pkgs/servers/web-apps/engelsystem/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/servers/web-apps/engelsystem/default.nix Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Mar 16, 2020

It does not seem like this adds a NixOS module. I am unsure how useful this package is without a module.

@Kloenk
Copy link
Member Author

Kloenk commented Mar 16, 2020

will make it to an WIP, and add the module for it

@alyssais
Copy link
Member

We can merge the package first, without a module. It can still be useful to people who write their own configuration (non-reusable modules), or even to people who don’t run NixOS.

@alyssais
Copy link
Member

I'm still unsure of the way of handling the storage directory and the config file

Don’t do either in the package — the NixOS module should handle those.

@Kloenk
Copy link
Member Author

Kloenk commented Mar 17, 2020

I'm still unsure of the way of handling the storage directory and the config file

Don’t do either in the package — the NixOS module should handle those.

What I ment are both of the symlinks one pointing to etc, and one pointing to /var/lib

@Mic92
Copy link
Member

Mic92 commented Mar 17, 2020

I'm still unsure of the way of handling the storage directory and the config file

Don’t do either in the package — the NixOS module should handle those.

What I ment are both of the symlinks one pointing to etc, and one pointing to /var/lib

The symlinks in the derivation look fine to me.

@Kloenk Kloenk requested review from alyssais and a user March 17, 2020 10:37
@alyssais
Copy link
Member

What I ment are both of the symlinks one pointing to etc, and one pointing to /var/lib

The symlinks in the derivation look fine to me.

@Mic92 look at the code. There are two symlinks that are basically no-ops AFAICT because they link to the build directory in the installPhase.

@Kloenk
Copy link
Member Author

Kloenk commented Mar 17, 2020

@alyssais which two? I only have three symlinks, one to /var/lib/engelsystem one to /etc/engelsystem/config.php and one to the migrate script inside the package. all three are working on my side

@Mic92
Copy link
Member

Mic92 commented Mar 17, 2020

@Mic92 look at the code. There are two symlinks that are basically no-ops AFAICT because they link to the build directory in the installPhase.

Those symlinks are than copied to $out with the rest of the source later, so this is correct.

@Kloenk
Copy link
Member Author

Kloenk commented Mar 17, 2020

I'm just implementing the module for it, becaus I probably have to change the migrate script for that. The proplem I have is how to escape '' in a '' ... '' block?

I found this solution, but it seems ugly: '${""}'

@Kloenk
Copy link
Member Author

Kloenk commented Apr 9, 2020

Works in my prod env, without any user setup, besides changing the admin pass via the web interface

nixos/modules/services/web-apps/engelsystem.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/engelsystem.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/engelsystem.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/engelsystem.nix Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/engelsystem.nix Outdated Show resolved Hide resolved
@Kloenk Kloenk force-pushed the feature/engelsystem branch 2 times, most recently from ca969c2 to 2a7566b Compare May 7, 2020 11:01
@talyz
Copy link
Contributor

talyz commented May 24, 2020

@GrahamcOfBorg test engelsystem

@talyz
Copy link
Contributor

talyz commented May 24, 2020

@Kloenk I fixed some small issues and added a test. If you want to squash the fixes into your commit, I'm fine with that - I just didn't want to get rid of your gpg signature. The test should probably stay in its own commit, though. :)

@Kloenk
Copy link
Member Author

Kloenk commented May 24, 2020

@talyz I like the changes. Should I squash them to make git log cleaner, or is that not needed?

@talyz
Copy link
Contributor

talyz commented May 24, 2020

Yeah, squash the fixes, but not the test.

@Kloenk
Copy link
Member Author

Kloenk commented May 24, 2020

Squashed. Can someone write ofborg to restest it?

@talyz
Copy link
Contributor

talyz commented May 24, 2020

@GrahamcOfBorg test engelsystem

@talyz
Copy link
Contributor

talyz commented May 24, 2020

Great work! Thanks! 👍

@talyz talyz merged commit 825e20f into NixOS:master May 24, 2020
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

6 participants