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

qr-backup: init at 1.1.4 #392426

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

acuteaangle
Copy link
Contributor

@acuteaangle acuteaangle commented Mar 23, 2025

This adds qr-backup, a utility to create paper backups of files using QR codes.

Homepage: https://github.com/za3k/qr-backup
License: CC0-1.0

Currently blocked by upstream issues: Resolved

Closes: gh-343799 ("Package request: qr-backup")

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

Add a 👍 reaction to pull requests you find important.

@acuteaangle acuteaangle changed the title [DRAFT] qr-backup: init at 1.1-unstable-2024-12-08 [DRAFT] qr-backup: init at 1.1.4 Mar 25, 2025
@acuteaangle
Copy link
Contributor Author

@za3k Any idea what might cause every other test to fail consistently?

Output
[...]
correct-restore --instructions both 4s
backup-regression --instructions cover 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --instructions cover
  result: 792e4a1704c3d36edc8ce2b96d9e17003a2b23feafb612a5ebd1c68ebbf61a1f != 4757a096d77b5c1cc0774f69577d1365d35d3113115901bbddf63d61411c3ed9
correct-restore --instructions cover 4s
backup-regression --instructions none 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --instructions none
  result: c952c9fb7fc3fd4876ab9dc4e67848867e94fcb3ebe57942039b4d2bf00fb8bc != 361e065a0addbe02a11385a8bb9510940e6d09cae6f0482223ca2f08013a1d00
correct-restore --instructions none 3s
backup-regression --note (1) 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --note A
  result: fed91611f9dac0d50faa33b0fec76d517f876a0618a08a2b9e58bf04e3a7345c != 49191147dbbe7d9a411567b54f74f86b235f387c8c4a28fe68dbcb86dc310fca
correct-restore --note (1) 3s
backup-regression --note (2) 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --note A --note B
  result: 80d1bc7418493dced58e19523698ad8d98a18f48b3bbbdea3618c1de35c02f36 != 1dee193b34d777aa2a20097e3e800f2b4206d73e908168a338baa8011f9b23d6
correct-restore --note (2) 3s
backup-regression --num-copies 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --num-copies 3
  result: 64918cb781e99364bbb2c8b510bb0649137f41468d093517d6ceb1cb987674a6 != ba5f2936cfe981ffb5fa074f0893f084747527b00b313f0d86a29260b4f02f07
correct-restore --num-copies 3s
backup-regression --page 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --page 100 200
  result: b351bac4ac779fa1f29acd0e7f2ba1ab5c34732ff55ef16e2dded67f635b3fa5 != 551cfb9c092bd3c837f0cbf7fb14c7321dfcf5f0d7cb9d12a79e363004be4ce4
correct-restore --page 1s
backup-regression --qr-version 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --qr-version 15
  result: 2c779ae1d17f86e8eca3a588c752fcfdc015b158656e2229df15e2401090cb7d != eebf76bd76f62d6d0f097417ae8fd95cb10963c5dc71e68d88740420899a3325
correct-restore --qr-version 3s
backup-regression --scale 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --scale 10
  result: ffb1de2af1d6029f1406725639d473ac0b9bd6fb25dd543bf477d22573aeadf7 != 7c402f001634271bdd04e58b7ebd5bbb71b3d249bc378efa4f5f57bb6d26d8de
correct-restore --scale 3s
backup-regression --no-shuffle 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --no-shuffle
  result: 8bdb1e5613fadb295233bea746acdc81cf01dab489eb738ecf1c75cabe8836db != 8ce2b34dc9a57a3fb521bd59ba1f0ec1a7d51f8b67c54fc7ad21eec87454fe0f
correct-restore --no-shuffle 3s
backup-regression 1K zeros 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22
  result: ff17a9733e63fb225df81965d8baa62c3412636a20809baa677c105bb24addc1 != c952b0a40d1fa655b367b274672e0a9b1d56fc034fc690230a0ad842bc4eef53
correct-restore 1K zeros 3s
backup-regression 10K zeros 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22
  result: 720cdb63154857fdd39a6b008c88f6ff329ef42dd9eea026bcf95769e4581432 != 1889f9ad99d9d7df224e3e65283d14602a62b14253bb1adff3c99ae38bc9734b
