Navigation Menu

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

Jump to bottom

Resilio sync configuration fix and NixOS test #62948

Closed
wants to merge 2 commits into from
Closed

Resilio sync configuration fix and NixOS test #62948

wants to merge 2 commits into from

Conversation

danieldk
Copy link
Contributor

Motivation for this change

This change bundles two commits:

  • The first commit fixes an error in the configuration when directoryRoot is used. The option was placed in the wrong section for current versions of Resilio Sync.
  • The second commit adds tests for Resilio Sync. This test would have caught the configuration error solved in the first commit.
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@danieldk danieldk changed the title Resilio config fix Resilio sync configuration fix and NixOS test Jun 10, 2019
@danieldk
Copy link
Contributor Author

Since Resilio Sync is unfree, I was not sure if a test could be added. If not, I will remove the test from this PR.

@danieldk
Copy link
Contributor Author

@GrahamcOfBorg test resilio

@danieldk
Copy link
Contributor Author

@GrahamcOfBorg test resilio

@danieldk
Copy link
Contributor Author

If tests for non-free packages are not possible, I can do a new PR containing just the bug fix for the Resilio Sync module.

@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/23

@yesbox
Copy link
Contributor

yesbox commented Feb 23, 2020

The Nixpkgs manual explains, "Note that we are not able to test or build unfree software on Hydra due to policy. Most unfree licenses prohibit us from either executing or distributing the software.".

That doesn't mean we can't have a unfree test or that a unfree test isn't useful. It's usefulness would be limited due to no Hydra, but if run manually when making changes it's still useful.

Does anyone want to please merge this?

@netvl
Copy link

netvl commented Apr 19, 2020

Hello,

I was going to report the bug about directoryRoot, but apparently it is already a known issue. This prevents me from configuring rslsync correctly. Any chance of getting this merged? Thanks!

@danieldk
Copy link
Contributor Author

danieldk commented Apr 20, 2020 •
edited

@yesbox @netvl The best way to get this merge is to review the PR and once you are done post in

https://discourse.nixos.org/t/prs-already-reviewed/2617

Though I guess I also have to rebase this to resolve conflicts in nixos/tests/all-tests.nix. I can do that later today.

@danieldk
resilio: fix directoryRoot configuration
e8ce8fe 
The resilio module places the directoryRoot configuration in the webui
section. However, the generated configuration fails on the current
version of Resilio Sync with:

Invalid key context: 'directory_root' must be in global config section

This change places this key in the global configuration section to
solve this error.
@danieldk
nixos/tests/resilio: add a test for Resilio Sync
2b7cb30 
This test verifies that Resilio Sync is running and that the
authentication credentials specified in the configuration work.
@danieldk
Copy link
Contributor Author

Rebased, also updated the test script to Python.

Ping resilio-sync maintainers: @domenkozar @thoughtpolice @cwoac

@danieldk danieldk closed this Apr 21, 2020
@danieldk danieldk mentioned this pull request Apr 21, 2020
Merged
10 tasks
@danieldk danieldk mentioned this pull request Jul 20, 2020
Closed
@danieldk danieldk deleted the resilio-config-fix branch August 17, 2020 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@danieldk @nixos-discourse @yesbox @netvl