Accommodating custom GitHub URL's #506

ghuiber · 2014-06-24T21:24:35Z

devtools::install_github() might accept an extra parameter, repo_path, that defaults to https://github.com/ but can point to a custom Enterprise GitHub URL.

This default ensures that this feature will change nothing in the way this function is used now, but the new parameter would allow devtools to be used as a way to disseminate packages between R users who are members of closed Enterprise GitHub communities.

Hoping to get install_github() to work with an enterprise GitHub repo with a different path than https://github.com

hadley · 2014-06-25T12:34:34Z

Thanks! Two comments:

I'm not sure repo_path is the right name for that parameter. Maybe github_url?
Maybe it should be a separate function, install_github_enterprise(), which could then pull the github url from an environment variable or R option so you didn't have to set it every time?

cc @cscheid, @hilaryparker

hilaryparker · 2014-06-25T12:43:14Z

Nice!!

I agree that github_url is probably more intuitive than repo_path.

I'm pretty agnostic to it being a separate function vs. an option in install_github(). For my use case it's mostly me just installing my own functions, so I'm not often calling the function to begin with.

hadley · 2014-06-25T12:50:09Z

@hilaryparker btw the way @ghuiber was inspired to start writing packages (and hence this pull request) because of your packages blog post 😄

hilaryparker · 2014-06-25T12:53:29Z

YAY!! It comes full circle.. now he's helping me! :D

tbates · 2014-06-25T14:49:53Z

install_github_enterprise() would be nice with an accompanying
devtools_git_enterprise() function which returned this info when called with no arguments and wrote them to the environment when given a parameter value.

then this would work:

install_github_enterprise(repo="kickass", github_url = devtools_git_enterprise() )

ghuiber · 2014-06-25T16:47:46Z

I like github_url much better than repo_path, so I made the change, recompiled the docs, tested, built, installed, committed and pushed. That was an easy fix.

As to install_github() vs. install_github_enterprise(): what got me to nose around devtools in the first place was my client's wondering whether there was such a thing as install_github_[your company name here](). That's one piece of evidence that people would indeed find a separate function a more convenient place for a custom URL functionality than another argument to install_github(). The latter was just easier for me to implement.

How would I go about writing it with as little code duplication as possible? Add an install_github_enterprise() function definition to my install_github.r as a kind of convenience wrapper? This way my current version of install_github() continues to work with github_url explicitly set, or people can ignore that part altogether and just use install_github_enterprise().

Finally, I have no idea about two unrelated bits

How to do what @tbates suggests
What to make of this Travis CI build fail. I'd like to make this pull request as easy on @hadley as I can, but I'm at a loss. Some shell script seems to be the snag, but what do I have to do with it?

hadley · 2014-06-25T17:04:44Z

src-i386/devtools.c

@@ -0,0 +1,6 @@
+#include <R.h>


Can you please remove these two files?

Deleted src-i386/devtools.c and src-x64/devtools.c.

On Wed, Jun 25, 2014 at 10:04 AM, Hadley Wickham notifications@github.com
wrote:

In src-i386/devtools.c:

@@ -0,0 +1,6 @@
+#include <R.h>

Can you please remove these two files?

—
Reply to this email directly or view it on GitHub
https://github.com/hadley/devtools/pull/506/files#r14198881.

hadley · 2014-06-25T17:06:05Z

I'd make install_github_enterprise() take a github_url argument, and then have install_github() call it with https://github.com. To implement Tim's idea, you'd write a function that guessed the enterprise url (probably from Sys.getenv(), or getOption() or both) and then make that the default argument to github_url.

hadley · 2014-06-25T17:06:26Z

The build looks like it's failing for other reasons - I think because I haven't updated the docs for some previous changes that I made.

Uses helper devtools_git_enterprise()

Also ran test() and document() anew.

ghuiber · 2014-06-26T06:24:51Z

Done. There is now an install_github_enterprise() wrapper for install_github(), and a helper devtools_git_enterprise() that retrieves the GITHUB_URL environment variable as Tim suggested.

ghuiber · 2014-06-27T20:26:24Z

Resolved merge conflict in my install-github.r to accommodate new GitHub API call #474. Travis CI build probably fails for whatever the original reason was 3 days ago.

hadley · 2014-06-27T20:36:53Z

Now it fails for a legitimate reason - check https://travis-ci.org/hadley/devtools/builds/28599041#L963

ghuiber · 2014-06-27T22:56:01Z

Well, one warning still squeaked by, but I don't think it's aimed at me. It has to do with duplicate references to this SVN functionality. See https://travis-ci.org/hadley/devtools/builds/28632916#L954

…ut of github_get_conn().

ghuiber · 2014-07-01T02:20:19Z

I have no idea what's going on here. Travis turns up the same lone warning about one Rd file with duplicated names: svn_path.Rd. I have seen something to do with this in my test() output, shown below, but I also saw some other warnings which I thought would be both more serious and my own fault.