correct-restore 10K zeros 3s
backup-regression 100K zeros 1s
  command: /nix/store/riyz1x9z6mxdrkf3qxdbg3x9gha271k3-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22
  result: 475a13f5f0b2cf4f7ab9b2bbabbd0f8d2a93802da26d191237594117e2da1dbb != 1ddfe806cd82e74c5d5f025aae2ce9f38577c9d9d2347337a75fe274e9477e2b
[...]

@acuteaangle
Copy link
Contributor Author

acuteaangle commented Mar 25, 2025

Seems like it’s all but one regression test and a couple non-regression tests.

correct-restore 100b zeros 3s
backup-regression default options 1s
  command: /nix/store/7sp2q6gq4px1z8ndabybvac3qjy2rcqx-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --dpi 300 --compress --erasure-coding --filename file --instructions page --num-copies 1 --output - --page 500 600 --qr-version 10 --scale 5 --shuffle
  result: e08ce722ad6b6d0079a07672e21e1c25b4fae1c981d2f139003d3de7ffab51f2 != 4d2cd56bc5a890f2a87419ceec58d29cf9b576b14020ebbdabdd75597827add7
correct-restore default options 3s
backup-no-regression --no-compress 1s
correct-restore --no-compress 3s
backup-regression --dpi 1s
  command: /nix/store/7sp2q6gq4px1z8ndabybvac3qjy2rcqx-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --dpi 50
  result: 0a1be4e2a2d147701509c0f43f407b9ddeb0cdf95d262fa8ee76713bcb7112d3 != 22e15a09b519d543c209286636c6faf52c5fc800bab611487d4a47a2a477160b
correct-restore --dpi 4s
failing-command --encrypt 1s
  CRITICAL: Conversion from PDF to TIF failed: -

failing-command --encrypt-print-passphrase 1s
  CRITICAL: Conversion from PDF to TIF failed: -

Time to manually inspect how the regression test is failing :/

@acuteaangle
Copy link
Contributor Author

acuteaangle commented Mar 25, 2025

Well, one of the QR codes is obviously different.
regression.pdf
regression2.pdf
In this case, looking nearly—but not actually—identical to the first one on the same page. I wonder if it’s using a different mask for some reason. Need to check if the data is actually different.

@za3k
Copy link

za3k commented Mar 25, 2025

To be honest, I've never quite understood the idea that tests should run on every machine that installs something, rather than only for developers or as a troubleshooting step. I have added a qr-backup issue to have a version of tests that is expected to pass more reliably, for packagers who feel it is necessary to to do this.

There is a guide I wrote newly as of yesterday which I believe should be printing in the output of the tests, explaining how to debug exactly this problem. If it's not, I'm confused why not. Can you paste the output of the first two sets of tests (100b zeros, default options)?

The error is that yes, the contents of the two QR codes are slightly different. This is fixed in master already (in za3k/qr-backup@8d40ed3)

If you would like to work around the issue in v1.1.4 (the next release will include this fix), you can switch qr-backup's shebang line to run with Python 3.10 or later -- the problem is that different Python versions give (slightly) different gzip output.

Edit: Alternatively, don't run the tests. They take something like 5 minutes to run, so I'd rather not subject everyone who installs the software to that.

@acuteaangle
Copy link
Contributor Author

acuteaangle commented Mar 25, 2025

To be honest, I've never quite understood the idea that tests should run on every machine that installs something, rather than only for developers or as a troubleshooting step.

Running tests on install is fairly uncommon in NixOS, but running tests after building from source is preferred.
Being a standalone Python script blurs those lines a bit.

I don’t believe the tests would run when a typical user installs the package, assuming they haven’t configured their system to build everything from source.
The tests would only run on the system that actually builds the package (fetch the source, apply any Nix-specific patches, move things to the correct directories).

It would be trivial to not run the tests, but it makes it a lot easier to approve package updates for Nixpkgs if there was testing in CI on the Nicole’s side as well.
(There is a bot which can automatically create an update PR and ping package maintainers after an upstream release; having CI on that (and being able to make use of the tests locally on NixOS) makes it way easier to catch NixOS-specific issues in cases you might not try manually.)

There is a guide I wrote newly as of yesterday which I believe should be printing in the output of the tests, explaining how to debug exactly this problem. If it's not, I'm confused why not.

That guide did print; I cut it for brevity; it is very useful and I followed it until realizing the regression tests are far too fragile for Nixpkgs—however, once I stopped focusing on getting it working, I realized they’re not actually useful or necessary here.

