remotes icon indicating copy to clipboard operation
remotes copied to clipboard

allowing git to passthrough for a git external remote as compared to …

Open muschellij2 opened this issue 2 years ago • 17 comments

This passes the git argument all the way through on the install_remotes so that if you have any remote dependencies with a prefix of git:: and you set git = "external" on an install* (my use case of using SSH keys and no GITHUB_PAT).

Here's a reprex that shows the use case. I can turn into an issue if that is better for tracking.

library(remotes)
library(desc)
tfile = tempfile()
dir.create(tfile)
desc_file = file.path(tfile, "DESCRIPTION")
desc = desc::description$new("!new")
desc$set("Package", "temporary")
desc$del("Maintainer")
desc$set_dep(package = "stringr", type = "Imports", version = ">= 1.0.0")
desc$add_remotes("git::[email protected]:hadley/stringr")
desc$write(desc_file)

x = sessioninfo::session_info("stringr")$package
#> Warning in system("timedatectl", intern = TRUE): running command 'timedatectl'
#> had status 1
src = x$source[x$package == "stringr"]
src = sub(".*\\((.*)\\)", "\\1", src)
remove.packages("stringr")
#> Removing package from '/home/jupyter/.R/library'
#> (as 'lib' is unspecified)
remotes::install_local(path = tfile, git = "external")
#> stringr (NA -> 5f48256b5...) [Git]
#> Downloading git repo [email protected]:hadley/stringr
#> '/usr/bin/git' clone --depth 1 --no-hardlinks [email protected]:hadley/stringr /tmp/RtmpkfNJg9/file3f4d5354a09
#> 
#> * checking for file ‘/tmp/RtmpkfNJg9/file3f4d5354a09/DESCRIPTION’ ... OK
#> * preparing ‘stringr’:
#> * checking DESCRIPTION meta-information ... OK
#> * checking for LF line-endings in source and make files and shell scripts
#> * checking for empty or unneeded directories
#> * building ‘stringr_1.4.0.9000.tar.gz’
#> Installing package into '/home/jupyter/.R/library'
#> (as 'lib' is unspecified)
#> Skipping install of 'stringr' from a xgit remote, the SHA1 (5f48256b) has not changed since last install.
#>   Use `force = TRUE` to force installation
#> * checking for file ‘/tmp/RtmpkfNJg9/file3f4d70f9acd3/file3f4d650bbaaa/DESCRIPTION’ ... OK
#> * preparing ‘temporary’:
#> * checking DESCRIPTION meta-information ... OK
#> * checking for LF line-endings in source and make files and shell scripts
#> * checking for empty or unneeded directories
#> * creating default NAMESPACE file
#> * building ‘temporary_1.0.0.tar.gz’
#> Installing package into '/home/jupyter/.R/library'
#> (as 'lib' is unspecified)
remove.packages("temporary")
#> Removing package from '/home/jupyter/.R/library'
#> (as 'lib' is unspecified)
remotes::install_local(path = tfile)
#> Error: Failed to install 'temporary' from local:
#>   Error in 'git2r_remote_ls': error authenticating:
remove.packages("temporary")
#> Removing package from '/home/jupyter/.R/library'
#> (as 'lib' is unspecified)
#> Error in find.package(pkgs, lib): there is no package called 'temporary'
remotes::install_git(src, upgrade = FALSE, git = "external")
#> Downloading git repo [email protected]:tidyverse/stringr.git
#> '/usr/bin/git' clone --depth 1 --no-hardlinks [email protected]:tidyverse/stringr.git /tmp/RtmpkfNJg9/file3f4d64e1397c
#> '/usr/bin/git' fetch origin dd909b714b20ff3add2eedb0a1c917ba6938e40e
#> '/usr/bin/git' checkout FETCH_HEAD
#> * checking for file ‘/tmp/RtmpkfNJg9/file3f4d64e1397c/DESCRIPTION’ ... OK
#> * preparing ‘stringr’:
#> * checking DESCRIPTION meta-information ... OK
#> * checking for LF line-endings in source and make files and shell scripts
#> * checking for empty or unneeded directories
#> * building ‘stringr_1.4.0.9000.tar.gz’
#> Installing package into '/home/jupyter/.R/library'
#> (as 'lib' is unspecified)

res = dev_package_deps(pkgdir = tfile)
#> Error in git2r::remote_ls(remote$url, credentials = remote$credentials): Error in 'git2r_remote_ls': error authenticating:
res
#> Error in eval(expr, envir, enclos): object 'res' not found
res = dev_package_deps(pkgdir = tfile, git = "external")
res
#> Needs update -----------------------------
#>  package installed    available    is_cran remote
#>  stringr dd909b714... 5f48256b5... FALSE   Git

