insertPackage should work with absolute filenames?

[shabbychef]

The following code:

drat::insertPackage('/home/spav/trunk/code/r/staging/foo.plot_0.1.tar.gz',
 repodir='/mnt/NAS/installers/r/drat/')

fails with an error like:

In file.copy(file.path(curwd, file), srcdir, overwrite = TRUE) :
  problem copying /home/spav/myworking/directory/basically/pwd//home/spav/trunk/code/r/staging/foo.plot_0.1.tar.gz  to /mnt/NAS/installers/r/drat//src/contrib/foo.plot_0.1.tar.gz: No such file or directory

I am using drat 0.2.0 with git2r, from the directory /home/spav/myworking/directory/basically/pwd

[eddelbuettel]

Yes indeed it should. I must have busted that when generalizing for git(hub) use. We need to look at srcdir and repodir -- do you have a few spare cycles to look at it?

[shabbychef]

I'll give it a try; my inner perl hacker (ugh) says to rewrite

file.copy(file.path(curwd, file), srcdir, overwrite=TRUE)

as

file.copy(file.path(curwd, file), srcdir, overwrite=TRUE) || file.copy(file, srcdir, overwrite=TRUE)

[shabbychef]

oh, and maybe wrap that all with stopifnot.

[eddelbuettel]

I'd pre-test with

if (!file.exists(file))  {  whine() }

etc

[shabbychef]

There is already a check for the file at the head of the function.

[eddelbuettel]

It's not quite right.

    stopifnot(file.copy(file.path(curwd, file), srcdir, overwrite=TRUE) || 
                        file.copy(file, srcdir, overwrite=TRUE))

If we are in curwd, then both branches are equivalent, so this does not yet help enough. I'll look at it some more on the train home in a few hours.

[gaborcsardi]

It might help if you convert the path to absolute in the first place. E.g.

absolute_path <- function(path) {
  path <- path.expand(path)
  if (substr(path, 1, 1) == "/") {
    path
  } else {
    file.path(getwd(), path)
  }
}

Btw. why can't you just say file.copy(file, srcdir, ...)? I haven't read the complete code, so maybe this is not an option.

[eddelbuettel]

Nice idea, thanks.

It should actually be pretty simple. file.copy() surely takes anything, absolute or relative, but I need a partial path later in the (optional) git2r::add() call, and that is where I went off the road prior to 0.0.2.

[eddelbuettel]

The file.copy(file.path(curwd, file), srcdir) is wrong, and it arose because of the optional setwd() earlier.

[shabbychef]

I'm not sure both "both branches are equivalent":

> file.path(getwd(),'/tmp/foobar.tar.gz')
[1] "/home/spav//tmp/foobar.tar.gz"

but you're right, I forget to fix the git2r::add.

[eddelbuettel]

The stopifnot( ... || ...) is still weird, and I don't like it. We are in curwd, so both are the same. At the most, use explicit tests and stop() calls.

The basename() looks good.

[eddelbuettel]

Ack re your comment of both branches being different. The problem was the thinko related to the earlier setwd(). At that point all we need is file.copy(file, dir) having tested that both file and dir exist.

[eddelbuettel]

This may actually be much less scary than we thought. Can you look at what I just committed? Either via checkout, or by simply pulling the relevant file as is ?

[shabbychef]

LGTM for absolute paths.

Should there be a catch on file.copy returning `FALSE`` (e.g. due to permissions issues)?

[eddelbuettel]

Great, and a) sorry for the earlier runaround and b) yes that is a very good idea. Help page suggest that a boolean is returned to lemme add that.