I would appreciate any hints:

<snip>
Warning messages:
1: install_svn.Rd not generated by roxygen2. Skipped. 
2: install_svn_single.Rd not generated by roxygen2. Skipped. 
Testing devtools
Loading devtools
Checks : .
Data : ............................
Dependencies : ..............
DESCRIPTION checks : ..
Compiled DLLs : ................
GitHub : ........................
help : ...................
Imports : .....
Load: collate : ......
Load hooks : ....................................
Metadata : .................
Namespace : .............................
Rtools tests : ....
s4-export : .
s4-unload : ......................
shim : ................
Vignettes : ......................
...
With : ...............

Warning messages:
1: In if (!file.exists(pkg$path, "src")) return(TRUE) :
  the condition has length > 1 and only the first element will be used
2: In if (!file.exists(pkg$path, "src")) return(TRUE) :
  the condition has length > 1 and only the first element will be used
3: In if (!file.exists(pkg$path, "src")) return(TRUE) :
  the condition has length > 1 and only the first element will be used
4: In if (!file.exists(pkg$path, "src")) return(TRUE) :
  the condition has length > 1 and only the first element will be used
5: In if (!file.exists(pkg$path, "src")) return(TRUE) :
  the condition has length > 1 and only the first element will be used
"C:/PROGRA~1/R/bin/x64/R" --vanilla CMD build "C:\Users\ghuiber\Documents\GitHub\devtools" --no-manual  \
  --no-resave-data 

* checking for file 'C:\Users\ghuiber\Documents\GitHub\devtools/DESCRIPTION' ... OK
* preparing 'devtools':
* checking DESCRIPTION meta-information ... OK
* cleaning src
* checking for LF line-endings in source and make files
* checking for empty or unneeded directories
Removed empty directory 'devtools/tests/testthat/testMarkdownVignettes/inst/doc'
Removed empty directory 'devtools/tests/testthat/testMarkdownVignettes/inst'
Removed empty directory 'devtools/tests/testthat/testVignetteExtras/inst/doc'
Removed empty directory 'devtools/tests/testthat/testVignetteExtras/inst'
Removed empty directory 'devtools/tests/testthat/testVignettes/inst/doc'
Removed empty directory 'devtools/tests/testthat/testVignettes/inst'
* building 'devtools_1.5.0.99.tar.gz'

[1] "C:/Users/ghuiber/Documents/GitHub/devtools_1.5.0.99.tar.gz"
Warning message:
In if (!file.exists(pkg$path, "src")) return(TRUE) :
  the condition has length > 1 and only the first element will be used

This is all. Travis is not worried about any of the pkg$path condition length warnings. My local version of install_github_enterprise() works.

Doesn't work yet. Old-school archive/master.zip redirect still does.

…ide.

ghuiber · 2014-08-15T20:22:22Z

All may look well, but it's not. While load_all(); document(); test(); build() went without a hitch and Travis CI build passed too, now public GitHub doesn't work either. My API v3 call to get the archive zip file goes through, which is progress, but upon attempting actual installation I get this:

> library(devtools)
> install_github('ghuiber/syncR',auth_token=Sys.getenv()[['GITHUB_PAT_PUBLIC']])
Installing github repo ghuiber/syncR@master from ghuiber
Downloading syncR.zip from https://api.github.com/repos/ghuiber/syncR/zipball/master
Installing package from C:\Users\ghuiber\AppData\Local\Temp\Rtmp6Rb2qt/syncR.zip
Error in system(full, intern = quiet, ignore.stderr = quiet, ...) : 
  '"internal"' not found

Any ideas? Maybe Rtools rotted again? I re-installed them not a month ago.

ghuiber · 2014-08-15T23:12:25Z

All seems well, but it's not and re-installing Rtools did not fix the problem. This version of devtools will choke on known, working GitHub packages such as Slidify with the same error message:

> install_github("ramnathv/slidify")
Using github PAT from envvar GITHUB_PAT
Installing github repo ramnathv/slidify@master from ramnathv
Downloading slidify.zip from https://api.github.com/repos/ramnathv/slidify/zipball/master
Installing package from C:\Users\ghuiber\AppData\Local\Temp\RtmpQN2X5y/slidify.zip
Error in system(full, intern = quiet, ignore.stderr = quiet, ...) : 
  '"internal"' not found
> install_github("ramnathv/slidify",auth_token=NULL)
Installing github repo ramnathv/slidify@master from ramnathv
Downloading slidify.zip from https://api.github.com/repos/ramnathv/slidify/zipball/master
Installing package from C:\Users\ghuiber\AppData\Local\Temp\RtmpQN2X5y/slidify.zip
Error in system(full, intern = quiet, ignore.stderr = quiet, ...) : 
  '"internal"' not found
>

Edit: this works on the Mac though (OS X Mavericks, R 3.1.1).

The list element param$github_url does not exist so it’s always NULL. The function should check instead the element param$url, which may or may not be NULL.

