Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cran_revdeps_versions internal bugged #340

Open
maksymiuks opened this issue Sep 28, 2022 · 1 comment
Open

cran_revdeps_versions internal bugged #340

maksymiuks opened this issue Sep 28, 2022 · 1 comment

Comments

@maksymiuks
Copy link
Contributor

maksymiuks commented Sep 28, 2022

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
Copy link
Contributor Author

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

gaborcsardi added a commit that referenced this issue May 10, 2024
Add cran parameter (#333 and #340 candidate fix)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant