Get closest commit

[mihem]

@b-rodrigues as you know i really like the automatic handling of remote dependencies. However, I wasn't aware of the caveats at the time, when we were working on that. In your docs you now clearly explain:

rix will automatically use the latest commit from these repositories as the revision. This also means that if these repositories are being actively worked on, rebuilding these environments will actually pull another version of these packages.

This makes sense. However, this is not a good default, as it's quite dangerous I think.

With this PR, I tried to get the commit that is closest to the date of the commit hash given (only earlier in time not later).
This actually turned out much more complex than what I anticipated, but with some help of ChatGPT, Claude Sonnet I have a working solution. However, this involves quite some changes so please carefully check. This also needs some formal testing I think.

Al of this relies on api github naturally. In the beginning i used your try_download function, but then ChatGPT told me that is more efficient to just use fromJson as it does not require creating files, downloading, deleting etc.
Since this requires quite some API calls, especially if there are many remote dependencies (as in seurat-wrappers which i used for testing) and many commits (espeically step2), I hit the API limit when testing. I though about changing to gh which automatically used your GitHub token if i understood correctly (then you have 5000 instead of 60 requests per hour). But that would have involved a new package, so for now I sticked with fromJson. I limited API request by only downloading 300 commits max.

I used the following example for testing

rix(
  r_ver = "4.4.1",
  r_pkgs = NULL,
  system_pkgs = NULL,
  git_pkgs = list(
    package_name = "seurat-wrappers",
    repo_url = "https://github.com/satijalab/seurat-wrappers",
    commit = "5b63c23b211002611b0ccc33da009237d0dbafb6"
  ),
  ide = "other",
  project_path = ".",
  overwrite = TRUE,
  print = TRUE
)

Algorithm:

Step 1 get the date of the commit has given

This is done by the new function get_commit_date. Pretty simple. In my example I used, it correctly identified the date of commit 5b63c23b211002611b0ccc33da009237d0dbafb6 as 2022-05-25T21:58:55Z. I used tryCatch and this faills back to the current date.

Step 2 download all commits for each remote dependency if no ref is given

This is the most complex step and can require a lot of API calls. This is done with the new download_all_commits function. This is used in the new revolsed_package_commit function. This is done for each remote dependency in get_imports
in lapply in get_imports and integrated in the
for each remote dependency, yet only if there is not ref given. By default only 30 commits were shown, so this requires looping through multiple pages, but I limited it to 300 commits because of API limits. This falls back to HEAD.

Step 3 get closest commit of each remote dependency

Get the closest commit (earlier not later) of each package. This is done in the new function get_closest_commit. Nothing fancy here.

So e.g for the demote dependency monocle I get

                            sha                date
52 87f6e88760566065179ae5039d524da7d032cc15 2022-05-23 17:51:13

Here's the entire default.nix that I get with the above rix call.. default.nix.log

This still fails due to seurat-disk appearing twice. This should be fixed with my other PR (#388) although I did not test it yet because I didn't want to mix PRs (maybe you can first review the other PR, then I can also test this). Then only seurat-data with commit hash d6a8ce61ccb21a3b204f194d07009772c822791d should be present.

However, even if I remove the other seurat-data entry manually, nix-build fails with

npacking 'https://github.com/rstats-on-nix/nixpkgs/archive/2024-10-01.tar.gz' into the Git cache...
error:
       … while calling the 'derivationStrict' builtin
         at <nix/derivation-internal.nix>:34:12:
           33|
           34|   strict = derivationStrict drvAttrs;
             |            ^
           35|

       … while evaluating derivation 'nix-shell'
         whose name attribute is located at /nix/store/z0h0p53czz33cnhsp6v4drzjsdclifl0-source/pkgs/stdenv/generic/make-derivation.nix:336:7

       … while evaluating attribute 'buildInputs' of derivation 'nix-shell'
         at /nix/store/z0h0p53czz33cnhsp6v4drzjsdclifl0-source/pkgs/stdenv/generic/make-derivation.nix:383:7:
          382|       depsHostHost                = elemAt (elemAt dependencies 1) 0;
          383|       buildInputs                 = elemAt (elemAt dependencies 1) 1;
             |       ^
          384|       depsTargetTarget            = elemAt (elemAt dependencies 2) 0;

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: Package ‘r-riverplot-0.10’ in /nix/store/z0h0p53czz33cnhsp6v4drzjsdclifl0-source/pkgs/development/r-modules/generic-builder.nix:56 is marked as broken, refusing to evaluate.

       a) To temporarily allow broken packages, you can use an environment variable
          for a single invocation of the nix tools.

            $ export NIXPKGS_ALLOW_BROKEN=1

          Note: When using `nix shell`, `nix build`, `nix develop`, etc with a flake,
                then pass `--impure` in order to allow use of environment variables.

       b) For `nixos-rebuild` you can set
         { nixpkgs.config.allowBroken = true; }
       in configuration.nix to override this.

       c) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
         { allowBroken = true; }
       to ~/.config/nixpkgs/config.nix.

