rPackages.littler: fix build #51282
rPackages.littler: fix build #51282
Conversation
| littler = old.littler.overrideAttrs (attrs: with pkgs; { | ||
| buildInputs = [ pcre lzma zlib bzip2 icu which ] ++ attrs.buildInputs; | ||
| propagatedBuildInputs = [ which ] ++ attrs.propagatedBuildInputs; | ||
| outputs = [ "out" "man" "doc" ]; |
There was a problem hiding this comment.
No other R package provides those outputs, so don't think it's a good idea to add them here because nobody will expect those outputs to exist.
There was a problem hiding this comment.
Do you think I should just leave split-outputs out (put everything into $out) or that this package should rather do to top-level all-packages.nix? It differs from most other R packages in the way that it provides a separate entry point into the R ecosystem.
| @@ -950,6 +950,18 @@ let | |||
| preConfigure = "patchShebangs configure"; | |||
| }); | |||
|
|
|||
| littler = old.littler.overrideAttrs (attrs: with pkgs; { | |||
| buildInputs = [ pcre lzma zlib bzip2 icu which ] ++ attrs.buildInputs; | |||
| propagatedBuildInputs = [ which ] ++ attrs.propagatedBuildInputs; | |||
There was a problem hiding this comment.
Why does this package have to propagate which?
There was a problem hiding this comment.
Because it won't build otherwise. Example error output in case this is left out:
** testing if installed package can be loaded
Error: package or namespace load failed for 'littler':
.onAttach failed in attachNamespace() for 'littler', details:
call: system(paste(which, shQuote(names[i])), intern = TRUE, ignore.stderr = TRUE)
error: error in running command
Error: loading failed
Taking it one level back it seems that access to which is actually a core dependency of R. There are 15+ package overrides present in https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/r-modules/default.nix that put which into propagatedBuildInputs on a per-package basis. Perhaps it would even cleaner to move which from buildInputs to propagatedBuildInputs in https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/science/math/R/default.nix#L18. Thoughts?
| propagatedBuildInputs = [ which ] ++ attrs.propagatedBuildInputs; | ||
| outputs = [ "out" "man" "doc" ]; | ||
| postInstall = '' | ||
| install -D inst/bin/r $out/bin/r |
There was a problem hiding this comment.
The install process copies dozens of files into $out, albeit into strange locations. Now, this code will add even more files, some of them clearly duplicates. I think it would be better to clean up the generated output, i.e. remove files that aren't needed and move others into the right place.
There was a problem hiding this comment.
Yeah, I'll clean it up. But we should probably decide upon the questions at top.
littler gives a wrapped called `r` (or `lr` for non-case-preserving systems like Darwin) which works as shebang or pipe target. The build was completely broken before (missing deps). Symlink executables and manpage into standard locations so that this packages also works in `environment.systemPackages`.
|
@peti Second attempt. Things changed:
Now acceptable? |
Motivation for this change
The littler build was completely broken. Fix littler by providing necessary deps and moving build artefacts into bin, man, doc.
Usage:
Provides an alias
lrfor non-case-preserving filesystems (think Darwin) as proposed by the package author.Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after) -> N/A (build was broken before)