Created on 2021-12-14 by the reprex package (v2.0.1)

Session info
sessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value
#>  version  R version 4.1.2 (2021-11-01)
#>  os       Debian GNU/Linux 10 (buster)
#>  system   x86_64, linux-gnu
#>  ui       X11
#>  language (EN)
#>  collate  C.UTF-8
#>  ctype    C.UTF-8
#>  tz       Etc/UTC
#>  date     2021-12-14
#>  pandoc   2.14.0.2 @ /opt/conda/bin/ (via rmarkdown)
#> 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package     * version    date (UTC) lib source
#>  callr         3.7.0      2021-04-20 [2] CRAN (R 4.1.0)
#>  cli           3.1.0.9000 2021-12-07 [1] Github (r-lib/cli@aaf7cf1)
#>  crayon        1.4.2      2021-10-29 [1] CRAN (R 4.1.0)
#>  curl          4.3.2      2021-06-23 [1] CRAN (R 4.1.0)
#>  desc        * 1.4.0      2021-09-28 [1] CRAN (R 4.1.0)
#>  digest        0.6.29     2021-12-01 [1] CRAN (R 4.1.2)
#>  evaluate      0.14       2019-05-28 [2] CRAN (R 4.1.0)
#>  fastmap       1.1.0      2021-01-25 [2] CRAN (R 4.1.0)
#>  fs            1.5.2      2021-12-08 [1] CRAN (R 4.1.2)
#>  git2r         0.29.0     2021-11-22 [1] CRAN (R 4.1.0)
#>  glue          1.5.1      2021-11-30 [1] CRAN (R 4.1.0)
#>  highr         0.9        2021-04-16 [2] CRAN (R 4.1.0)
#>  htmltools     0.5.2      2021-08-25 [1] CRAN (R 4.1.0)
#>  knitr         1.36       2021-09-29 [1] CRAN (R 4.1.0)
#>  lifecycle     1.0.1      2021-09-24 [1] CRAN (R 4.1.0)
#>  magrittr      2.0.1      2020-11-17 [2] CRAN (R 4.1.0)
#>  pkgbuild      1.3.0      2021-12-09 [1] CRAN (R 4.1.2)
#>  prettyunits   1.1.1      2020-01-24 [2] CRAN (R 4.1.0)
#>  processx      3.5.2      2021-04-30 [2] CRAN (R 4.1.0)
#>  ps            1.6.0      2021-02-28 [2] CRAN (R 4.1.0)
#>  R6            2.5.1      2021-08-19 [1] CRAN (R 4.1.0)
#>  remotes     * 2.4.1.9000 2021-12-14 [1] local
#>  reprex        2.0.1      2021-08-05 [1] CRAN (R 4.1.0)
#>  rlang         0.4.12     2021-10-18 [1] CRAN (R 4.1.0)
#>  rmarkdown     2.11       2021-09-14 [1] CRAN (R 4.1.0)
#>  rprojroot     2.0.2      2020-11-15 [2] CRAN (R 4.1.0)
#>  rstudioapi    0.13       2020-11-12 [2] CRAN (R 4.1.0)
#>  sessioninfo   1.2.2      2021-12-06 [1] CRAN (R 4.1.2)
#>  stringi       1.7.6      2021-11-29 [1] CRAN (R 4.1.0)
#>  stringr       1.4.0.9000 2021-12-14 [1] xgit ([email protected]:tidyverse/stringr.git@dd909b7)
#>  vctrs         0.3.8      2021-04-29 [2] CRAN (R 4.1.0)
#>  withr         2.4.3      2021-11-30 [1] CRAN (R 4.1.0)
#>  xfun          0.28       2021-11-04 [1] CRAN (R 4.1.0)
#>  yaml          2.2.1      2020-02-01 [2] CRAN (R 4.1.0)
#> 
#>  [1] /home/jupyter/.R/library
#>  [2] /usr/local/lib/R/site-library
#>  [3] /usr/lib/R/site-library
#>  [4] /usr/lib/R/library
#> 
#> ──────────────────────────────────────────────────────────────────────────────

muschellij2 avatar Dec 14 '21 02:12 muschellij2

Only failure seems to be a windows fail, not due to the code here.

muschellij2 avatar Feb 02 '22 04:02 muschellij2