This is then Nix, where I don't really know how to debug this.

[b-rodrigues]

Thank you for this I'll have to carefully test it, but quickly, the issue with riverplot is that it was removed from CRAN as was liger (the package pulling it), so this is something that cannot be resolved, also outside of Nix, to fix this, riverplot should be listed as a remote so it's pulled from github instead of CRAN

[b-rodrigues]

here is the description file of liger at that revision where riverplot is in the imports

https://raw.githubusercontent.com/welch-lab/liger/8ee61cafd28cce6aaf62fd68acdc4dabb857b59f/DESCRIPTION

you either need to use a more recent version that doesn't rely on riverplot anymore or use an older date for your environment that includes riverplot

[b-rodrigues]

using the date 2022-01-16 will likely solve this part of the puzzle

[mihem]

This is just a toy example and definitely more complex than my other PR, so please revise the other one first.

Ah, this makes sense. So liger at that time required CRAN installation, which worked at that time, but since it's removed from CRAN this is problematic.
Are you sure, that this is solved by using a commit of liger of 2022-01-16? Becuase riverplot was deprecated till 2023. welch-lab/liger@3208082

I thought about it again. Wasn't the mistake that I specified r-vers: 4.4.1? I mean my intention was to build a project with all packages at a given date, so 2022-05-25 in this example.

SO then it makes more sense to use this, right?

rix(
  
    date = "2022-04-19",
    r_pkgs = NULL,
    system_pkgs = NULL,
    git_pkgs = list(
        package_name = "seurat-wrappers",
        repo_url = "https://github.com/satijalab/seurat-wrappers",
        commit = "5b63c23b211002611b0ccc33da009237d0dbafb6"
    ),
    ide = "other",
    project_path = ".",
    overwrite = TRUE,
    print = TRUE
)

Here is the default.nix
default.nix.2.log

