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

crater: init at 4.2.0 #141822

Closed
wants to merge 4 commits into from
Closed

crater: init at 4.2.0 #141822

wants to merge 4 commits into from

Conversation

DieracDelta
Copy link
Member

Motivation for this change

Note: opening this on behalf of @MatthewCroughan

This adds a NixOS module and package for Crater, an invoicing app. Now Nix (ab)users can finally get paid!

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages 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/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 15, 2021
Comment on lines 39 to 57
meta = with lib; {
# description = "";
# license = ;
# homepage = "";
# maintainers = with maintainers; [ ];
platforms = platforms.all;
};
Copy link
Member

@fabianhjr fabianhjr Oct 16, 2021

Choose a reason for hiding this comment

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

Should add this information. From a quick glance seems like:

@MatthewCroughan MatthewCroughan force-pushed the crater branch 3 times, most recently from 6a7f998 to 3d7ce76 Compare October 17, 2021 17:13
@fabianhjr
Copy link
Member

AAL seems to be missing from https://github.com/NixOS/nixpkgs/blob/master/lib/licenses.nix, it should be added there.

@github-actions github-actions bot removed 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS labels Oct 19, 2021
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 19, 2021
@MatthewCroughan MatthewCroughan force-pushed the crater branch 2 times, most recently from 10889b7 to 345a99f Compare October 20, 2021 15:00
lib/licenses.nix Outdated Show resolved Hide resolved
MatthewCroughan and others added 4 commits October 23, 2021 01:57
Co-authored-by: Techjar <tecknojar@gmail.com>
Co-authored-by: Aaron Andersen <aaron@fosslib.net>
Co-authored-by: Techjar <tecknojar@gmail.com>
Co-authored-by: Aaron Andersen <aaron@fosslib.net>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
@MatthewCroughan
Copy link
Contributor

There is an issue with Crater since it stores the path to laravel "storage" in the database.

[nix-shell:~]# strings /var/lib/crater/database.sqlite | grep nix
33local_privateSYSTEMlocal"{\"root\":\"\\\/nix\\\/store\\\/64l9q7imbhhvvc4x07n1qmrdvwbwwimq-crater-4.2.0\\\/storage\\\/app\",\"driver\":\"local\"}"2021-10-23 01:53:532021-10-23 01:53:53
'33local_publicSYSTEMlocal"{\"driver\":\"local\",\"root\":\"\\\/nix\\\/store\\\/64l9q7imbhhvvc4x07n1qmrdvwbwwimq-crater-4.2.0\\\/storage\\\/app\\\/public\",\"url\":\"http:\\\/\\\/crater.test\\\/storage\",\"visibility\":\"public\"}"2021-10-23 01:53:532021-10-23 01:53:53

This path resolves to /var/lib/storage, but sadly it uses the absolute path to the Nix store. I think this is because the PHP function that creates this path is not following symlinks.

This means that changes to the dataDir option of the module are not going to work upon a rebuild.

Despite this, I think the module is in a good working state, and it is as good as it is going to get without radically changing the way the program works.

Is it possible that @aanderse could review this?

@aanderse
Copy link
Member

gitea used to do something similar 😟

Very unfortunately, you should probably use the sqlite binary to update this value in a script that runs on rebuilds... think about what happens the next time you run the garbage collector 🤯 I'm glad you caught this!

@MatthewCroughan
Copy link
Contributor

@aanderse The user can use mysql, in that case this is unfixable. They could be using any database software they want. We can only fix this for sqlite.

@MatthewCroughan
Copy link
Contributor

MatthewCroughan commented Dec 6, 2021

As a workaround for the database issue, I currently use the following. It's a script that:

  1. Waits until database.sqlite is non-zero AND file_disks exists.
  2. Pipes the json string from file_disks into jq
  3. Creates a new json string with the incorrect path replaced with the dataDir:
  4. Uses sqlite3 to UPDATE this value in the database
  systemd.services.phpfpm-crater.postStart = ''
    until [[ -s ${config.services.crater.dataDir}/database.sqlite && $(${pkgs.sqlite}/bin/sqlite3 ${config.services.crater.dataDir}/database.sqlite "select credentials from file_disks;") ]] 
    do
      sleep 1
      echo "DATABASE IS BLOODY EMPTY"
    done
    STORAGE_PUBLIC=`${pkgs.sqlite}/bin/sqlite3 ${config.services.crater.dataDir}/database.sqlite "select credentials from file_disks where id=1;" | ${pkgs.jq}/bin/jq -r | ${pkgs.jq}/bin/jq '.root = "${config.services.crater.dataDir}/storage/app/public"' | ${pkgs.jq}/bin/jq -c`
    STORAGE=`${pkgs.sqlite}/bin/sqlite3 ${config.services.crater.dataDir}/database.sqlite "select credentials from file_disks where id=2;" | ${pkgs.jq}/bin/jq -r | ${pkgs.jq}/bin/jq '.root = "${config.services.crater.dataDir}/storage/app"' | ${pkgs.jq}/bin/jq -c`
  
    ${pkgs.sqlite}/bin/sqlite3 ${config.services.crater.dataDir}/database.sqlite "UPDATE file_disks SET credentials = '$STORAGE_PUBLIC' where id=1;"
    ${pkgs.sqlite}/bin/sqlite3 ${config.services.crater.dataDir}/database.sqlite "UPDATE file_disks SET credentials = '$STORAGE' where id=2;"
  '';

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 12, 2022
@onny

This comment was marked as off-topic.

@aanderse
Copy link
Member

aanderse commented Nov 7, 2022

I should probably take a look at this again. I've actually been using crater for some time now... but imperatively installed under something like /var/www/crater. Definitely a more challenging application to "properly" package for NixOS.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Nov 7, 2022
@MatthewCroughan
Copy link
Contributor

@aanderse I would avoid Crater. It is not the correct way to manage your taxes, or expenses. It will only lead you to accounting troubles, since it is not modelling accounting correctly. Instead, I started using Hledger which correctly models accounting. In addition to Crater making many mistakes regarding accounting, it also has many software engineering mistakes such as the ones we've already discussed and ran into in this thread. Hledger has a web interface that isn't as pretty, that is available in NixOS already via services.hledger-web.

@onny
Copy link
Contributor

onny commented Nov 9, 2022

Trying to update it to the latest version and testing it #200379
@DieracDelta feel free to these incorporate changes into your PR

@MatthewCroughan
Copy link
Contributor

@onny There are bound to be a lot of runtime bugs that you cannot count on discovering, without using the software. It gets worse with each release.

@onny
Copy link
Contributor

onny commented Nov 9, 2022

That sounds bad. I'll give it a try and see how it does

@MatthewCroughan
Copy link
Contributor

Just to be clear, I'm talking about bugs unique to Nix because of the read-only store. And we still do not have a solution to the problem outlined in #141822 (comment) where there are illegal references to the /nix/store being stored in the database. Which is a choice of the program that we can't override without patching the source code. It seems like a lot of time to spend on this (in my view) bad software.

@aanderse
Copy link
Member

aanderse commented Nov 9, 2022

@aanderse I would avoid Crater. It is not the correct way to manage your taxes, or expenses.

Thanks for the advice @MatthewCroughan. Fortunately I only use a minimal feature set and don't rely on it for anything which is impacted by what you said. I'll keep it that way now too. 🙇‍♂️

@onny if you're still set on pushing through with this software I'll try to give this some thought. Let me know.

@onny onny mentioned this pull request Nov 10, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants