Hey @SRGOM.

First of all, I'm not using onedrive, I just thought it's cool and that we should package it and I don't mind maintaining it much. But, upstream provides a systemd service. Do we package it? If we do, I don't think we should add our own version of it. If we don't package it, let's start by adding it to our package and perhaps make sure everything here:

https://github.com/abraunegg/onedrive/tree/master/contrib

Will get packaged as well. BTW it needs a small version update (see) it could be nice to get on the way..

@doronbehar thanks for responding. I have to confess :

1. I don't know much about systemd in general.

2. I don't know how to package existing services in nixos (I will find out)

That said, this code allows multiple oned instances for the same user. So at least one of the two services is different.

Also, sorry I don't know what you mean by "adding it to our package". Don't modules live in diff hierarchy? (First time module writer )

I may not be able to spend effort to package other things, sorry. I believe a service is critical hence adding.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

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

@doronbehar I did a find for .service files in the nixpkgs repository, and there's no such file. So I guess you meant just copy paste the service from upstream into a nix style service. Got it.

@doronbehar I have addressed your comment and incorporated the service from onedrive upstream. Any other comments you'd like to add?

@jonringer Since you too reviewed (and approved) #73360, may I further abuse your hospitality by requesting you to review this too? I still have to add documentation but the service part is done (as best as I can- please see the Ideal Todo part in my original post)

you may, however, I'm still largely ignorant of nixos module best practices

cc @infinisil

@SRGOM I'm pretty sure @doronbehar meant you should use systemd.packages instead of writing (or copy+pasting) systemd unit files. Do a quick grep through nixpkgs and you'll see when upstream has a systemd unit file we prefer that.

Also please keep to a consistent 2 spaces per level of indentation.

I don't know how to package existing services in nixos (I will find out)

When I said "package files from upstream", I meant that we need to make sure our onedrive package definition should include the files from https://github.com/abraunegg/onedrive/tree/master/contrib . I assumed you are somewhat familiar with the nixpkgs packages' build system (this is where most people start) - sometimes, packages build systems "install" such files to the the prefix which is $out in our case. @SRGOM If almost every word / phrase I'm using here isn't understood to you, please read about this in the manual, as I can't fill for you all of the necessary gaps here and now.

In general, my comment comes from the question: What does this service / module does that the package doesn't provide by it self? And if the package should provide something, I think it'd be better to leave it that way.

Alright, I'm going to address your comments as soon as I can (I'll probably need to read up quite a bit). But quickly, @doronbehar- my code allows running the service for an unlimited number of onedrive accounts for the same user with very little effort. Just list all the config dirs in ~/.config/onedrive-launcer and onedrive services will be instantiated for all those accounts.

Actually I just re-read both your comments and yeah, I happen to (fortunately) understand nix packaging. I understand (now) how services in Nix are handled, just like bins are moved to $out/bin, services are moved to $out/....servies path.... Fine.

With that understanding I want to say that the module I am submitting is very different and cannot make use of anything but adapted code from upstream.

In my code- I have two things- one is an instantiable "onedrive@" service, which instantiates on config directories. The other is a launcher service which launches each of these instantiated services.

The upstream code cannot be reused for that purpose without adapting.

@aanderse I will fix spacing.

Documentation screenshot-

I am also adding config.services.onedrive.package to environment.systemPackages- this is because first run needs to be manual for setting up oauth by visiting link on the browser.

@SRGOM Looks great!

Just out of curiosity... is this module something which might be better suited for home-manager instead?

I'm not sure what defines the boundaries and shared regions but in this particular case, if multiple users use a single OneDrive account, no configuration on their part besides the initial oauth is required once the sysadmin enables the service. That's a strong argument in favor of having a "global " service.

That said, I'm always curious why somebody would use home manager at all? Having packages on a multi user system, sure- but it's a bad idea as far as config management is concerned - you cannot edit a file and let things apply on the fly (unlike the dotfile + stow approach). And you don't have a "stateless" config directory anyway- things are very stateful. Also when I open my ~/.ssh/config in vim, ProxyCommand is colored but PoxyCommand isn't. Not something home manager can give yopu.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

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

• Use $XDG_CONFIG_HOME instead of .config. The author of the main package- onedrive, uses .config and not $XDG_CONFIG_HOME in https://github.com/abraunegg/onedrive/blob/master/docs/USAGE.md. I would prefer using XDG but would go with .config because that's what the author seemingly uses, not sure what he's doing in code

@SRGOM

Great question. Based on extensive testing and fixing issues / bugs from the original client, many distro's do not set correctly the XDG_CONFIG_HOME value, combined with that if the system is entirely headless (no X11) this value will also not exist.

