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

Jump to bottom

install_git() Assumes github-style urls and credential-less web server access #625

Closed
dgkf opened this issue Jun 14, 2021 · 3 comments
Closed

install_git() Assumes github-style urls and credential-less web server access #625

dgkf opened this issue Jun 14, 2021 · 3 comments

Comments

@dgkf
Copy link
Contributor

@dgkf dgkf commented Jun 14, 2021 •
edited

Despite being able to clone a repository in the command line, I was running into errors using install_git to install dependencies. After some investigation, I found a few assumptions that might contribute to errors:

remotes/R/install-git.R

Lines 148 to 151 in a14cf00

if (grepl("^https?://", remote$url)) {
url <- build_url(sub("\\.git$", "", remote$url), "raw", remote_sha(remote, ...), description_path)
download(tmp, url)
read_dcf(tmp)$Package

These lines in remote_package_name.git2r_remote assume a github-style raw file content url without a safe fallback. For example, where GitHub will use .../raw/<ref>/<path>, GitLab will use .../-/raw/<ref>/<path>. Edit: after further testing, the GitHub style is totally fine for GitLab.

Likewise, this assumes that one has unauthenticated access to these web pages. At least in my org, I run into the error below because read.dcf is trying to parse the log-in page that unauthenticated access redirects to.

Error: Failed to install 'unknown package' from Git:
  Line starting '<!DOCTYPE html> ...' is malformed!

Would it be possible to fall back to return NA or fall back to a method that only assumes a basic git functionality when such assumptions cause errors?
@jimhester
Copy link
Member

@jimhester jimhester commented Jun 15, 2021

That code is from #603, you might want to talk to @niheaven about supporting more services but generally I don't think it will be possible to support everything.

@dgkf
Copy link
Contributor Author

@dgkf dgkf commented Jun 15, 2021 •
edited

Thanks @jimhester (& thanks @niheaven for your work on this already - it's generally worked perfectly with only this slight hiccup!)

@niheaven - would it be okay to add such a fallback?

if (grepl("^https?://", remote$url)) {
+  tryCatch({
    url <- build_url(sub("\\.git$", "", remote$url), "raw", remote_sha(remote, ...), description_path)
    download(tmp, url)
    read_dcf(tmp)$Package
+  }, error = function(e) NA_character_)
}

Wrapping this in a try block, similar to the git archive method below would provide a safe fallback when the url follows a different structure or requires authentication. I'd be happy to help put together a PR if you'd like. PR now submitted.

@dgkf dgkf mentioned this issue Jun 16, 2021
Merged
@niheaven
Copy link
Contributor

@niheaven niheaven commented Jun 16, 2021

@dgkf 👍 for your PR

@dakep dakep mentioned this issue Jun 20, 2021
Closed
rnorberg added a commit to rnorberg/remotes that referenced this issue Jul 9, 2021
@rnorberg
remote_package_name.git2r_remote uses (some) credentials
4d48a28 
Related to r-lib#632 and r-lib#625. Now, when trying to download a package's `DESCRIPTION` file from a repo that requires authentication, some types of credentials (`git2r::cred_user_pass`, `git2r::cred_env`, and `git2r::cred_token`) are passed to the (internal) `download` function.
@rnorberg rnorberg mentioned this issue Jul 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jimhester @niheaven @dgkf