Then riverplot at that time (so version 0.10? and still available at CRAN https://cran.r-project.org/src/contrib/Archive/riverplot/) should be used?

However, this then fails with:

unpacking 'https://github.com/rstats-on-nix/nixpkgs/archive/2022-04-19.tar.gz' into the Git cache...
error:
       … while calling the 'derivationStrict' builtin
         at <nix/derivation-internal.nix>:34:12:
           33|
           34|   strict = derivationStrict drvAttrs;
             |            ^
           35|

       … while evaluating derivation 'nix-shell'
         whose name attribute is located at /nix/store/lbqvwmvhmjq7gwmrzqrnbqn4jwrr2gfy-source/pkgs/stdenv/generic/make-derivation.nix:270:7

       … while evaluating attribute 'buildInputs' of derivation 'nix-shell'
         at /nix/store/lbqvwmvhmjq7gwmrzqrnbqn4jwrr2gfy-source/pkgs/stdenv/generic/make-derivation.nix:314:7:
          313|       depsHostHost                = lib.elemAt (lib.elemAt dependencies 1) 0;
          314|       buildInputs                 = lib.elemAt (lib.elemAt dependencies 1) 1;
             |       ^
          315|       depsTargetTarget            = lib.elemAt (lib.elemAt dependencies 2) 0;

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: attribute 'R' missing
       at /home/mischko/develop/rix_remote_dependencies/default.nix:84:11:
           83|         inherit (pkgs.rPackages)
           84|           R
             |           ^
           85|           Rcpp
       Did you mean one of AR, ER, R0, R6 or WR?

This probably has to do with lines 83 ff, which is harmony, similar to here #388 . However, if I remove harmony and put it into propagatedBuildinputs i get the following issue with Rhdf5 lib, which i cannot solve

error:
       … while calling the 'derivationStrict' builtin
         at <nix/derivation-internal.nix>:34:12:
           33|
           34|   strict = derivationStrict drvAttrs;
             |            ^
           35|

       … while evaluating derivation 'nix-shell'
         whose name attribute is located at /nix/store/lbqvwmvhmjq7gwmrzqrnbqn4jwrr2gfy-source/pkgs/stdenv/generic/make-derivation.nix:270:7

       … while evaluating attribute 'buildInputs' of derivation 'nix-shell'
         at /nix/store/lbqvwmvhmjq7gwmrzqrnbqn4jwrr2gfy-source/pkgs/stdenv/generic/make-derivation.nix:314:7:
          313|       depsHostHost                = lib.elemAt (lib.elemAt dependencies 1) 0;
          314|       buildInputs                 = lib.elemAt (lib.elemAt dependencies 1) 1;
             |       ^
          315|       depsTargetTarget            = lib.elemAt (lib.elemAt dependencies 2) 0;

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: path '/nix/store/lbqvwmvhmjq7gwmrzqrnbqn4jwrr2gfy-source/pkgs/development/r-modules/patches/Rhdf5lib.patch' does not exist

[mihem]

Note: It was really painful to debug using fromJson because I constantly hit the API limits.
So I switched to {gh}. The disadvantage is that you have one more package as a dependency. The advantage is that you now have 5000 instead of 60 requests per hour (as long as you set your GitHub token). At least as long as you want to work with something like seurat-wrappers and my current solution this is necessary, I think.

[b-rodrigues]

There are several things that need to be fixed: I realize that by backporting fixes to rhdf5lib, I actually broke it instead 😄 so now I’m trying to fix that

Then there seems to be an issue with one of the dependencies listing R itself:

       error: attribute 'R' missing
       at /home/mischko/develop/rix_remote_dependencies/default.nix:84:11:
           83|         inherit (pkgs.rPackages)
           84|           R
             |           ^
           85|           Rcpp
       Did you mean one of AR, ER, R0, R6 or WR?

as you can see, R is listed, where it shouldn’t be. There is code in rix to handle this, so I’m surprised it doesn’t seem to work in this case, or is not being called at all. I’ll need to check this.

[b-rodrigues]

There are several things that need to be fixed: I realize that by backporting fixes to rhdf5lib, I actually broke it instead 😄 so now I’m trying to fix that

this is fixed now, so the remaining issue would be to understand why R is listed as a dep, when it should be removed.

[mihem]

Just out of curiosity: could you link how you fixed rhdf5 and why harmony fails

[b-rodrigues]

check the commit history of today: https://github.com/rstats-on-nix/nixpkgs/commits/2022-01-16/

it’s a bit messy, I did from the phone on the train this morning

for harmany, no idea, I’ll have to dig deeper

[mihem]

yeah agree the Rhdf5 error is gone.

So running the following code with this PR:

# try to fix problem with correct version of remote
rix(
    date = "2025-01-14",
    r_pkgs = NULL,
    system_pkgs = NULL,
    git_pkgs = list(
        package_name = "seurat-wrappers",
        repo_url = "https://github.com/satijalab/seurat-wrappers",
        commit = "a1eb0d8b039ad6d5ef0ff1332fd8eb1c0c223553"
    ),
    ide = "other",
    project_path = ".",
    overwrite = TRUE,
    print = TRUE
)

after removing double entry of seurat-data and putting harmony into propagatedBuild now fails with:

ERROR: lazy loading failed for package 'Nebulosa'
* removing '/nix/store/dnx263faqvrc3n9wivv4585likf0pj4p-r-Nebulosa/library/Nebulosa'
error: builder for '/nix/store/0hxlkzcchk3ibfqsjn557h3qd4vqnb0f-r-Nebulosa.drv' failed with exit code 1;
       last 19 log lines:
       > Running phase: unpackPhase
       > unpacking source archive /nix/store/acp3jsy6iy6c110h9sb0yz554nc7y94j-Nebulosa-1f0714d
       > source root is Nebulosa-1f0714d
       > Running phase: patchPhase
       > Running phase: updateAutotoolsGnuConfigScriptsPhase
       > Running phase: configurePhase
       > Running phase: buildPhase
       > Running phase: installPhase
       > * installing *source* package 'Nebulosa' ...
       > ** using staged installation
       > ** R
       > ** inst
       > ** byte-compile and prepare package for lazy loading
       > Error in loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]) :
       >   there is no package called 'SeuratObject'
       > Calls: <Anonymous> ... loadNamespace -> withRestarts -> withOneRestart -> doWithOneRestart
       > Execution halted
       > ERROR: lazy loading failed for package 'Nebulosa'
       > * removing '/nix/store/dnx263faqvrc3n9wivv4585likf0pj4p-r-Nebulosa/library/Nebulosa'
       For full logs, run 'nix log /nix/store/0hxlkzcchk3ibfqsjn557h3qd4vqnb0f-r-Nebulosa.drv'.
