revdepcheck icon indicating copy to clipboard operation
revdepcheck copied to clipboard

cran_revdeps_versions internal bugged

Open maksymiuks opened this issue 3 years ago • 1 comments

Hi,

today I found out that the cran_revdeps_versions helper that is used by revdep_init is bugged and incorrectly determinates reverse dependencies if there are two packages following the naming convention pkgA and pkgA.something. I'll show it using the admiral cran package as an example.

> revdepcheck:::cran_revdeps_versions("admiral")
       package version
1      admiral   0.8.1
2   admiraldev   0.1.0
3  admiralonco   0.1.0
4       xportr   0.1.0
5 admiral.test   0.3.0

it doesn't take much time to realize that this list is definitely faulty. It contains the package itself (admiral) which obviously should not happen as the package cannot be its own dependency. Further, inspection showcase other problems. Both admiraldev and admiral.test are not reverse dependencies of admiral, quite the contrary, it is admiral to import them.

I've investigated the issue and I believe it comes from the fault regexp in the following line: https://github.com/r-lib/revdepcheck/blob/main/R/deps.R#L23

it is supposed to mach an entire word, but for regexp dot splits the word, meaning \badmiral\b will be matched not only to admiral but also to admiral.test which should not be the case. That's why admiral and admiraldev appeared in the list as they depend on admiral.test which was mistakenly classified as admiral.

This renders revdepcheck of admiral (and similar packages with the same convention) impossible, as the source against we check is removed after it was checked against... admiral itself. Meaning packages checked later cannot find the installation.

The fix could be connected to do enhancement of the previous line https://github.com/r-lib/revdepcheck/blob/main/R/deps.R#L22. If you remove the package name from that 'super-string' it should be easier to write an appropriate regexp, now we kinda fall in a danger zone when the package could match itself.

Hopefully, my explanation is clear. Let me know if I can help you anyhow with solving that.

maksymiuks avatar Sep 28 '22 14:09 maksymiuks

My small investigation and assessment of the situation show that this should work (,| |\\n)(package)(,| |\\n). Although I'm not a regeexp expert so don't take it for granted :). Boundary requirements come from the fact that in the parsed yaml where deps were pasted together, the package name can start (or end) either with ",' (first or the last package in a certain category), blank space, or the \n (if there was a line break). Such regexp should then match all we want

maksymiuks avatar Sep 29 '22 09:09 maksymiuks