The regression tests are only useful upstream; they say very little about the correctness of the Nix package.
I think it makes the most sense to disable the regression tests on the Nix side, and only keep the roundtrip tests (as checking that the program still runs and produces sensible output on Nix after an update is important)

The two failing --encrypt* tests do seem to be a real problem in the current revision of the Nix package; I’ll need to figure out why that’s happening.

@acuteaangle
Copy link
Contributor Author

Can you paste the output of the first two sets of tests (100b zeros, default options)?

This is what was included in my previous log posted here; are you referring to something different?

correct-restore 100b zeros 3s
backup-regression default options 1s
  command: /nix/store/7sp2q6gq4px1z8ndabybvac3qjy2rcqx-qr-backup-1.1.4/bin/qr-backup --skip-checks - --backup-date 2022-09-22 --dpi 300 --compress --erasure-coding --filename file --instructions page --num-copies 1 --output - --page 500 600 --qr-version 10 --scale 5 --shuffle
  result: e08ce722ad6b6d0079a07672e21e1c25b4fae1c981d2f139003d3de7ffab51f2 != 4d2cd56bc5a890f2a87419ceec58d29cf9b576b14020ebbdabdd75597827add7

@za3k
Copy link

za3k commented Mar 26, 2025

Can you paste the output of the first two sets of tests (100b zeros, default options)?

This is what was included in my previous log posted here; are you referring to something different?

You can ignore that. I was just trying to figure out why debug instructions weren't being printed (they were).

I have added a --fast option to the test runner, which skips reproducibility tests (which I renamed from the less clear "regression" tests). See za3k/qr-backup@e7d028e. No release is planned immediately -- my advice to use Python 3.10+ for tests/nix stands.

@acuteaangle
Copy link
Contributor Author

acuteaangle commented Mar 26, 2025

I have added a --fast option to the test runner, which skips reproducibility tests (which I renamed from the less clear "regression" tests).

Ah, that would be less janky than my current solution.

With the hacky patch, I’m still getting a couple errors on the encrypt tests. I’m not sure why the conversion is failing yet.

[...]
correct-restore 100b zeros 3s
correct-restore default options 3s
correct-restore --no-compress 3s
correct-restore --dpi 4s
failing-command --encrypt 1s
  CRITICAL: Conversion from PDF to TIF failed: -

failing-command --encrypt-print-passphrase 1s
  CRITICAL: Conversion from PDF to TIF failed: -

correct-restore --no-erasure-coding 3s
correct-restore --error-correction L 3s
correct-restore --error-correction Q 3s
correct-restore --error-correction H 3s
correct-restore --filename 3s
correct-restore --instructions both 4s
correct-restore --instructions cover 4s
correct-restore --instructions none 3s
correct-restore --note (1) 3s
correct-restore --note (2) 3s
correct-restore --num-copies 3s
correct-restore --page 1s
correct-restore --qr-version 3s
correct-restore --scale 3s
correct-restore --no-shuffle 3s
correct-restore 1K zeros 3s
correct-restore 10K zeros 3s
correct-restore 100K zeros 3s
correct-restore 100b random 3s
correct-restore 1K random 3s
too-fast 10K random 2s, <2^1
correct-restore 10K random 8s
too-fast 50K random 8s, <2^3
correct-restore 50K random 38s
too-fast 50K random 38s, <2^6
correct-restore 1K zeros, self-check 3s
correct-restore 1K random, self-check 3s
2 failures
[...]

@acuteaangle
Copy link
Contributor Author

No release is planned immediately

No problem :) Little point pushing a release just for a feature in tests

@za3k
Copy link

za3k commented Mar 26, 2025

#ghostscript # FIXME - listed upstream but don't know what needs it yet

ghostscript is needed by 'convert' and 'zbarimg' to read PDFs.
In turn, that's needed by qr-backup for restore, or for backups with --no-skip-checks (the default)

The failing line is not printing the exact command it's running (will fix) but the commands the test actually runs are:

python3 qr-backup --skip-checks - --backup-date 2022-09-22 --encrypt PASSWORD
python3 qr-backup --restore - --encrypt PASSWORD

You'll want to replace the bare - in each with an actual file for testing.

@acuteaangle
Copy link
Contributor Author

acuteaangle commented Mar 26, 2025

#ghostscript # FIXME - listed upstream but don't know what needs it yet

