[Nix-dev] Re: [Nix-commits] SVN commit: nix - 18431 - raskin - in nixos/trunk/modules: . services/networking

[Eelco Dolstra]

Hi,

Michael Raskin wrote:

> --- nixos/trunk/modules/services/networking/gvpe.nix	                        (rev 0)
> +++ nixos/trunk/modules/services/networking/gvpe.nix	2009-11-18 14:54:37 UTC (rev 18431)
> @@ -0,0 +1,140 @@
> +# GNU Virtual Private Ethernet
> +
> +{config, pkgs, ...}: 

In order to make the expression more readable, could you include some empty
lines between definitions?  Like:

> +let 
> +  inherit (pkgs.lib) mkOption mkIf;
  <-- here
> +  cfg = config.services.gvpe;
  <-- here
> +  finalConfig = if cfg.configFile != null then 
> +    cfg.configFile
> +  else if cfg.configText != null then
> +    pkgs.writeTextFile {
> +      name = "gvpe.conf";
> +      text = cfg.configText;
> +    }
> +  else 
> +    throw "You must either specify contents of the config file or the config file itself for GVPE";
  <-- here
> +  ifupScript = if cfg.ipAddress == null || cfg.subnet == null then 
> +     throw "Specify IP address and subnet (with mask) for GVPE" 
> +   else if cfg.nodename == null then 
> +     throw "You must set node name for GVPE" 
> +   else
> +   (pkgs.writeTextFile { 
> +    name = "gvpe-if-up";
> +    text = ''
> +      #! /bin/sh
> +      
> +      export PATH=$PATH:${pkgs.iproute}/sbin
> +
> +      ip link set $IFNAME up
> +      ip address add ${cfg.ipAddress} dev $IFNAME
> +      ip route add ${cfg.subnet} dev $IFNAME
> +
> +      ${cfg.customIFSetup}
> +    '';
> +    executable = true;
> +  });
  <-- here
> +  exec = "${pkgs.gvpe}/sbin/gvpe -c /var/gvpe -D ${cfg.nodename} "
> +    + " ${cfg.nodename}.pid-file=/var/gvpe/gvpe.pid"
> +    + " ${cfg.nodename}.if-up=if-up"
> +    + " &> /var/log/gvpe";
  <-- here
> +  inherit (cfg) startOn stopOn;
  <-- here
> +in
> +
> +{
> +  options = {
> +    services.gvpe = {
> +      enable = mkOption {
> +        default = false;
> +        description = ''
> +          Whether to run gvpe
> +        '';
> +      };
> +      startOn = mkOption {
> +        default = "network-interfaces/started";

Upstart 0.6 should make this unnecessary, because we can let jobs depend on a
runlevel-like "networking" event that includes whatever services are needed
(such as gw6c).

> +        description = ''
> +          Condition to start GVPE
> +        '';
> +      };
> +      stopOn = mkOption {
> +        default = "network-interfaces/stop";
> +        description = ''
> +          Condition to stop GVPE
> +        '';
> +      };
> +      nodename = mkOption {
> +        default = null;
> +        description =''
> +          GVPE node name
> +        '';
> +      };
> +      configText = mkOption {
> +        default = null;
> +        example = ''
> +          tcp-port = 655
> +          udp-port = 655
> +          mtu = 1480
> +          ifname = vpn0
> +          
> +          node = alpha
> +          hostname = alpha.example.org
> +          connect = always
> +          enable-udp = true
> +          enable-tcp = true
> +          on alpha if-up = if-up-0
> +          on alpha pid-file = /var/gvpe/gvpe.pid
> +        '';
> +        description = ''
> +          GVPE config contents
> +        '';
> +      };
> +      configFile = mkOption {
> +        default = null;
> +        example = "/root/my-gvpe-conf";
> +        description = ''
> +          GVPE config file, if already present
> +        '';
> +      };
> +      ipAddress = mkOption {
> +        default = null;
> +        description = ''
> +          IP address to assign to GVPE interface
> +        '';
> +      };
> +      subnet = mkOption {
> +        default = null;
> +        example = "10.0.0.0/8";
> +        description = ''
> +          IP subnet assigned to GVPE network
> +        '';
> +      };
> +      customIFSetup = mkOption {
> +        default = "";
> +        description = ''
> +          Additional commands to apply in ifup script
> +        '';
> +      };
> +    };
> +  };
> +  config = mkIf cfg.enable 
> +  {

This should be:

  config = mkIf cfg.enable {

> +    jobs.gvpe = {
> +      description = "GNU Virtual Private Ethernet node";
> +      
> +      inherit startOn stopOn;
> +
> +      preStart = ''
> +        mkdir -p /var/gvpe
> +        mkdir -p /var/gvpe/pubkey
> +        chown root /var/gvpe
> +        chmod 700 /var/gvpe
> +        cp ${finalConfig} /var/gvpe/gvpe.conf
> +	cp ${ifupScript} /var/gvpe/if-up

There seems to be a tab here.

-- 
Eelco Dolstra | http://www.st.ewi.tudelft.nl/~dolstra/