Make sha work for shallow git copies

[nparley]

Fix fetching the latest git commit so that it also works for shallow git copies (e.g. ropensci/git2r#219). Fixes #1046

[jimhester]

Thanks for looking into the problem and the fix. This actually will happen often in practice because travis does a shallow clone of the repository for every build.

It would be great to have a test for this case, though I would prefer it be of a local git repository rather than a remote one.

Could you also add a note to NEWS.md.

[nparley]

@jimhester writing a test for this is not actually straight forward as git2r does not support creating shallow clones (e.g. ropensci/git2r#129). So the git repo to test against would have to be made another way, for example with the command line, or package a repo inside test?

[nparley]

@jimhester also would the note go under 1.10.0.9000 in the bug fixes, thanks.

[hadley]

Oops, I messed up the headings. Just put it directly under 1.10.0.9000

[jimhester]

@nparley I would make a dummy package with a shallow checkout using command line git and put it in tests/testthat, then use that for testing.

[gaborcsardi]

You'll probably need some trick with the .git file, AFAIR that is ignored/complained about by R CMD build or check. So you might need to put in a git file without a dot, and then copy it to .git in the test, and then remove it in on.exit().

I am not completely sure, but don't be surprised if it happens.

[jimhester]

Good point Gabor, an easy fix is to use a --bare clone, so you don't have any .git directory. We don't actually need any files for this test, just the git objects.

git clone --depth=50 --bare https://github.com/hadley/devtools.git /tmp/devtools
Rscript -e 'r <- git2r::repository("/tmp/devtools")' -e 'git2r::branch_target(git2r::head(r))'
[1] "21d5d9401101dcf2baada3db424e4de0e9c7f869"

[jennybc]

What @gaborcsardi says will definitely happen (.git will be Rbuildignored). I wrestle with this elsewhere and can't necessarily use @jimhester's bare repo solution. You know how you can invert gitignores with !? Is there anything like that but for Rbuildignore? So you can Rbuild-UN-ignore things instead of fiddling with the names? I can't find any evidence of such but hope I am just missing something.

[gaborcsardi]

@jennybc I don't think that you can do this with .Rbuildignore. But you can create a .git or the whole test folder dynamically. Maybe the best is to just zip up the folder and extract it in the test. Quite cumbersome....

[jimhester]

@jennybc You can store .git with other names, see variety of answers at http://stackoverflow.com/questions/505467/can-i-store-the-git-folder-outside-the-files-i-want-tracked

I even wrote a post about this a few years ago (http://www.jimhester.com/blog/2012/11/08/external-git/) although the solution there still has a .git file, so won't help here.

[nparley]

I have added a test for the shallow clone repo and added a shallow bare copy into the testthat directory to test against. CMD check is now giving the note:

Found the following non-portable file paths:
  devtools/tests/testthat/testBareDepthRepo/objects/pack/pack-c4e0f1d1d68408f260cbbf0a533ad5f6bfd5524e.idx
  devtools/tests/testthat/testBareDepthRepo/objects/pack/pack-c4e0f1d1d68408f260cbbf0a533ad5f6bfd5524e.pack

also the test

pkg_sha <- git_sha1(path='testBareDepthRepo')
expect_equal(pkg_sha, '21d5d94011')

assumes something of the path and so might not work in all situations?

[jimhester]

The Found the following non-portable file paths: NOTE occurs because the filename is more than 100 characters long. See (https://stat.ethz.ch/R-manual/R-devel/library/utils/html/tar.html) for details.

nchar("devtools/tests/testthat/testBareDepthRepo/objects/pack/pack-c4e0f1d1d68408f260cbbf0a533ad5f6bfd5524e.idx")
#> [1] 104

renaming the testBareDepthRepo directory to be shorter should fix the NOTE.

testthat tests are run with tests/testthat as the working directory, so that test should be correct.

You can remove all the sample hooks from tests/testthat/testBareDepthRepo/hooks/*, might as well not include them as they won't be used.

[nparley]

OK that's solved the non-portable file paths problem and the sample hooks have been removed.

[hadley]

Can you please merge/rebase and squash?

[nparley]

Done

[hadley]

Thanks!