Skip to content

Fix R functional test failures on Windows ARM runners #3660

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.

Sign up for GitHub

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

Draft
zackverham wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fix R functional test failures on Windows ARM runners #3660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 2 additions & 45 deletions internal/inspect/dependencies/renv/manifest_packages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"io/fs"
"os"
"slices"
"strconv"
"strings"

"github.com/posit-dev/publisher/internal/bundles"
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
16 changes: 10 additions & 6 deletions internal/inspect/dependencies/renv/manifest_packages_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +212 to +213
Copy link
Collaborator

@jonkeane jonkeane Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missing some context, but

This happens on binary-only repos (e.g. P3M/RSPM on Windows ARM) where the binary version leads the source version.

This sounds like something else is wrong, either with the P3M repo or our configuration of repos when we make this call. For the R / CRAN-like ecosystem I don't think there should ever be a circumstance where there is a binary available but the source isn't (or is older than the binary). Could you explain a little more about what's going on here?

Copy link
Collaborator Author

@zackverham zackverham Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2026-03-06 at 11 15 03 AM

This is the issue - cran for whatever reason is proving the wrong version for windows

Copy link
Collaborator

@jonkeane jonkeane Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ah ah maybe I misread "the binary version leads the source version." then? That screenshot the other way around from what I thought you were describing: the binary is an earlier number than the source. That is totally possible and common. And whatever the publisher does with that should be robust: where the version is recorded (and the repo + other details), such that when it's pulled on connect connect will find the right thing (ideally from a binary if it's available, but will fall back to source if that's not).

In this case: maybe our test harness is too strict in requiring a specific number baked into the lockfile (or elsewhere, I haven't dug on these tests recently). But I would hope that we could have this test run such that so long as some renv was installed it doesn't matter too much if it's 1.1.8 or 1.1.7

Copy link
Collaborator Author

@zackverham zackverham Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The binary mismatch I think is coming from what we get from P3M - which on the windows github runners is coming through as 1.1.8. I haven't done the deep dive to identify where the mismatch is precisely - I was running these experiments in the background, hoping cran would resolve things within a day or so.

Copy link
Collaborator Author

@zackverham zackverham Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.e.
Screenshot 2026-03-06 at 11 37 13 AM

base := s.testdata.Join("dev_version")
lockfilePath := base.Join("renv.lock")
libPath := base.Join("renv_library")
Expand All @@ -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() {
Expand Down
Loading