error: 1 dependencies of derivation '/nix/store/c1pwlb2vq8spnbrxv4hmi59y3wzkiy6h-r-seurat-wrappers.drv' failed to build
error: 1 dependencies of derivation '/nix/store/xbc0s0sb37wjkg2vrdg76r5lw8wjv27v-nix-shell.drv' failed to build

So probably the same issue as in #390 and not directly related to this PR because it also occurs with rix before this PR.
Nonetheless, this PR requires some testing I think. Maybe I can write some test when I find time or how else can I help?

[b-rodrigues]

Would it be possible to not use gh, but invite users to configure the token instead? Using calls to curl should be ok, right? Adding dependencies to rix is not something we really like to do.

[b-rodrigues]

do the tests pass locally ?

[mihem]

I reviewed the git diffs and saw that i accidently removed rmeote-pkgs_names_and_refs. Tried to fix it.
You are too fast ;)

[mihem]

so checks looks better now, still there is some issue with download_all_commits. Always falls back to head, need to further debug this.

But in general:
Well curl is also a dependency, isn't it? But okay, you already use it and you are right, that it's possible to use curl for authenticated github api requests I learned. So I tried to implement that. I tried to create some safety nets, so use tryCatch for the API requests, so that the rix call does not get interrupted if that fails and then just falls backs to the current date or HEAD.