Only failure seems to be a windows fail, not due to the code here.

muschellij2 avatar Feb 02 '22 04:02 muschellij2

Thanks! I wonder if using an option + env var would make more sense in this case, instead of pushing this argument through everywhere, which is somewhat error prone if we also pass ... around.

gaborcsardi avatar Feb 02 '22 08:02 gaborcsardi

I think that would work better in that case but this seems to pass and didn’t raise any flags, but it’s hard to know if you’ve gone down every … rabbit hole. Without the pass through though, some of my installs fail if I don’t add a PAT in addition to the SSH key.

On Wed, Feb 2, 2022 at 3:37 AM Gábor Csárdi @.***> wrote:

Thanks! I wonder if using an option + env var would make more sense in this case, instead of pushing this argument through everywhere, which is somewhat error prone if we also pass ... around.

— Reply to this email directly, view it on GitHub https://github.com/r-lib/remotes/pull/679#issuecomment-1027700320, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIGPLRJIIMJJQBCBJFDIX3UZDUL5ANCNFSM5J7X2NWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

-- Best, John

muschellij2 avatar Feb 02 '22 13:02 muschellij2

Without the pass through though, some of my installs fail if I don’t add a PAT in addition to the SSH key.

Why is that? If we set the default of the git argument to getOption("remotes.git", "auto") everywhere, that should work, no?

gaborcsardi avatar Feb 02 '22 13:02 gaborcsardi

I think the issue is that if you specify git in getOption, the passthrough may still be needed. The issue is that install_deps has a ... but git is not passed to dev_package_deps. If you specify git in a passthrough to dev_package_deps, it then must pass to extra_deps, which then passes to parse_one_extra. If you create a option for this, it may work, but I'm not sure if that will be sufficient.

muschellij2 avatar Feb 02 '22 14:02 muschellij2

If we use getOption() every time git is an argument, and every time we need to choose between command like git and the other one, I don't see how it would be possible that it is not used? Maybe I am missing something, though.

gaborcsardi avatar Feb 02 '22 14:02 gaborcsardi

I think I've teased this out some more. The getOption solution will work, but can have a mismatch between the option and what executes depending on specifying git in the arguments:

Scenario

You have a DESCRIPTION file for a package A with Remotes: git::[email protected]:USER/REPO. You then install package A using install_git(..., git = "external") and I would like to install USER/REPO using external git as well.

I think I can boil the issue down to this how remotes:::parse_one_extra works by default without a git argument

x = "git::[email protected]:USER/REPO"
remotes:::parse_one_extra(x)
#> $url
#> [1] "[email protected]:USER/REPO"
#> 
#> $subdir
#> NULL
#> 
#> $ref
#> NULL
#> 
#> $credentials
#> NULL
#> 
#> attr(,"class")
#> [1] "git2r_remote" "remote"

We see this is git2r_remote (as git2r is installed):

remotes:::parse_one_extra(x, git = "external")
#> $url
#> [1] "[email protected]:USER/REPO"
#> 
#> $subdir
#> NULL
#> 
#> $ref
#> NULL
#> 
#> attr(,"class")
#> [1] "xgit_remote" "remote"

Created on 2022-02-03 by the reprex package (v2.0.1)

This all happens because remotes:::git_remote takes in git: https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/install-git.R#L72 If you set that option to getOption, I think this behavior would be resolved for me. The only issue with this is that you specify git in install_git and that may not be preserved/the same in remotes:::git_remote (and therefore parse_one_extra) depending on the mismatch of the default option vs. explicit git argument, which I'm not sure is an "issue" vs. just the behavior.

Rabbit Hole

Here I just run down where the pass through of git or ... is working for my proposed solution.

Starting with install_git

Let's say you're using install_git with git = "external": https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/install-git.R#L34. Changing that to getOption("remotes.git", "auto") would be fine, but if I pass in "external" directly, then let's see where it goes.

install_remote

