Please use haskell.lib.addExtraLibrary to add that extra dependency.

• The [] ++ bit is redundant.
• pkgs.lib.optional might be a better choice than pkgs.lib.optionals.

The difference between optionals and optional is that the former takes a list argument whereas the latter takes an element. Just changing the name of the function won't result in valid code. You have to adapt your arguments, too. You'll notice what I mean when you, like, test the patch you're submitting.

Also, if you want to pass a list of dependencies here, then you'll need to use addExtraLibraries instead.

I'm still able to build the package with the current patch on macos & Linux (nix-env -f./ -iA haskellPackages.OpenAL).

Yes, you are right. This is an unexpected consequence of the way Nix casts lists into a string by concatenating the individual elements cast to a string. In this particular case, that gives a usable result, i.e. the [pkg] ends up being essentially the same thing as pkg (which is what we really want here). It's kind of unfortunate that this bogus code still gives a correct result, mostly due to chance. It's on those occasions where one would really like Nix to have a type system.

This change should be similar to other changes already in the file:

There is a small problem with the current code:

addExtraLibrary super.OpenAL (pkgs.lib.optional pkgs.stdenv.isDarwin pkgs.darwin.apple_sdk.frameworks.OpenAL);

Here, addExtraLibrary wants to take a derivation for its second argument. (If you grep through the nixpkgs source code for addExtraLibrary, you can see that normally it takes things like udev, libpcap, etc.)

However, the output of optional is a list:

$ nix repl '<nixpkgs>'
nix-repl> lib.optional true "foo"
[ "foo" ]
nix-repl> lib.optional false "foo"
[ ]

Now, in practice, I think that this still works out. The derivations passed to addExtraLibrary eventually get passed to buildInputs for the Haskell package derivation. Haskell package derivations eventually get built with stdenv.mkDerivation, which apparently knows how to deal with inputs in sublists:

$ nix-shell -E 'with (import <nixpkgs> { }); runCommand "dummy" { buildInputs = [ gcc6 ]; } ""' --run 'gcc --version'
gcc (GCC) 6.5.0
$ nix-shell -E 'with (import <nixpkgs> { }); runCommand "dummy" { buildInputs = [ [ gcc6 ] ]; } ""' --run 'gcc --version'
gcc (GCC) 6.5.0

However, it is somewhat confusing to rely on this functionality when writing nix code, so it would be best to change the way it is currently written.

As an aside, you could also do the following:

addExtraLibraries super.OpenAL (pkgs.lib.optional pkgs.stdenv.isDarwin pkgs.darwin.apple_sdk.frameworks.OpenAL);

This works because addExtraLibraries expects a list of derivations as the second argument.

However, personally, I feel that just using an if statement is the clearest way to write it. It is also used in other parts of the file already.

Also, you may want to just rebase your commits into a single, clean commit.

Sorry for going back and forth so much on this change!

No worries, I'm still new to nix so thanks for the patience.

I'm a bit confused on what are the best practices regarding the overrides because both the if-then code and my original patch are already used.

If possible, the "best practices" are to use the functions provided in pkgs/development/haskell/lib.nix.

However there are a lot of places not using the functions provided. Ideally they would all be fixed where possible.