In the code itself (config.d) there is the following:

// Determine the base directory relative to which user specific configuration files should be stored.
if (environment.get("XDG_CONFIG_HOME") != ""){
	log.vdebug("configDirBase: XDG_CONFIG_HOME environment variable set");
	configDirBase = environment.get("XDG_CONFIG_HOME");
} else {
	// XDG_CONFIG_HOME does not exist on systems where X11 is not present - ie - headless systems / servers
	log.vdebug("configDirBase: WARNING - no XDG_CONFIG_HOME environment variable set");
	configDirBase = homePath ~ "/.config";
}

This allows to check if XDG_CONFIG_HOME is set, but if not use ~/.config

If NixOS is reliably setting XDG_CONFIG_HOME 100% of the time, and this is the norm for the distro from a documentation perspective, I would rather you follow your documentation standards than 'break' that just for this application.

Other than that, the documentation changes / additions look fine to me.

@abraunegg Thank you, clearly you've spent time worrying about stability. That if else check makes me want to use XDG_CONFIG_HOME but (and I forgot to update my original post) XDG_CONFIG_HOME isn't available in the systemd service.

Entirely headless => no XDG_CONFIG_HOME

Interesting.

You mention "not sure". Is that a final in which case I will close because I have no motivation to write a home manager module. As I noted above, and I'm not being snarky,

That said, I'm always curious why somebody would use home manager at all? Having packages on a multi user system, sure- but it's a bad idea as far as config management is concerned - you cannot edit a file and let things apply on the fly (unlike the dotfile + stow approach). And you don't have a "stateless" config directory anyway- things are very stateful.

Also when I open my ~/.ssh/config in vim, ProxyCommand is colored but PoxyCommand isn't. Not something home manager can give yopu.

This isn't the occasion to talk about Home Manager but suggesting an unofficial solution here seems a bit off when it fits well in the official solution just fine. Creating a config file in the home directory is like creating a vimrc or emacs .el. . It's optional.

