From e34409b5a1326ad9cbd2e09050fe67ff565c979d Mon Sep 17 00:00:00 2001 From: zackverham <96081108+zackverham@users.noreply.github.com> Date: Thu, 5 Mar 2026 15:19:17 -0500 Subject: [PATCH] Fix R functional test failures on Windows ARM runners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a CRAN-like package's installed version (from a binary repo like P3M/RSPM) is newer than what available.packages(type="source") returns, the isDevVersion check was incorrectly blanking out Source and Repository, triggering renvPackageSourceMissing. This is a false positive — the package came from a known repository; the version difference is just binary-vs-source repo timing. Remove the isDevVersion branch for CRAN-like packages so their lockfile repository info is always preserved. Also remove the now-unused isDevVersion, findAvailableVersion, and package_version functions. Co-Authored-By: Claude Opus 4.6 --- .../dependencies/renv/manifest_packages.go | 47 +------------------ .../renv/manifest_packages_test.go | 16 ++++--- 2 files changed, 12 insertions(+), 51 deletions(-) diff --git a/internal/inspect/dependencies/renv/manifest_packages.go b/internal/inspect/dependencies/renv/manifest_packages.go index 7a736fb1bf..f6e3718a41 100644 --- a/internal/inspect/dependencies/renv/manifest_packages.go +++ b/internal/inspect/dependencies/renv/manifest_packages.go @@ -8,7 +8,6 @@ import ( "io/fs" "os" "slices" - "strconv" "strings" "github.com/posit-dev/publisher/internal/bundles" @@ -55,15 +54,6 @@ func NewPackageMapper(base util.AbsolutePath, rExecutable util.Path, log logging }, err } -func findAvailableVersion(pkgName PackageName, availablePackages []AvailablePackage) string { - for _, avail := range availablePackages { - if avail.Name == pkgName { - return avail.Version - } - } - return "" -} - // isCRANLike returns true if the repository name is CRAN or a Posit Package // Manager variant (RSPM, PPM, P3M). These are all CRAN mirrors and should be // treated equivalently for source validation purposes. @@ -76,34 +66,6 @@ func isCRANLike(repo RepoURL) bool { } } -func package_version(vs string) []int { - // https://www.rdocumentation.org/packages/base/versions/3.6.2/topics/numeric_version - // "Numeric versions are sequences of one or more non-negative integers, - // usually represented as character strings with the elements of the sequence - // concatenated and separated by single . or - characters" - parts := strings.FieldsFunc(vs, func(c rune) bool { - return c < '0' || c > '9' - }) - values := []int{} - for _, part := range parts { - // There shouldn't be any invalid parts because we only took digits - v, _ := strconv.Atoi(part) - values = append(values, v) - } - return values -} - -func isDevVersion(pkg *Package, availablePackages []AvailablePackage) bool { - // A package is a dev version if it's newer than the one - // available in the configured repositories. - repoVersion := findAvailableVersion(pkg.Package, availablePackages) - if repoVersion == "" { - return false - } - cmp := slices.Compare(package_version(pkg.Version), package_version(repoVersion)) - return cmp > 0 -} - func findRepoNameByURL(repoUrl RepoURL, repos []Repository) string { for _, repo := range repos { if repo.URL == repoUrl { @@ -142,13 +104,8 @@ func toManifestPackage(pkg *Package, repos []Repository, availablePackages, bioc // treated equivalently. See renv's lockfile-write.R which // treats changes between these as spurious. if isCRANLike(pkg.Repository) { - if isDevVersion(pkg, availablePackages) { - out.Source = "" - out.Repository = "" - } else { - out.Source = string(pkg.Repository) - out.Repository = findRepoUrl(pkg.Package, availablePackages) - } + out.Source = string(pkg.Repository) + out.Repository = findRepoUrl(pkg.Package, availablePackages) } else { // Repository comes from DESCRIPTION and is set by repo, so can be // anything. So we must look up from the package name. diff --git a/internal/inspect/dependencies/renv/manifest_packages_test.go b/internal/inspect/dependencies/renv/manifest_packages_test.go index 17a753b7c8..b148942af3 100644 --- a/internal/inspect/dependencies/renv/manifest_packages_test.go +++ b/internal/inspect/dependencies/renv/manifest_packages_test.go @@ -206,6 +206,11 @@ func (s *ManifestPackagesSuite) TestVersionMismatch() { } func (s *ManifestPackagesSuite) TestDevVersion() { + // When a CRAN-like package's installed version is newer than what + // available.packages(type="source") returns, the package should still + // be accepted with its lockfile repository info preserved. This happens + // on binary-only repos (e.g. P3M/RSPM on Windows ARM) where the binary + // version leads the source version. base := s.testdata.Join("dev_version") lockfilePath := base.Join("renv.lock") libPath := base.Join("renv_library") @@ -226,13 +231,12 @@ func (s *ManifestPackagesSuite) TestDevVersion() { mapper.(*defaultPackageMapper).lister = lister manifestPackages, err := mapper.GetManifestPackages(base, lockfilePath, logging.New()) - s.NotNil(err) - s.Nil(manifestPackages) + s.NoError(err) + s.NotNil(manifestPackages) - aerr, isAgentErr := types.IsAgentError(err) - s.Equal(isAgentErr, true) - s.Equal(aerr.Code, types.ErrorRenvPackageSourceMissing) - s.Equal(aerr.Message, "Cannot re-install packages installed from source; all packages must be installed from a reproducible location such as a repository. Package mypkg, Version 1.2.3.") + pkg := manifestPackages["mypkg"] + s.Equal("CRAN", pkg.Source) + s.Equal("https://cran.rstudio.com", pkg.Repository) } func (s *ManifestPackagesSuite) TestMissingDescriptionFile() {