Add github-pkgs.nix?

[b-rodrigues]

As discussed here: #467

there are some popular packages exclusively hosted on Github. We don't want to offer github packages through nix in upstream, but we could add this to our rstats-on-nix fork of nixpkgs.

The idea would be to:

• make a curated list of github exclusive hosted packages that require nix-specific fixes to work
• create the appropriate nix expression in a github-pkgs.nix file that would be hosted on rstats-on-nix/github-pkgs (for example)
• when users pass a package name to r_ver from this list, use the definition of github-pkgs
• add some way of detecting if the user is passing a package from the list to git_pkgs and instead offer to install from our curated list

@mihem fyi

[b-rodrigues]

Installing directly from r-universe is not a viable option: #469

So this might be the alternative.

[b-rodrigues]

working prototype: https://github.com/rstats-on-nix/nixgitpkgs/

[mihem]

Nice. Happy to test this.
You created this github repo, but it's not in rix yet (dev branch or so), right?
So rix function would need to download https://github.com/rstats-on-nix/nixgitpkgs/blob/master/github-pkgs.nix check, if the url (maybe safer than package Name) is the same, and then use this nix-expression. Maybe just a sample use_github_pkgs = FALSE (defaults to TRUE) to deactivate this feature?
I am not sure if this will fully work with complex packages like SeuratWarappers, where we have remote dependencies plus the issue of non-working nix expression.

[b-rodrigues]

I think that ideally, it should all be very transparent, so if the user puts a package in git_pkgs with the same commit is the github-pkgs.nix file, then the expression in github-pkgs.nix should be used. If the commit is different, then a warning should be raised inviting the user to consider changing the commit to the one in github-pkgs.nix to benefit from the fixes.
As for remote deps, that's indeed an issue, for example SeuratWrappers requires velocyto.R (which requires fixes to work), so we would need to apply the same check for remote deps.

Ideally, I'd like not to introduce new arguments if I can avoid it.

[b-rodrigues]

Coming back to this with a few thoughts:

• I am a bit hesitant to implement something like this, because I don't use such packages, which means this is a feature I won't be dogfooding, and this is not ideal.
• I wonder if doing this could break the expression in ways that are not obvious. What I'm essentially doing in the repo is joining two sets of packages. Not sure what could happen here.
• But my main issue is that I don't know (yet, and maybe never) what the good interface for this should be. Let's take BPCells as an example: what happens if the user enters it in r_pkgs? Currently, this leads to an error since the package is not on CRAN. So the user now adds it as a git_pkg, great, but now what? So let's say the user adds the latest available commit, which is not the same as the one in the github-pkgs.nix file. Should this be ignored and instead the fixed version should be provided? Should it error? Should the error raised explain and invite the user to use the commit that is in github-pkgs.nix? And how do we handle versions? If github-pkgs.nix evolves and keeps providing the latest versions of packages, how do we go back to previous versions? Does this mean that we need to provide revisions of github-pkgs.nix as well then?

@mihem any thoughts?

🤔

[mihem]

My thoughts:

For my use cases it would be quite handy, if it didn't have to manually adjust the expression each time a new default.nix is generated.

I think this list does not have to be exhaustive and is probably not so long. So fair I've encountered 2-3 packages, which required manualy adjustments, so this is not a huge thing.

I do understand your concerns regarding versions. However, I think and hope that this does not require changes for each commit. I think in the case of BPCells from the first commit till some time in August this only required adding this nativeBuildInputs and postPatch, and then after this they decided to split it into subdirs so this requires adding BPCells-src. Couldn't be a way to specify the range? So specify the first commit and last commit, where this fix works. This would probably require fetching commits from github (as I did in rix to get the "correct" data) to sort them (so that you don't have to specify each commit, but only a range)?
I would also make this an experimental feature and maybe add a variable to turn it off.. And this would only apply to very few packages, so I would assume that even in the worst case this wouldn't break too much.

[b-rodrigues]

I think this is overly complex for only 3 packages, maybe we should instead have a "gallery" of examples expression covering these packages and directing users to them if they want to add these packages to an expression

[mihem]

