Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

manage_deps() doesn't respect pinned version #628

Open
andrewGhazi opened this issue Dec 20, 2024 · 9 comments
Open

manage_deps() doesn't respect pinned version #628

andrewGhazi opened this issue Dec 20, 2024 · 9 comments
Assignees
Labels
package cache Related to the {renv} package cache solution 💡 A temporary solution is in the comments

Comments

@andrewGhazi
Copy link

Hi there, I maintain a new module in the Carpentries incubator. I used sandpaper::pin_version("duckdb@1.1.2") in an attempt to pin the dependency on duckdb to version 1.1.2. You can see the updated renv.lock file here. The build process that gets triggered shows the dependency on the right version but then ignores that and tries to install the latest bugged version when setting up the cache to build the site, causing the whole build to fail. The subsequent re-attempts also fail from the same issue.

I believe the offending sandpaper::manage_deps() call in the github workflow is this line. I've tried to understand the source code for both the github action and sandpaper::manage_deps() but there's just too many layers of abstraction for me to identify the source of the error.

Sidenote: The failure from duckdb has something to do with a cleanup script in the latest version of duckdb which is why I'm trying to pin to 1.1.2.

@froggleston
Copy link
Contributor

Hi @andrewGhazi - I'm currently on holiday, and not very knowledgeable in the internals and intricacies of renv, but I'm wondering if it's down to a dependent package pulling in a newer version?

e.g. CuratedAtlasQueryR also depends on duckdb, so I don't know if manage_deps() takes the pin into account if another package also pulls in a newer version...

@andrewGhazi
Copy link
Author

That seems plausible, but it seems like there must be a way to solve it. It's a pretty frustrating situation because I had thought the entire purpose of all this renv/CI/build infrastructure was to be able to set specific dependency versions.

@froggleston
Copy link
Contributor

I agree - I'll be able to take a look at this in the new year most likely, but I'll probably need some community assistance as renv isn't my strong suit.

@froggleston
Copy link
Contributor

Hi @andrewGhazi - I've managed to narrow down the offending part of the script: https://github.com/carpentries/actions/blob/c90283f3bc25fa13d80d56bb6cfc7325ada6d995/setup-lesson-deps/action.yaml#L118

This line will always install all requisite dependencies, regardless of version, so whatever you specify in renv.lock will be overwritten with the latest packages available. I'm thinking about ways to solve this.

Thank you for your patience!

@froggleston
Copy link
Contributor

froggleston commented Jan 13, 2025

Hi again @andrewGhazi - could you try adding an environment variable to your repo? @zkamvar gave us a useful suggestion!

  • add the R_REMOTES_UPGRADE variable to your repo environment variables in your settings menu for your lesson
  • set the value of this variable to never

This should stop remotes::install_deps() upgrading your packages that you've pinned in your lockfile. Could you try this out and feed back if this works OK? We could then add it to the sandpaper documentation!

@froggleston froggleston self-assigned this Jan 13, 2025
@froggleston froggleston added package cache Related to the {renv} package cache solution 💡 A temporary solution is in the comments labels Jan 13, 2025
@andrewGhazi
Copy link
Author

Assuming that I applied that environment variable correctly (the place to apply it is main repo page > Settings tab > Environments > github-pages > Environment variables , right?), it still ignores it. The same failure mode as before, where it acknowledges the correct dependency initially but then tries to pull the latest version from CRAN. One other detail that might be relevant is that there is no cached version to upgrade from at this point (I cleared the cache in an attempt to fix this).

@zkamvar
Copy link
Contributor

zkamvar commented Jan 14, 2025

Assuming that I applied that environment variable correctly (the place to apply it is main repo page > Settings tab > Environments > github-pages > Environment variables , right?), it still ignores it.

That's not quite right. You need to set the environment variable in the workflow directly.

You can add it under line 32: https://github.com/carpentries-incubator/bioc-scrnaseq/actions/runs/12770220416/workflow#L32

    env:
      R_REMOTES_UPGRADE: never

@andrewGhazi
Copy link
Author

It hasn't completed yet but the workflow still seems to be pulling the latest version from CRAN assuming that the entry in the workflow file is correct.

@froggleston
Copy link
Contributor

I found this issue whilst searching for insights: r-lib/remotes#815

And this has me worried - does upgrade = "never" have some undocumented exclusions that it only works with CRAN packages? Or am I misunderstanding?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package cache Related to the {renv} package cache solution 💡 A temporary solution is in the comments
Projects
None yet
Development

No branches or pull requests

3 participants