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

nixos/restic: ensure newline between backup.paths and outputs of dynamicFilesFrom #319807

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

Conversation

hellodword
Copy link
Contributor

@hellodword hellodword commented Jun 14, 2024

Description of changes

  1. Add --latest=1 for restic snapshots to minimize the output or reduce the requests (in extreme cases)
  2. Add a newline after the result of concatStringsSep "\n" backup.paths to prevent it from being incorrectly concatenated with the output of the subsequent command
:b pkgs.writeScript "hello" ''
    filesFromTmpFile="$(mktemp)"
    cat ${pkgs.writeText "staticPaths" (lib.concatStringsSep "\n" ["/path/to/file1" "/path/to/file2"])} >> "$filesFromTmpFile"
    ${pkgs.writeScript "dynamicFilesFromScript" "echo /path/to/file3"} >> "$filesFromTmpFile"
    cat "$filesFromTmpFile"
''

The result is:

/path/to/file1
/path/to/file2/path/to/file3

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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

@hellodword hellodword force-pushed the nixos-restic-add-unlock branch 2 times, most recently from 90d18b9 to 634d1ac Compare June 22, 2024 03:13
@hellodword hellodword changed the title nixos/restic: Add runUnlock option nixos/restic: ensure newline between backup.paths and outputs of dynamicFilesFrom Jun 22, 2024
@robryk
Copy link
Contributor

robryk commented Jun 22, 2024

Please amend the test in nixos/tests/restic.nix so that it checks that the backup produced by remote-from-file-backup has correct contents: we actually have a test that exercises paths and dynamicFilesFrom at the same time, but that one didn't check backup's contents.

Other than that, this change seems fine.

@hellodword
Copy link
Contributor Author

Please amend the test in nixos/tests/restic.nix so that it checks that the backup produced by remote-from-file-backup has correct contents: we actually have a test that exercises paths and dynamicFilesFrom at the same time, but that one didn't check backup's contents.

Other than that, this change seems fine.

Thanks! Added a check for it

nixos/tests/restic.nix Outdated Show resolved Hide resolved
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

2 participants