Well for a single user 3 (maybe also a few more) packages. And I would argue the other way around if it were 100 packages this would be a lot of work but for a few packages this should not too much work to specify the manual adjustments.

Yes that s also a good idea.

But how does this solve the problem that you have to manually adjust the default.nix after it s created?

Right now I build my analysis scripts and need to add a package quite often, this requires adding it generate_env.r, running this and then nix-build. This is fine without manual adjustments but now I have to adjust the default.nix for each iteration.

[b-rodrigues]

I think the easiest way for you is to modify the generated default.nix programmatically in the same generate_env.R script. This could also be part of the gallery

[mihem]

After some trial and error with GitHub Copilot, I came up with this fix, which works well for me:

# --- BPCells block fix ---
nix_file <- "default.nix"
nix_lines <- readLines(nix_file)

bp_start <- grep("BPCells = \\(pkgs.rPackages.buildRPackage \\{", nix_lines)
bp_end <- grep("^\\s*\\}\\);", nix_lines)
bp_end <- bp_end[bp_end > bp_start][1]

if (length(bp_start) == 1 && !is.na(bp_end)) {
  # Extract repo URL, rev, sha256 from BPCells block
  url_line <- grep("url = ", nix_lines[bp_start:bp_end], fixed = TRUE) +
    bp_start -
    1
  rev_line <- grep("rev = ", nix_lines[bp_start:bp_end], fixed = TRUE) +
    bp_start -
    1
  sha_line <- grep("sha256 = ", nix_lines[bp_start:bp_end], fixed = TRUE) +
    bp_start -
    1

  url <- sub(".*url = \"(.*)\";.*", "\\1", nix_lines[url_line])
  url_src <- sub("/r$", "", url) # Remove trailing /r for BPCells-src
  rev <- sub(".*rev = \"(.*)\";.*", "\\1", nix_lines[rev_line])
  sha <- sub(".*sha256 = \"(.*)\";.*", "\\1", nix_lines[sha_line])

  # Create complete replacement block
  replacement_block <- c(
    "    BPCells-src = pkgs.fetchgit {",
    sprintf("      url = \"%s\";", url_src),
    sprintf("      rev = \"%s\";", rev),
    sprintf("      sha256 = \"%s\";", sha),
    "    };",
    "",
    "    BPCells = (pkgs.rPackages.buildRPackage {",
    "      name = \"BPCells\";",
    "      src = \"${BPCells-src}/r\";",
    "      postPatch = \"patchShebangs configure\";",
    "      nativeBuildInputs = [ pkgs.hdf5.dev ];"
  )

  propagatedBuildInputs <- grep(
    "propagatedBuildInputs = ",
    nix_lines[bp_start:bp_end],
    fixed = TRUE
  ) +
    bp_start -
    1

  # Replace BPCells block with corrected version
  nix_lines <- c(
    nix_lines[1:(bp_start - 1)],
    replacement_block,
    nix_lines[propagatedBuildInputs:bp_end],
    nix_lines[(bp_end + 1):length(nix_lines)]
  )

  writeLines(nix_lines, nix_file)
}

cat(
  "###################################################################################################\n"
)
cat(
  "###################################################################################################\n"
)
cat(
  "###################################################################################################\n"
)
cat("Updated nix files successfully:\n")
print(nix_lines)

So maybe something for the gallery.
I tried to make it more generic by using the url, rev, and sha given in the inital rix.call instead of hardcoding everything.
However,only tested in this scenario.

[mihem]

@b-rodrigues and if you are interested in another real-life example where rix is used (in combination with targets). I've uploade my code here https://github.com/mihem/texas_sural

[b-rodrigues]

Wow, that’s really neat! I didn’t know you could define "sub-pipelines" with targets and call them like this with a main script! Very nice, I should implement this in rixpress :)

[mihem]

Welcome.
Yes, read that somewhere and found it super useful in large projects. Otherwise you will end up with one large pipelines (equivalent to one large script), which is quite messy.

Yeah I also wanted to give rixpress a try but targets is such a mature, well documented package, which is hard to beat, and did not see any advantages using rixpress?

[b-rodrigues]

For now, the only nice thing with rixpress is if you need both R and Python, otherwise, agreed, targets is the GOAT!