Hello Christopher, Christopher Baines writes: >> >> > Yep, I think I just stopped writing the test after finding the >> >> > issue with the PID file. >> >> > >> >> > I haven't looked in to how to fix this in the test, so if you >> >> > could, that would be great. Otherwise, I'll probably have time >> >> > to look at this again within a week or so. >> >> > >> >> > You'll probably need to refactor the test a bit, as at the >> >> > moment, the information regarding the port isn't available where >> >> > you run the commands. >> >> >> >> Of course I'll try. By the way, how to run a “vm”? Previous >> >> method “./pre-inst-env guix system vm gnu/tests/rsync.scm” doesn't >> >> work for me. >> > >> > I'm guessing that you'll need to make the file evaluate (I'm not >> > sure if that is the right word here) to an operating-system, e.g. >> > put %rsync-os-with-port-2000 right at the bottom of the file, and >> > then guix system vm should give you a start script that will start >> > a VM for that OS. >> >> I did some work on rsync service: >> >> - Fixed PID and synchronization to specific port. >> - Merged two rsync oses in one with optional port. >> - Added ports to rsync synchronization tests and change protocol from >> ssh to rsync. >> - Added some logic to config: chroot (can use only root), user and >> group. >> >> All tests passed successfully for me. > > Great :) Now that the tests pass at least, I don't see any reason to > not merge this soonish. > > I've still done some thingking about how the configuration works > though, and I've been considering a few ways of tweaking this so that > its harder to break, and clear in how it works. > > One way I've managed to break the service so far is setting the user > and group to root in the configuration. This causes the tests to fail, > and in a odd way, as I think the problem is that the creation of > the /var/run/rsync directory relies on the account service, but I'm > guessing that the account service does nothing in this case, as root is > already an account. > > One way of making this harder to break would be to explicitly create > the necessary directories when this service is activated, e.g.: > > (mkdir-p (dirname #$(rsync-configuration-pid-file config))) > (mkdir-p (dirname #$(rsync-configuration-lock-file config))) > > I think there is also the opportunity to make the service configuration > clearer here, as considering the default port test, the default > configuration says it will run as the rsync user, but the service will > actually run as the root user. > > This could be improved by making the configuration more uncertain by > default, e.g. user defaults to #f, which means the correct user is > decided based on the port. > > Also on the subject of clarity, the use-chroot? option is something > that can be specified, but the value might not be used. My preference > would be to change the "logic" in the configuration file generation, to > validation, e.g.: > > (if (and use-chroot? (not (eq? user "root"))) > (error "rsync-service: to run rsync in a chroot, the user must be root")) > > The use-chroot? option might also benefit from making the user default > to #f, as then the service could decide the user based on the port and > use-chroot? settings, without contradicting the configuration. I tried to apply all your suggestion, but I really don't know why 'rsync-configuration-user' is ignored (it's always set to "rsyncd"). I use 'pk' to debug this, but still confused. --8<---------------cut here---------------start------------->8--- ;;; (rsync-config-file-config #< package: # port-number: 581 pid-file: "/var/run/rsyncd/rsyncd.pid" lock-file: "/var/run/rsyncd/rsyncd.lock" log-file: "/var/log/rsyncd.log" use-chroot?: #f share-path: "/srv/rsyncd" share-comment: "Rsync share" read-only?: #f timeout: 300 user: "rsyncd" group: "rsyncd" uid: #f gid: #f>) --8<---------------cut here---------------end--------------->8---