Goes to install_remotes (this doesn't currently pass git): https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/install-git.R#L54, then install_remote: https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/install-remote.R#L91.

(Just a note, not an issue): This calls remote_sha: https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/install-remote.R#L33, but since git = "external" was given, then we know it's a xgit remote. So this preserves git option because of how the remote is created.

install

Then we hit the standard install (again, no git passed from install_remotes): https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/install-remote.R#L73, which passes to install_deps: https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/install.R#L22, which passes to dev_package_deps: https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/install.R#L191 Here, dev_package_deps takes only set arguments (no ...).

dev_package_deps

In dev_package_deps we see how the default git or would affect the installation of the package in the git::[email protected]:USER/REPO Remote.

In dev_package_deps extra_deps is called: https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/deps.R#L145 Now we have a repo, like [email protected]:USER/REPO that is private and only accessible from SSH creds (no PAT): https://github.com/r-lib/remotes/blob/eb15a1eb21cca47ce5601747fd4e033d8606a648/R/deps.R#L589

Now we see without git in parse_one_extra, it will call git_remote and use the default "auto", which forces git2r_remote, which is where we started above. So we can run install_git(..., git = "external"), but depending on getOption("remotes.git", "auto"), we would have different "gits". This isn't an issue really (sometimes you may want to install this git via external git but all deps via git2r, but I'm not sure.

muschellij2 avatar Feb 03 '22 16:02 muschellij2

To summarize the behavior (with the getOption("remotes.git", "auto") proposed solution):

Minimal change required: git_remote takes in getOption("remotes.git", "auto").

Then when you run: install_git(..., git = "external"), the repos passed here will be installed via external git, but the dependencies are installed via git2r (assuming git2r installed).

All scenarios below are assuming git2r is installed

Package uses external and Deps uses external

options(remotes.git = "external")
install_git(..., git = "external")

Package uses git2r and Deps uses external

options(remotes.git = "external")
install_git(..., git = "auto")

Package uses external and Deps uses git2r (current behavior)

options(remotes.git = "auto")
install_git(..., git = "external")

Package git2r and Deps git2r

options(remotes.git = "auto")
install_git(..., git = "auto")

muschellij2 avatar Feb 03 '22 16:02 muschellij2

Let me know which way you'd like me to go with this. I think the passthrough is fine, but may be harder to maintain, but the above issues exist for the getOption proposal.

muschellij2 avatar Feb 09 '22 21:02 muschellij2

Yeah, I agree that this is not ideal. OTOH if you are always using xgit or git2r, then you just need to set the option, no? So I am kind of OK with the dependencies not following the argument.

Here is another idea, though. At the beginning of the functions that take the git argument, we set the option, and it will be valid until on.exit(). This solves our issues I believe. But it does seem like a bit of an over-engineered solution.

gaborcsardi avatar Feb 10 '22 07:02 gaborcsardi

That works as well. Either way, I’m just looking for something that will work with external git only, even if git2r is installed. And I think I’ve found all the pass throughs, but would like a solution that’s easier for you to maintain while keeping functionality. Could be a getOption + on.exit

On Thu, Feb 10, 2022 at 2:28 AM Gábor Csárdi @.***> wrote:

Yeah, I agree that this is not ideal. OTOH if you are always using xgit or git2r, then you just need to set the option, no? So I am kind of OK with the dependencies not following the argument.

Here is another idea, though. At the beginning of the functions that take the git argument, we set the option, and it will be valid until on.exit(). This solves our issues I believe. But it does seem like a bit of an over-engineered solution.

— Reply to this email directly, view it on GitHub https://github.com/r-lib/remotes/pull/679#issuecomment-1034578649, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIGPLWQCF5FT33665T57MDU2NSIVANCNFSM5J7X2NWQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

-- Best, John

muschellij2 avatar Feb 10 '22 12:02 muschellij2

Just seeing where this stands as we're currently relying on my branch for production

muschellij2 avatar Mar 04 '22 15:03 muschellij2

So should we implement this change or implement a remotes.git option? The reason I ask is without this change, we cannot install our private git packages correctly.

muschellij2 avatar Sep 01 '22 14:09 muschellij2

May I ask will this be merged? Because I'm having the same problem here: I want to install pkgA in a private repo, which depends on pkgB in another private repo. The git repo to pkgB is presented in remotes filed of pkgA's DESCRIPTION. In our rstudio server, git2r is installed without ssh feature, so when installing pkgA I'm using

remotes::install_git("ssh://git@url_to_pkgA.git", git = "external")

But when installing the dependency pkgB, remotes uses git2r then fails the installation.

fenguoerbian avatar Sep 12 '23 03:09 fenguoerbian

@fenguoerbian You can probably use pak::pkg_install() instead of remotes.

gaborcsardi avatar Sep 12 '23 06:09 gaborcsardi

@fenguoerbian You can probably use pak::pkg_install() instead of remotes.

Thanks for the advice. Finally I asked the admin to install necessary dependencies and re-install git2r then everything works fine.

fenguoerbian avatar Sep 15 '23 08:09 fenguoerbian