Also created a check for github_path so that it uses the github token if set (validating the token with a regex stolen from here (https://gist.github.com/magnetikonline/073afe7909ffdd6f10ef06a00bc3bc88), and otherwise uses unauthorized API requests.

I am still not 100% sure about the limit. Now it's maximum 300 commits (i think that's 3 requests). If you don't have you github token set, this already uses 50% of your available requests only with seurat-wrappers. If you would set the limit even higher (e.g. to 1000 commits), this could take some time and would lead to problems with unauthorized requests.
On the other hand, if we lower it to 50, it will probably not get the correct date if the repo is quite active and the date is further in the past. So maybe something like 300 makes sense, but open for suggestions.

[mihem]

Wow, curl really caused me a lot of headaches. There was some issue with curl_download leading to this error:

Failed to open file /tmp/RtmpZtuDSd/commit_data.json.curltmp

I now use curl_fetch_memory which seems to work fine in my manual tests.

I also realized that I could only see warnings in debugging mode. So when using debugonce my warnings were printed and helpful, but when I called rix as a whole, it just said there were warnings, using warnings() to see them. But calling warnings() did not output anything. So I converted them to messages. Not sure this is the right thing to do, happy for feedback.

[b-rodrigues]

Thank you for your efforts!

It would be nice to understand why warnings aren’t being shown though. I’ll try to take a look soon.

[mihem]

I started adding tests. This works fine locally. The tests also pass, but if you look at the logs on github you can see that the tests fails.

On macos-latest:

── Failed tests ────────────────────────────────────────────────────────────────
Error ('test-fetchers.R:141:3'): get_commit_date works with valid repo and commit
Error in `value[[3L]](cond)`: Failed to get commit date for cd7a53f7c670bd5106a94b48573d5f824174170f: API request failed with status code: 403
Backtrace:
    ▆
 1. └─rix:::get_commit_date("ropensci/rix", "cd7a53f7c670bd5106a94b48573d5f824174170f") at test-fetchers.R:141:3
 2.   └─base::tryCatch(...) at rix/R/fetchers.R:455:3
 3.     └─base (local) tryCatchList(expr, classes, parentenv, handlers)
 4.       └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 5.         └─value[[3L]](cond)

Error ('test-fetchers.R:177:3'): get_commit_date fails when no GitHub token is found
Error in `value[[3L]](cond)`: Failed to get commit date for cd7a53f7c670bd5106a94b48573d5f824174170f: API request failed with status code: 403
Backtrace:
     ▆
  1. ├─testthat::expect_message(...) at test-fetchers.R:177:3
  2. │ └─testthat:::expect_condition_matching(...)
  3. │   └─testthat:::quasi_capture(...)
  4. │     ├─testthat (local) .capture(...)
  5. │     │ └─base::withCallingHandlers(...)
  6. │     └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
  7. └─rix:::get_commit_date("ropensci/rix", "cd7a53f7c670bd5106a94b48573d5f824174170f")
  8.   └─base::tryCatch(...) at rix/R/fetchers.R:455:3
  9.     └─base (local) tryCatchList(expr, classes, parentenv, handlers)
 10.       └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 11.         └─value[[3L]](cond)
[ FAIL 2 | WARN 0 | SKIP 6 | PASS 65 ]

Probably this is because Sys.getenv("GITHUB_PAT") does not work with github actions?

BTW, on ubuntu-latest there are severy other tests that fails, unrelated to this PR (excerpt):

Failure ('test-renv_helpers.R:227:5'): Testing `renv2nix()` on actual renv.lock files
Snapshot of `save_renv2nix_test(...)` to 'renv_helpers/default_datathin.nix' has changed
Run `testthat::snapshot_review('renv_helpers/')` to review changes

Error ('test-rix.R:417:3'): rix(), frozen-edge
Error: Request `curl::curl_fetch_memory(url = 'https://api.github.com/repos/rstats-on-nix/nixpkgs/commits?sha=r-daily')` failed:
 HTTP response code said error [api.github.com]: The requested URL returned error: 403
Backtrace:
    x
 1. \-rix:::get_right_commit("frozen-edge") at test-rix.R:417:3
 2.   \-rix:::try_get_request(url = api_url, handle = h) at rix/R/get_latest.R:88:3
 3.     \-base::tryCatch(...) at rix/R/get_latest.R:107:3
 4.       \-base (local) tryCatchList(expr, classes, parentenv, handlers)
 5.         \-base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 6.           \-value[[3L]](cond)

Error ('test-rix.R:376:7'): rix(), frozen-edge
Error in `eval(code, test_env)`: object 'frozen_edge_commit' not found
Backtrace:
    x
 1. +-base::system(paste0("sed -i 's/", frozen_edge_commit, "/REVISION/' _snaps/rix/frozen-edge_default.nix")) at test-rix.R:376:7
 2. \-base::paste0("sed -i 's/", frozen_edge_commit, "/REVISION/' _snaps/rix/frozen-edge_default.nix")

[ FAIL 4 | WARN 0 | SKIP 2 | PASS 68 ]

[mihem]

@b-rodrigues regarding github token with github actions. I used

     - name: Create .Renviron
        run: |
          echo "GITHUB_PAT=${{ secrets.GITHUB_TOKEN }}" >> ~/.Renviron
        shell: bash

And now the tests also seem to pass.
Before (ubuntu-latest) 4 failes tests, 2 due to this PR

-- Failed tests ----------------------------------------------------------------
Failure ('test-fetchers.R:127:3'): Test fetchgit gets a package with several remote deps and commits
fetchgit(...) (`actual`) not equal to "\n    httr2 = (pkgs.rPackages.buildRPackage {\n      name = \"httr2\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/b-rodrigues/httr2\](https://github.com/b-rodrigues/httr2/)";\n        rev = \"HEAD\";\n        sha256 = \"sha256-ny4J2WqUL4LPLWRKS8rgVqwvgMOQ2Rm/lbBWtF+99PE=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          cli\n          curl\n          glue\n          lifecycle\n          magrittr\n          openssl\n          R6\n          rappdirs\n          rlang\n          vctrs\n          withr;\n      };\n    });\n\n    gh = (pkgs.rPackages.buildRPackage {\n      name = \"gh\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/b-rodrigues/gh\](https://github.com/b-rodrigues/gh/)";\n        rev = \"HEAD\";\n        sha256 = \"sha256-POXEMZv8kqHokAxK8LoWkS0qYrcIcVdQi5xyGD992KU=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          cli\n          gitcreds\n          glue\n          ini\n          jsonlite\n          lifecycle\n          rlang;\n      } ++ [ httr2 ];\n    });\n\n\n    highlite = (pkgs.rPackages.buildRPackage {\n      name = \"highlite\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/jimhester/highlite\](https://github.com/jimhester/highlite/)";\n        rev = \"HEAD\";\n        sha256 = \"sha256-lkWMlAi75MYxiBUYnLwxLK9ApXkWanA4Mt7g4qtLpxM=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          Rcpp\n          BH;\n      };\n    });\n\n\n    memoise = (pkgs.rPackages.buildRPackage {\n      name = \"memoise\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/b-rodrigues/memoise\](https://github.com/b-rodrigues/memoise/)";\n        rev = \"74d62c8\";\n        sha256 = \"sha256-fsdop66VglkOIYrJ0LKZKikIZmzQ2gqEATLy9tTJ/B8=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          digest;\n      };\n    });\n\n    lookup = (pkgs.rPackages.buildRPackage {\n      name = \"lookup\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/b-rodrigues/lookup/\](https://github.com/b-rodrigues/lookup//)";\n        rev = \"ee5505c817b19b59d37236ed35a81a65aa376124\";\n        sha256 = \"sha256-jiSBuC1vzJbN6OckgVX0E+XuMCeZS5LKsldIVL7DNgo=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          Rcpp\n          codetools\n          crayon\n          rex\n          jsonlite\n          rstudioapi\n          withr\n          httr;\n      } ++ [ highlite gh memoise ];\n    });\n" (`expected`).

lines(actual)[3:9] vs lines(expected)[3:9]
  "      name = \"httr2\";"
  "      src = pkgs.fetchgit {"
  "        url = \"[https://github.com/b-rodrigues/httr2\](https://github.com/b-rodrigues/httr2/)";"
- "        rev = \"15243331d3f6192e1a2c210b2959d6fec63402c4\";"
+ "        rev = \"HEAD\";"
  "        sha256 = \"sha256-ny4J2WqUL4LPLWRKS8rgVqwvgMOQ2Rm/lbBWtF+99PE=\";"
  "      };"
  "      propagatedBuildInputs = builtins.attrValues {"

lines(actual)[46:52] vs lines(expected)[46:52]
  "      name = \"highlite\";"
  "      src = pkgs.fetchgit {"
  "        url = \"[https://github.com/jimhester/highlite\](https://github.com/jimhester/highlite/)";"
- "        rev = \"767b122ef47a60a01e1707e4093cf3635a99c86b\";"
+ "        rev = \"HEAD\";"
  "        sha256 = \"sha256-lkWMlAi75MYxiBUYnLwxLK9ApXkWanA4Mt7g4qtLpxM=\";"
  "      };"
  "      propagatedBuildInputs = builtins.attrValues {"

Failure ('test-renv_helpers.R:227:5'): Testing `renv2nix()` on actual renv.lock files
Snapshot of `save_renv2nix_test(...)` to 'renv_helpers/default_datathin.nix' has changed
Run `testthat::snapshot_review('renv_helpers/')` to review changes

Error ('test-rix.R:417:3'): rix(), frozen-edge
Error: Request `curl::curl_fetch_memory(url = 'https://api.github.com/repos/rstats-on-nix/nixpkgs/commits?sha=r-daily')` failed:
 HTTP response code said error [api.github.com]: The requested URL returned error: 403
Backtrace:
    x
 1. \-rix:::get_right_commit("frozen-edge") at test-rix.R:417:3
 2.   \-rix:::try_get_request(url = api_url, handle = h) at rix/R/get_latest.R:88:3
 3.     \-base::tryCatch(...) at rix/R/get_latest.R:107:3
 4.       \-base (local) tryCatchList(expr, classes, parentenv, handlers)
 5.         \-base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 6.           \-value[[3L]](cond)

Error ('test-rix.R:376:7'): rix(), frozen-edge
Error in `eval(code, test_env)`: object 'frozen_edge_commit' not found
Backtrace:
    x
 1. +-base::system(paste0("sed -i 's/", frozen_edge_commit, "/REVISION/' _snaps/rix/frozen-edge_default.nix")) at test-rix.R:376:7
 2. \-base::paste0("sed -i 's/", frozen_edge_commit, "/REVISION/' _snaps/rix/frozen-edge_default.nix")

[ FAIL 4 | WARN 0 | SKIP 2 | PASS 68 ]

Now, 2 failes tests, not caused by this PR:

-- Failed tests ----------------------------------------------------------------
Failure ('test-fetchers.R:127:3'): Test fetchgit gets a package with several remote deps and commits
fetchgit(...) (`actual`) not equal to "\n    httr2 = (pkgs.rPackages.buildRPackage {\n      name = \"httr2\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/b-rodrigues/httr2\](https://github.com/b-rodrigues/httr2/)";\n        rev = \"HEAD\";\n        sha256 = \"sha256-ny4J2WqUL4LPLWRKS8rgVqwvgMOQ2Rm/lbBWtF+99PE=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          cli\n          curl\n          glue\n          lifecycle\n          magrittr\n          openssl\n          R6\n          rappdirs\n          rlang\n          vctrs\n          withr;\n      };\n    });\n\n    gh = (pkgs.rPackages.buildRPackage {\n      name = \"gh\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/b-rodrigues/gh\](https://github.com/b-rodrigues/gh/)";\n        rev = \"HEAD\";\n        sha256 = \"sha256-POXEMZv8kqHokAxK8LoWkS0qYrcIcVdQi5xyGD992KU=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          cli\n          gitcreds\n          glue\n          ini\n          jsonlite\n          lifecycle\n          rlang;\n      } ++ [ httr2 ];\n    });\n\n\n    highlite = (pkgs.rPackages.buildRPackage {\n      name = \"highlite\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/jimhester/highlite\](https://github.com/jimhester/highlite/)";\n        rev = \"HEAD\";\n        sha256 = \"sha256-lkWMlAi75MYxiBUYnLwxLK9ApXkWanA4Mt7g4qtLpxM=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          Rcpp\n          BH;\n      };\n    });\n\n\n    memoise = (pkgs.rPackages.buildRPackage {\n      name = \"memoise\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/b-rodrigues/memoise\](https://github.com/b-rodrigues/memoise/)";\n        rev = \"74d62c8\";\n        sha256 = \"sha256-fsdop66VglkOIYrJ0LKZKikIZmzQ2gqEATLy9tTJ/B8=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          digest;\n      };\n    });\n\n    lookup = (pkgs.rPackages.buildRPackage {\n      name = \"lookup\";\n      src = pkgs.fetchgit {\n        url = \"[https://github.com/b-rodrigues/lookup/\](https://github.com/b-rodrigues/lookup//)";\n        rev = \"ee5505c817b19b59d37236ed35a81a65aa376124\";\n        sha256 = \"sha256-jiSBuC1vzJbN6OckgVX0E+XuMCeZS5LKsldIVL7DNgo=\";\n      };\n      propagatedBuildInputs = builtins.attrValues {\n        inherit (pkgs.rPackages) \n          Rcpp\n          codetools\n          crayon\n          rex\n          jsonlite\n          rstudioapi\n          withr\n          httr;\n      } ++ [ highlite gh memoise ];\n    });\n" (`expected`).

lines(actual)[3:9] vs lines(expected)[3:9]
  "      name = \"httr2\";"
  "      src = pkgs.fetchgit {"
  "        url = \"[https://github.com/b-rodrigues/httr2\](https://github.com/b-rodrigues/httr2/)";"
- "        rev = \"15243331d3f6192e1a2c210b2959d6fec63402c4\";"
+ "        rev = \"HEAD\";"
  "        sha256 = \"sha256-ny4J2WqUL4LPLWRKS8rgVqwvgMOQ2Rm/lbBWtF+99PE=\";"
  "      };"
  "      propagatedBuildInputs = builtins.attrValues {"

lines(actual)[46:52] vs lines(expected)[46:52]
  "      name = \"highlite\";"
  "      src = pkgs.fetchgit {"
  "        url = \"[https://github.com/jimhester/highlite\](https://github.com/jimhester/highlite/)";"
- "        rev = \"767b122ef47a60a01e1707e4093cf3635a99c86b\";"
+ "        rev = \"HEAD\";"
  "        sha256 = \"sha256-lkWMlAi75MYxiBUYnLwxLK9ApXkWanA4Mt7g4qtLpxM=\";"
  "      };"
  "      propagatedBuildInputs = builtins.attrValues {"

Failure ('test-renv_helpers.R:227:5'): Testing `renv2nix()` on actual renv.lock files
Snapshot of `save_renv2nix_test(...)` to 'renv_helpers/default_datathin.nix' has changed
Run `testthat::snapshot_review('renv_helpers/')` to review changes

[ FAIL 2 | WARN 0 | SKIP 2 | PASS 79 ]

[mihem]

I also added tests for download_all_commits function that seem to work. Thereby, detected a bug in download_all_commits (was not having 100 not 300 commits ,, so yeah testing is useful :)) and tried to increase performance by pre-allocation (with the help of Clause 3.5), although most time is spend on the API request I think.