ghostscript is needed by 'convert' and 'zbarimg' to read PDFs.
In turn, that's needed by qr-backup for restore, or for backups with --no-skip-checks (the default)

Hmm, if it’s purely a transitive dependency, it shouldn’t need to be listed in the qr-backup package for Nixpkgs.

It seems like imagemagick in Nixpkgs has an optional ghostscript dependency, which defaults to false.

zbar then depends on imagemagickBig, but not on ghostscript directly.

I haven’t found where imagemagickBig is defined yet—it’s possible that variant comes with ghostscript enabled.

Update: found it; turns out imagemagickBig is exactly imagemagick with ghostscript enabled.
So we shouldn’t need ghostscript as I don’t believe we’re calling it directly anywhere.

@acuteaangle
Copy link
Contributor Author

acuteaangle commented Mar 26, 2025

The failing line is not printing the exact command it's running (will fix) but the commands the test actually runs are:

python3 qr-backup --skip-checks - --backup-date 2022-09-22 --encrypt PASSWORD
python3 qr-backup --restore - --encrypt PASSWORD

Those both work flawlessly when run manually—I’m guessing a dependency must be missing from the test environment for some reason? Weird. Welp; can hopefully just binary search that.

Nope.
I threw in breakpointHook. Maybe I can strace this from the test env.

@acuteaangle
Copy link
Contributor Author

acuteaangle commented Mar 26, 2025

bash-5.2# /nix/store/z04pv5kb84a3ppb86gx6nq5ighjl76fh-qr-backup-1.1.4/bin/qr-backup --skip-checks <<<'test' --backup-date 2022-09-22 --encrypt PASSWORD
CRITICAL: [gpg stdout] b''
CRITICAL: [gpg stderr] b"gpg: Fatal: can't create directory '/homeless-shelter/.gnupg': No such file or directory\n"
CRITICAL: [gpg exit] 2
bash-5.2#

sigh. That’ll do it.

gpg fails during testing because $HOME is set to a nonexistent directory during package builds.


Annnnnd, through the magic of export GNUPGHOME="$(mktemp -d)"

0 failures

@za3k
Copy link

za3k commented Mar 26, 2025

#ghostscript # FIXME - listed upstream but don't know what needs it yet

ghostscript is needed by 'convert' and 'zbarimg' to read PDFs.
In turn, that's needed by qr-backup for restore, or for backups with --no-skip-checks (the default)

Hmm, if it’s purely a transitive dependency, it shouldn’t need to be listed in the qr-backup package for Nixpkgs.

Yeah, but it's a transitive optional dependency, which is non-optional for qr-backup, so it should be listed.

imagemagickBig is exactly what we'd want, sounds perfect.

@acuteaangle
Copy link
Contributor Author

imagemagickBig is exactly what we'd want, sounds perfect.

Already listed :)
zbar depends on the Big variant, so we’re already pulling it in transitively.
So there’s no waste in using the Big variant for the direct dependency as well.

@acuteaangle
Copy link
Contributor Author

versionCheckHook wasn’t working properly because I was calling the (pre|post)Check instead of (pre|post)InstallCheck hooks in installCheckPhase.

versionCheckHook is a sanity check that makes sure the intended package version is found in the output of --version. Otherwise, if you forget to update a hash, Nix may silently keep using an old version, assuming it already has the source code with that hash and doesn’t need to fetch it again.

Fixed now

@acuteaangle acuteaangle changed the title [DRAFT] qr-backup: init at 1.1.4 qr-backup: init at 1.1.4 Mar 26, 2025
@acuteaangle acuteaangle marked this pull request as ready for review March 26, 2025 02:09
@za3k
Copy link

za3k commented Mar 26, 2025

In qr-backup, someone kindly donated https://github.com/za3k/qr-backup/blob/master/installers/flake.nix and the lock. Can those be safely deleted now?

@za3k
Copy link

za3k commented Mar 26, 2025

Thank you kindly for all your hard work on this!

@acuteaangle
Copy link
Contributor Author

In qr-backup, someone kindly donated https://github.com/za3k/qr-backup/blob/master/installers/flake.nix and the lock. Can those be safely deleted now?

I recommend dropping it once this PR is merged—this PR is unaffected by that, and I don’t know of any reason to keep maintaining the flake upstream.

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.

Package request: qr-backup
2 participants