[Nix-dev] Re: [patch] Amule improvements

[Ludovic Courtès]

Hi!

Thanks for the patch!

This looks good to me modulo the stylistic comments below.  Feel free to
commit it yourself (ask Eelco Dolstra for an SVN account).

Evgeny Egorochkin <phreedom.stdin at gmail.com> writes:

> +  configureFlags = ''
> +    --with-crypto-prefix=${cryptopp}
> +    ${if monolithic then "--enable-monolithic" else "--disable-monolithic"}
> +    ${if daemon then "--enable-amule-daemon" else "--disable-amule-daemon"}
> +    ${if client then "--enable-amule-gui" else "--disable-amule-gui"}
> +    ${if httpServer then "--enable-webserver" else "--disable-webserver"}
> +  '';

I find it more convenient to use a list, like this:

  configureFlags =
    [ "--foo" ]
    ++ stdenv.lib.optional monolithic "--bar"
    ...;

>    postInstall = ''
> -    wrapProgram "$out/bin/amule" \
> -      --prefix LD_LIBRARY_PATH ":" "${libupnp}/lib"
> +    ${if monolithic then "wrapProgram \"$out/bin/amule\" --prefix LD_LIBRARY_PATH \":\" \"${libupnp}/lib\"" else ""}
>    '';

Can you wrap the line as before and avoid the double quoting, like this:

  postInstall =
    stdenv.lib.optionalString monolithic
      '' wrapProgram ... \
           ...
      '';

Most importantly, do add yourself in ‘meta.maintainers’.  :-)

>    amule = import ../tools/networking/p2p/amule {
>      inherit fetchurl stdenv zlib perl cryptopp gettext libupnp makeWrapper
> -      wxGTK pkgconfig;
> +      wxGTK pkgconfig libpng;
> +    monolithic = getConfig ["amule" "monolithic"] true;
> +    daemon = getConfig ["amule" "daemon"] false;
> +    httpServer = getConfig ["amule" "httpServer"] false;
> +    client = getConfig ["amule" "client"] false;
>    };

I gather that the preferred way to customize packages is via
‘packageOverrides’ nowadays, so you might want to omit the ‘getConfig’
calls altogether.

Thanks,
Ludo’.