[b-rodrigues]

These tests are failing as expected, as the packages were pointing to HEAD, but now to the latest commit. Also, we need a failsafe for the case that was happening in CI: if no PAT is configured and the api can't be accessed, then the user should be informed

[b-rodrigues]

These tests are failing as expected, as the packages were pointing to HEAD, but now to the latest commit. Also, we need a failsafe for the case that was happening in CI: if no PAT is configured and the api can't be accessed, then the user should be informed

Actually no, they're now pointing to HEAD whereas they weren't before. So I guess some failsafe is kicking in where the commit is not being used and overwritten with HEAD

[b-rodrigues]

Also if tests are green on main but not in a PR, then it's for sure related to the PR, even if it doesn't look like it 😉

side effects are a bitch!

[mihem]

Well if not PAT is configured, it will still work, but now as an unauthorized request with 50 requests per hour

And there's the message

    message("No GitHub Personal Access Token found. Please set GITHUB_PAT in your environment. Falling back to unauthenticated API request.")

[mihem]

well but all tests (except lintr) are green. But if you open them you can still see tests failing.

[mihem]

But okay, you are right, there are not failing tests in main.
But there seems to be some problem with the workflows as 13/14 checks are successful, although tests are failing.

[b-rodrigues]

well but all tests (except lintr) are green. But if you open them you can still see tests failing.