hadley · 2014-08-20T15:48:12Z

Looks like I broke something in decompress. Will take a look tomorrow when I'm back at a computer

hadley · 2014-08-21T20:35:24Z

Now that the install_github() refactoring is complete, this is really easy for me to implement. I also think now that it doesn't need it's own function.

It's not quite as nice as your implementation (since you have always have to specify host), but that does have the advantage of being more explicit/reproducible.

ghuiber · 2014-08-21T21:20:15Z

Thank you so much! I can't wait for the next CRAN release :)

ghuiber added 2 commits June 23, 2014 18:49

Added param for alternate github repo path

76344c5

Hoping to get install_github() to work with an enterprise GitHub repo with a different path than https://github.com

Fixed defaults for alternate github repo path

e9e19a2

Changed repo_path to github_url

6e38904

hadley reviewed Jun 25, 2014
View reviewed changes

ghuiber added 4 commits June 25, 2014 10:26

Deleted two devtools.c files.

4fa78d3

Added install_github_enterprise()

761c8bd

Uses helper devtools_git_enterprise()

Fixed small typo

de5aea5

Markup fixed, links checked, rebuilt

3b30f91

Also ran test() and document() anew.

hadley mentioned this pull request Jun 26, 2014

use GitHub API to download archive in install_github #474

Merged

ghuiber added 2 commits June 27, 2014 08:06

Fixed my install-github.r, rebuilt.

3a879c0

Deleted the two devtools.c files.

d2e7ee9

Fixed doc for install_github_enterprise()

9adbfd2

ghuiber added 3 commits June 30, 2014 17:57

Updated parameter sets for install_github[_enterprise]() and the outp…

1f9b796

…ut of github_get_conn().

Rebuild docs for install_github[_enterprise]()

aba1e87

Fixed the install_url() call

2123613

ghuiber added 2 commits July 2, 2014 19:47

Fixed has_devel.r to match change by @krlmlr

600ffcd

Merge remote-tracking branch 'upstream/master'

a50212c

ghuiber added 5 commits July 16, 2014 11:05

Set github_url to API v3 redirect

2f99c35

Doesn't work yet. Old-school archive/master.zip redirect still does.

Merge remote-tracking branch 'upstream/master'

2b815b9

Deleted man/svn_path.Rd

c0af286

Merge remote-tracking branch 'upstream/master'

b78b21e

Took another crack at implementing API v3 archive URL on enterprise s…

fe65692

…ide.

Less typing, right URL

60343ce

Fixed url param in github_ref.github_pull()

9f8bbac

The list element param$github_url does not exist so it’s always NULL. The function should check instead the element param$url, which may or may not be NULL.

wch force-pushed the master branch from b498c3e to 697b03c Compare August 21, 2014 16:28

ghuiber added 2 commits August 21, 2014 10:07

Quick fix

3f02765

New Rd file for install_github_enterprise()

2e64563

hadley closed this in 41a57a4 Aug 21, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accommodating custom GitHub URL's #506

Accommodating custom GitHub URL's #506

ghuiber commented Jun 24, 2014

hadley commented Jun 25, 2014

hilaryparker commented Jun 25, 2014

hadley commented Jun 25, 2014

hilaryparker commented Jun 25, 2014

tbates commented Jun 25, 2014

ghuiber commented Jun 25, 2014

hadley Jun 25, 2014

ghuiber Jun 25, 2014

hadley commented Jun 25, 2014

hadley commented Jun 25, 2014

ghuiber commented Jun 26, 2014

ghuiber commented Jun 27, 2014

hadley commented Jun 27, 2014

ghuiber commented Jun 27, 2014

ghuiber commented Jul 1, 2014

ghuiber commented Aug 15, 2014

ghuiber commented Aug 15, 2014

hadley commented Aug 20, 2014

hadley commented Aug 21, 2014

ghuiber commented Aug 21, 2014

Accommodating custom GitHub URL's #506

Accommodating custom GitHub URL's #506

Conversation

ghuiber commented Jun 24, 2014

hadley commented Jun 25, 2014

hilaryparker commented Jun 25, 2014

hadley commented Jun 25, 2014

hilaryparker commented Jun 25, 2014

tbates commented Jun 25, 2014

ghuiber commented Jun 25, 2014

hadley Jun 25, 2014

Choose a reason for hiding this comment

ghuiber Jun 25, 2014

Choose a reason for hiding this comment

hadley commented Jun 25, 2014

hadley commented Jun 25, 2014

ghuiber commented Jun 26, 2014

ghuiber commented Jun 27, 2014

hadley commented Jun 27, 2014

ghuiber commented Jun 27, 2014

ghuiber commented Jul 1, 2014

ghuiber commented Aug 15, 2014

ghuiber commented Aug 15, 2014

hadley commented Aug 20, 2014

hadley commented Aug 21, 2014

ghuiber commented Aug 21, 2014