Thank you for spending the time to look and for maintaining NixOS. (I hope I didn't sound ungrateful with my above comments, it's just that I feel a bit strongly about home manager I feel that HM is the nail to the (amazing) Nix hammer. I am 100% against entitlement, this is your repo and you guys are free to do whatever and I am free to fork NixOS (and get nowhere). But I think most of what I'm doing is inline with other projects.

What if there was a compromise that meant the system admin could declare which users OneDrive is enabled for?

{
  services.onedrive.package = pkgs.onedrive;

  services.onedrive.users."ian" = {
    enable = true;
    configDirs = [ ".config/onedrive" ".config/office365" ];
  };
}

This means the systemd services are controlled by the system administrator rather than users.

The module would only set up the services where declared users have a config file in one of the prescribed config directories.

services.onedrive.users.<name?>.configDirs would be optional, defaults to value of services.onedrive.configDirs.

services.onedrive.configDirs would be optional, defaults to [ ".config/onedrive" ].

configDirs entries are always evaluated relative to the user's home dir.

A general problem with NixOS modules that control user-level things is that users can't control it unless they're a super-user: If you want to enable such a service, you need to be able to call nixos-rebuild switch.

In contrast, home-manager allows you to do that, because it runs on a per-user basis, with its config file in ~/.config/nixpkgs/home.nix. And for the people who have single-user systems, they can also run home-manager as part of NixOS, allowing you to configure home-manager from your configuration.nix:

{
  home-manager.users.alice = {
    services.onedrive.enable = true;
  };
}

home-manager is nowadays pretty much the standard way to configure user-level things. Yes it's a third-party tool, but I think we should embrace this, because nixpkgs is really quite big already. Once NixOS/rfcs#42 is finished it will also allow easier usage of third-party Nix code and libraries.

Given I have absolutely no knowledge of NixOS, please can you keep this in mind when reading below & making your decision as how best to run this under your OS / distribution:

1. If you run 'onedrive' as a 'super user' (eg root) either via cron or systemd, then all the files downloaded from OneDrive will have a UID/GID = this user. The issue here is that 'many' folk run this as a system service (see https://github.com/abraunegg/onedrive/blob/master/docs/USAGE.md#onedrive-service-running-as-a-non-root-user-via-systemd-without-notifications-or-gui). Example - you run as systemd, but configure the sync_dir to be /path/to/my/files - essentially you will get access / permission issues and problems when attempting to read / write the files.
2. Whilst the binary is installed with root privileges, standard users can run & configure the application as to how they want to 'use it'. Essentially the application 'has' to run as the user that intends to access / use the files. This allows then 'many' users on the same system, at the same time can use the same binary, but with their unique configuration to access OneDrive & sync in the manner that they want.
3. The ~/.config/onedrive user path serves the following purposes:
   • Storing the 'configuration' file if you want to 'modify' the application defaults (see: https://github.com/abraunegg/onedrive/blob/master/docs/USAGE.md#configuration)
   • Storing the 'application sync cache' data
   • Storing a 'hash' to detect configuration changes
   • Storing the users O2Auth information for accessing the OneDrive service
4. The config path is configurable via the CLI (or by modifying the systemd / initd scripts) by using --confdir - if ~/.config/onedrive if not applicable, then store elsewhere, but ensure that the --confdir is used at each application runtime.

@ianmjones

This means the systemd services are controlled by the system administrator rather than users.

See above - unless you enable the right user service, then it will cause issues when downloading files as a non super / system admin user.

@abraunegg said...

unless you enable the right user service, then it will cause issues when downloading files as a non super / system admin user.

Yup, this module for NixOS only creates user level systemd units, and only starts the service for the user if they have a recognised config dir for onedrive.

Thank you, I appreciate the good faith discussion. I can see how home manager shines here. I want to add one thing though (I hope I'm not answering a non-question)-

Unlike other distros, NixOS allows users to install their own packages. So I can see how it might be a concern that admins are required for a service. But that's true for a lot of other services, (redshift comes to mind). So this service isn't much different from others really.

Also, just to give an idea of how this interacts with config files.

Assume disabled service. Nothing happens.

Assume enabled service. If I as a user have .config/OneDrive, then I likely want the service to run anyway. But the module gives you an option anyway to not run the individual service if you do not wish.

Basically once the admin enables the service, only the oneshot onederivelauncher service starts mandatorily . Everything else is trivially manageable by the user.

In fact I'd argue that redshift (and probably others) don't belong in nix because there is no way to configure different color temperature preferred by different people. I like 1800k candle flame, most are used to 2700k tungsten filament bulb.

@ianmjones Sorry I missed your previous comment- I believe the concern infinisil's concern here is the lack of flexibility (please correct me if not). As such my previ0us comment tries to address that.

Was your concern in response to that. If not, does my previous comment seem to address what your motivation might have been? Happy to answer more

@SRGOM No worries, my previous comment was indeed trying to address @infinisil's original comment about not liking the requirement of users adding config files to their home dir that are specific to this module (that's how I read their comment). As you know, I too raised concerns about the ~/.config/onedrive-launcher file, it doesn't feel right.

It also doesn't feel right to me that we're currently needing to install a third party channel (home-manager) in order to get per user declarative config, hence my suggested compromise.

However, I do like the format that @infinisil's follow-up home-manager example used for enabling the onedrive service on a per user basis, so I'm going to give home-manager a try.

It is a shame though that we can't have a OneDrive service module that enables users to optionally use it simply by having the standard config in place, and doing the one-time manual authentication to confirm access.

I actually think that allowing the user the flexibility of turning on and off sync without a system rebuild is a positive. I believed that your concern arose from it not being a package defined standard but something my module created ( I share that but I don't think it's a deal breaker).

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-setup-user-ssh-keys/5745/4

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/using-dropbox-on-nixos/387/21

Is there a status on this PR? Upstream OneDrive for Linux client author seems to believe it's been rejected?

Definitely not rejected but there are a few things that need to be addressed.

I am not an expert in linux systemd related aspects. That combined with my computer's internet issues which require me to manually start internet after every login make it hard for me to understand if the service has started at login. My original code did work fine for me (I had good internet then)

That said, I want to thank @peterhoeg for the review and also clarify one bit about how the launcher works-

You can definitely start and stop a single service (I do it all the time to save bandwidth). What the launcher adds is the ability to start all your onedrive instances in one go. So the oneDriveLauncher is a good thing.

I am not sure when I will get time again to complete this PR. If someone else wants to go ahead with this, please feel free .

@devhell

Is there a status on this PR? Upstream OneDrive for Linux client author seems to believe it's been
rejected?

Updated the documentation (abraunegg/onedrive@ed1d13b) to reflect that this is not rejected, but needs work

@peterhoeg The way you made suggestions makes it sound like you understand systemd well, could you kindaly suggest a way to quickly test start-up of user services- I wonder if kill -9 -1, and kill -9 0 suffice?

@peterhoeg hi, let me know if you need further changes here. I've spent some time today and I can spend more time fixing this since I'm (again) familiar with the code. The service starts perfectly for me upon reboot and the subsequent login.

@peterhoeg I have continued this work in #91765 due to GitHub login issues.