yes there's something wrong with GA not reporting testthat failures for some reason. But tests all pass on main see

[mihem]

also added test for resolve_package_commit, now everything should be tested

[mihem]

ahh, i now get why they are failing.
Before you alwayed used HEAD. But this is what this PR is about. Try to get the commit of the same date or earlier.
So this is actually expected, I think, great!
That means we just need to adjust the tests!

[mihem]

Now all tests pass 💯

[ FAIL 0 | WARN 0 | SKIP 2 | PASS 86 ]

What I did:
I updated the rev from HEAD (which it was previously) to the hash of the matching date in test-fetchers.R
I updated the default_datathin.nix. This now also uses a commit hash instead of HEAD (actually this commit hash is the HEAD right now, but as soon as datathin gets a commit it's not)

[mihem]

@b-rodrigues rebasing was also a hassle, I hope I didn t mess anything up.
I manually checked the code and it looked okay but better double check

And no tests fail :)

[b-rodrigues]

Many thanks, this is really a great addition to the package!

[mihem]

This is great news. You are welcome.
Agree, at least in my use case (which involved packages likes Seurat wrappers with remote dependencies) this is a very convenient addition. And this has been my most complex PR so far :).
I think this also needs to be mentioned in docs, happy to help if you want, although you are great at docs writing.

[b-rodrigues]

I'm in the process of drafting something

[b-rodrigues]

what do you think https://docs.ropensci.org/rix/articles/z-advanced-topic-handling-packages-with-remote-dependencies.html

[mihem]

Yes pretty good, some suggestions here #408