Avoid failing to install git remotes when packfile parsing fails for unnecessary files

[dgkf]

Following on the comment in #354 (comment), this records a listing of paths in a git remote that failed to be parsed out of the packfile. Notably, this will allow for gracefully recovering from failing to clone submodules in cases where they are not required for the package to build, though it would apply in other instances as well.

Instead of failing immediately, this gives the opportunity to determine the severity of failing to parse the file tree out of the packfile and only throw an error if a file is necessary for the package to build.

This also implements a check for severity, disregarding files outside a remote subdir if one is provided, or files ignored in .Rbuildignore.

[gaborcsardi]

Thanks! Do you have some example repos we can use for testing?

[dgkf]

I do, but it's a private enterprise repo that I was using for testing. I only manually tested the case where it should succeed, but don't have a counter-example that should still emit a warning/error.

I don't mind setting them up, but I think it be easier to maintain if the testing repos were also under r-lib (or at least an org that shares a lot of org owners). For the time being, I can spin something up under my username, but I would prefer if they were transferred into r-lib or similar so that I don't accidentally break test cases some years down the road if I'm deleting stale work.

If that sounds reasonable to you, I can kick off some repos. I would make two with the following structures:

Negative Test Case

Should fail to install, as necessary submodule will not be parsed out of the packfile

Update: repo created

├── DESCRIPTION
├── inst
│   └── r-lib/generics @ 46fd716            # submodule, necessary for build
├── man
│   └── hello_world.md
├── NAMESPACE
└── R
    └── hello_world.R

Positive Test Case

Should successfully install as submodules are either outside the package subdir, or excluded via .Rbuildignore

Update: repo created

├── generics => r-lib/generics@46fd716      # submodule, unnecessary, outside subdir
└── hello
    ├── generics => r-lib/generics@46fd716  # submodule, unnecessary, excluded via .Rbuildignore
    ├── .Rbuildignore                       # ^generics$
    ├── DESCRIPTION
    ├── man
    │   └── hello_world.md
    ├── NAMESPACE
    └── R
        └── hello_world.R

[dgkf]

I added a "test" file in the tests/testthat/fixtures directory to rebuild the relevant packfiles. Happy to retool this if you have a different style in mind, I just wanted to make sure the process for generating some testing fixtures was reproducible.

[dgkf]

In the initial PR this was originally a warning, but if files are required for properly building the package I felt it should probably be an error.

Ideally we could emit a warning and let R continue and fail when it tries to build the package, but R doesn't have any way to know that the files are missing so it would just happily build a faulty package. I think this would be a worse failure mode so I converted it to an error.

[gaborcsardi]

@dgkf I have implemented git submodules now. Is there any other reason for files to be missing?

[dgkf]

@dgkf I have implemented git submodules now.

🎉 🎉

Sounds great! Thanks for investing what it would take to implement submodules back when I started this and it is no small task, so thank you for taking that on!

Is there any other reason for files to be missing?

Submodules are the only use case I'm aware of. I just left it ambiguous since it would work in other situations as well.

From what I can tell, your submodules implementation should address these needs as well, so feel free to close.