From 98ac2876013868a4bfc0bbdc316c80787f35b020 Mon Sep 17 00:00:00 2001 From: Mick Cooney Date: Thu, 1 Oct 2026 01:51:09 +0100 Subject: [PATCH] fix(r): skip renv, packrat, revdep and .Rcheck trees in package discovery Installed libraries and check output hold one DESCRIPTION per third-party package. Discovery treated them as repository content: a renv project library below the depth limit flagged discovery as truncated, which made every pkg::fn() qualifier unknown, and a shallow copy became a local package with no parsed files. The main walk and the hidden-package scan now share one predicate that skips a directory whose name is exactly renv, packrat or revdep, or ends in .Rcheck after at least one character. Look-alike names are still searched. --- .../ingestion/languages/r/package-config.ts | 34 +++- .../unit/r-package-discovery-vendored.test.ts | 154 ++++++++++++++++++ 2 files changed, 184 insertions(+), 4 deletions(-) create mode 100644 gitnexus/test/unit/r-package-discovery-vendored.test.ts diff --git a/gitnexus/src/core/ingestion/languages/r/package-config.ts b/gitnexus/src/core/ingestion/languages/r/package-config.ts index b760e100f..3f4bc2bd9 100644 --- a/gitnexus/src/core/ingestion/languages/r/package-config.ts +++ b/gitnexus/src/core/ingestion/languages/r/package-config.ts @@ -25,7 +25,9 @@ export interface RPackageConfig { * and, among equal depths, the one whose whole path string sorts first by * code unit (so `a-b/x` beats `a/x`: `-` sorts before `/`); the others are * not entries here (their NAMESPACE is still recorded in - * {@link namespaceInfoByPackageDir}, which is keyed by directory). + * {@link namespaceInfoByPackageDir}, which is keyed by directory). Installed + * libraries and check output (`renv`, `packrat`, `revdep`, `.Rcheck`) are + * not searched, so their packages are not entries here. */ packages: Map; /** Package-scoped NAMESPACE config keyed by package dir relative to repo root. */ @@ -113,6 +115,30 @@ export function reportRExportPatternProblems(config: RPackageConfig): void { /** Directories never descended into, by the main walk and the hidden-package scan alike. */ const SKIPPED_DIR_NAMES: ReadonlySet = new Set(['node_modules', '.git', '.Rproj.user']); +/** + * Directories R tooling fills with installed packages or check output: the renv project + * library, the packrat private library and the revdepcheck workspace. Each holds one + * `DESCRIPTION` per third-party package, none of which is part of the repository. + */ +const VENDORED_LIBRARY_DIR_NAMES: ReadonlySet = new Set(['renv', 'packrat', 'revdep']); + +/** Suffix of the `.Rcheck` directories `R CMD check` writes. */ +const RCHECK_SUFFIX = '.Rcheck'; + +/** + * True for a directory that discovery must not enter. The match is on the directory's own + * name, exact and case-sensitive: `renv`, `packrat` or `revdep`, or `.Rcheck` + * (a name that is only the suffix is not one). Look-alikes such as `renv-tools`, `Renv`, + * `revdeps` or `pkg.rcheck` are ordinary directories and may hold a real package. + */ +function isSkippedDirName(name: string): boolean { + return ( + SKIPPED_DIR_NAMES.has(name) || + VENDORED_LIBRARY_DIR_NAMES.has(name) || + (name.length > RCHECK_SUFFIX.length && name.endsWith(RCHECK_SUFFIX)) + ); +} + /** * Directory entries in code-unit name order. `readdir` order is filesystem-dependent * (and `localeCompare` is locale-dependent), so every walk below reads through this to @@ -141,7 +167,7 @@ const HIDDEN_SCAN_BUDGET = 5000; * Scan the subtrees the main walk skipped for a package it never discovered. * * Iterative DFS over `roots` under one global `budget` of directories (skipping - * `node_modules`, `.git` and `.Rproj.user`, as the main walk does). Returns + * the directories {@link isSkippedDirName} names, as the main walk does). Returns * `true` (cannot rule a hidden package out) when the budget is exceeded, when a * directory or `DESCRIPTION` cannot be read, or when a `DESCRIPTION` declares a * `Package:` name that is not in `knownPackageNames`. Returns `false` when every @@ -167,7 +193,7 @@ export async function hidesUndiscoveredPackage( } for (const entry of entries) { if (entry.isDirectory()) { - if (!SKIPPED_DIR_NAMES.has(entry.name)) stack.push(path.join(dir, entry.name)); + if (!isSkippedDirName(entry.name)) stack.push(path.join(dir, entry.name)); } else if (entry.isFile() && entry.name === 'DESCRIPTION') { try { const content = await fs.readFile(path.join(dir, entry.name), 'utf-8'); @@ -207,7 +233,7 @@ export async function loadRPackageConfig(repoRoot: string): Promise.Rcheck/`) as repository packages. + * + * Every test builds a real temporary tree. The skip rule is exact: the near-miss + * names below must still be searched, because a real package can live in a + * directory called `renv-tools`. + */ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import { + hidesUndiscoveredPackage, + loadRPackageConfig, +} from '../../src/core/ingestion/languages/r/package-config.js'; +import { rQualifierLocality } from '../../src/core/ingestion/languages/r/qualified-call.js'; + +let tmp: string; +let counter = 0; + +const desc = (name: string): string => `Package: ${name}\nVersion: 1.0\n`; + +async function write(root: string, rel: string, content: string): Promise { + const full = path.join(root, rel); + await fs.mkdir(path.dirname(full), { recursive: true }); + await fs.writeFile(full, content); +} + +async function newRepo(): Promise { + const root = path.join(tmp, `repo${counter++}`); + await fs.mkdir(root, { recursive: true }); + return root; +} + +/** A repository whose own package is `mypkg`, at the root. */ +async function rootPackageRepo(): Promise { + const root = await newRepo(); + await write(root, 'DESCRIPTION', desc('mypkg')); + await write(root, 'NAMESPACE', 'export(f)\n'); + return root; +} + +/** Directory names the rule skips, as a (name, a path inside it that holds a package) pair. */ +const VENDORED: readonly (readonly [label: string, dir: string])[] = [ + ['renv', 'renv/library/dplyr'], + ['packrat', 'packrat/lib/dplyr'], + ['revdep', 'revdep/library/dplyr'], + ['.Rcheck', 'mypkg.Rcheck/00_pkg_src/dplyr'], +]; + +/** Names that resemble the rule but must not be skipped. */ +const NEAR_MISSES: readonly string[] = [ + 'renv-tools', + 'Renv', + 'RENV', + 'renv2', + 'packrat_old', + 'revdeps', + 'Rcheck', + 'pkg.rcheck', + 'pkg.Rcheck.bak', + '.Rcheck', +]; + +describe('R package discovery skips vendored and generated trees', () => { + beforeAll(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-r-vendored-')); + }); + afterAll(async () => { + await fs.rm(tmp, { recursive: true, force: true }); + }); + + it('a renv library below the depth limit neither adds a package nor truncates discovery', async () => { + const root = await rootPackageRepo(); + await write(root, 'renv/library/x86_64-pc-linux-gnu/4.3/dplyr/DESCRIPTION', desc('dplyr')); + const cfg = await loadRPackageConfig(root); + expect([...cfg.packages.keys()]).toEqual(['mypkg']); + expect(cfg.truncated).toBe(false); + }); + + describe.each(VENDORED)('%s directory', (_label, pkgDir) => { + it('is not searched for packages within the depth limit', async () => { + const root = await rootPackageRepo(); + await write(root, `${pkgDir}/DESCRIPTION`, desc('dplyr')); + await write(root, `${pkgDir}/NAMESPACE`, 'export(filter)\n'); + const cfg = await loadRPackageConfig(root); + expect(cfg.packages.has('dplyr')).toBe(false); + expect([...cfg.namespaceInfoByPackageDir.keys()]).toEqual(['']); + expect(cfg.truncated).toBe(false); + }); + + it('is skipped when nested below another directory', async () => { + const root = await rootPackageRepo(); + await write(root, `analysis/${pkgDir}/DESCRIPTION`, desc('dplyr')); + const cfg = await loadRPackageConfig(root); + expect([...cfg.packages.keys()]).toEqual(['mypkg']); + expect(cfg.truncated).toBe(false); + }); + + it('does not truncate discovery when it lies beyond the depth limit', async () => { + const root = await rootPackageRepo(); + await write(root, `a/b/c/d/${pkgDir}/DESCRIPTION`, desc('dplyr')); + const cfg = await loadRPackageConfig(root); + expect([...cfg.packages.keys()]).toEqual(['mypkg']); + expect(cfg.truncated).toBe(false); + }); + + it('is not entered by the hidden-package scan', async () => { + const root = await newRepo(); + await write(root, `${pkgDir}/DESCRIPTION`, desc('dplyr')); + expect(await hidesUndiscoveredPackage([root], new Set())).toBe(false); + }); + }); + + it('a genuine package beside a vendored tree is still discovered and not displaced by a same-named copy inside it', async () => { + const root = await rootPackageRepo(); + await write(root, 'packages/dplyr/DESCRIPTION', desc('dplyr')); + await write(root, 'packages/dplyr/NAMESPACE', 'export(filter)\n'); + for (const [, dir] of VENDORED) await write(root, `${dir}/DESCRIPTION`, desc('dplyr')); + const cfg = await loadRPackageConfig(root); + expect([...cfg.packages].sort()).toEqual([ + ['dplyr', 'packages/dplyr'], + ['mypkg', ''], + ]); + expect([...cfg.namespaceInfoByPackageDir.keys()].sort()).toEqual(['', 'packages/dplyr']); + expect(cfg.truncated).toBe(false); + }); + + it('an installed library package is external for qualifier resolution', async () => { + const root = await rootPackageRepo(); + await write(root, 'renv/library/x86_64-pc-linux-gnu/4.3/dplyr/DESCRIPTION', desc('dplyr')); + const cfg = await loadRPackageConfig(root); + expect(rQualifierLocality('dplyr', cfg, new Set())).toBe('external'); + expect(rQualifierLocality('mypkg', cfg, new Set())).toBe('local'); + }); + + describe.each(NEAR_MISSES)('near-miss name %s', (name) => { + it('is still searched for packages', async () => { + const root = await rootPackageRepo(); + await write(root, `${name}/DESCRIPTION`, desc('real')); + await write(root, `${name}/NAMESPACE`, 'export(g)\n'); + const cfg = await loadRPackageConfig(root); + expect(cfg.packages.get('real')).toBe(name); + expect(cfg.namespaceInfoByPackageDir.has(name)).toBe(true); + }); + + it('is still searched by the hidden-package scan', async () => { + const root = await newRepo(); + await write(root, `${name}/DESCRIPTION`, desc('real')); + expect(await hidesUndiscoveredPackage([root], new Set())).toBe(true); + }); + }); +});