[runx] Fix platform compatibility check for artifacts (#401)

## Summary
This fixes two errors in the `findArtifactForPlatform` function:
* If the artifact from the GitHub repository is not compatible with the invoker's platform, now it correctly returns a `types.ErrPlatformNotSupported` error, and it returns a `types.NoKnownArchive` error in case the artifact isn't packaged in a known artifact format
* The way the artifact name string was parsed wasn't working with `x86_64` architecture: the string was split in two because of the underscore, thus never matching correctly. I simplified the logic, by simply checking if the artifact name string, forced to lowercase, contains the invoker's OS and architecture

## How was it tested?
The first issue was tested by creating an ad-hoc GitHub project, compiling it for windows only, and uploading the compiled artifact to a GitHub release, and trying to donwload the binary via both standalone `runx` and with a local `devbox` build from a `linux/amd64` machine. The `NoKnownArchive` error was tested by trying to download an ad-hoc private `.deb` package.

The second issue was tested by downloading the package `pb33f/wiretap@latest` from GitHub, previously not possible, with both standalone `runx` and a local `devbox` build.

pkg/runx/impl/registry/artifact.go

@@ -3,55 +3,46 @@ package registry
 import (
 	"path/filepath"
 	"strings"
-	"unicode"
 
 	"go.jetpack.io/pkg/runx/impl/types"
 )
 
-func findArtifactForPlatform(artifacts []types.ArtifactMetadata, platform types.Platform) *types.ArtifactMetadata {
-	var artifactForPlatform types.ArtifactMetadata
+func findArtifactForPlatform(artifacts []types.ArtifactMetadata, platform types.Platform) (types.ArtifactMetadata, error) {
+	foundPlatform := false
 	for _, artifact := range artifacts {
-		if isArtifactForPlatform(artifact, platform) {
-			artifactForPlatform = artifact
+		if isArtifactForPlatform(artifact.Name, platform) {
+			foundPlatform = true
 			if isKnownArchive(artifact.Name) {
 				// We only consider known archives because sometimes releases contain multiple files
 				// for the same platform. Some times those files are alternative installation methods
 				// like `.dmg`, `.msi`, or `.deb`, and sometimes they are metadata files like `.sha256`
 				// or a `.sig` file. We don't want to install those.
-				return &artifact
+				return artifact, nil
 			}
 		}
 	}
-	// Best attempt:
-	return &artifactForPlatform
+	if !foundPlatform {
+		return types.ArtifactMetadata{}, types.ErrPlatformNotSupported
+	}
+	return types.ArtifactMetadata{}, types.ErrNoKnownArchive
 }
 
-func isArtifactForPlatform(artifact types.ArtifactMetadata, platform types.Platform) bool {
+func isArtifactForPlatform(artifactName string, platform types.Platform) bool {
 	// Invalid platform:
 	if platform.Arch() == "" || platform.OS() == "" {
 		return false
 	}
 
-	// As a heuristic we tokenize the name of the artifact, and return the artifact that has
-	// tokens for both the OS and the Architecture.
-	tokens := strings.FieldsFunc(strings.ToLower(artifact.Name), func(r rune) bool {
-		return !unicode.IsLetter(r) && !unicode.IsNumber(r)
-	})
 	hasOS := false
 	hasArch := false
 
-	for _, token := range tokens {
-		if matchesOS(platform, token) {
-			hasOS = true
-			continue
-		}
-		if matchesArch(platform, token) {
-			hasArch = true
-			continue
-		}
-		if hasOS && hasArch {
-			return true
-		}
+	// We just check that the artifact name, forced to lowercase,
+	// contains the OS and architecture of the invoking system
+	if matchesOS(platform, strings.ToLower(artifactName)) {
+		hasOS = true
+	}
+	if matchesArch(platform, strings.ToLower(artifactName)) {
+		hasArch = true
 	}
 	return hasOS && hasArch
 }
@@ -60,17 +51,14 @@ var alternateOSNames = map[string][]string{
 	"darwin": {"macos", "mac"},
 }
 
-func matchesOS(platform types.Platform, token string) bool {
-	if token == platform.OS() {
-		return true
-	}
+func matchesOS(platform types.Platform, artifactName string) bool {
 	alts := alternateOSNames[platform.OS()]
 	for _, alt := range alts {
-		if token == alt {
+		if strings.Contains(artifactName, alt) {
 			return true
 		}
 	}
-	return false
+	return strings.Contains(artifactName, platform.OS())
 }
 
 var alternateArchNames = map[string][]string{
@@ -79,17 +67,14 @@ var alternateArchNames = map[string][]string{
 	"amd64": {"x86_64", "universal"},
 }
 
-func matchesArch(platform types.Platform, token string) bool {
-	if token == platform.Arch() {
-		return true
-	}
+func matchesArch(platform types.Platform, artifactName string) bool {
 	alts := alternateArchNames[platform.Arch()]
 	for _, alt := range alts {
-		if token == alt {
+		if strings.Contains(artifactName, alt) {
 			return true
 		}
 	}
-	return false
+	return strings.Contains(artifactName, platform.Arch())
 }
 
 var knownExts = []string{

pkg/runx/impl/registry/registry.go

@@ -72,11 +72,7 @@ func (r *Registry) GetArtifactMetadata(ctx context.Context, ref types.PkgRef, pl
 		return types.ArtifactMetadata{}, err
 	}
 
-	artifact := findArtifactForPlatform(release.Artifacts, platform)
-	if artifact == nil {
-		return types.ArtifactMetadata{}, types.ErrPlatformNotSupported
-	}
-	return *artifact, nil
+	return findArtifactForPlatform(release.Artifacts, platform)
 }
 
 func (r *Registry) GetArtifact(ctx context.Context, ref types.PkgRef, platform types.Platform) (string, error) {

pkg/runx/impl/registry/registry_test.go

@@ -1,8 +1,12 @@
 package registry
 
 import (
+	"errors"
 	"os"
+	"slices"
 	"testing"
+
+	"go.jetpack.io/pkg/runx/impl/types"
 )
 
 func TestIsBinary(t *testing.T) {
@@ -42,3 +46,72 @@ func TestIsBinary(t *testing.T) {
 		})
 	}
 }
+
+func TestIsKnownArchive(t *testing.T) {
+	tests := []struct {
+		name string
+		want bool
+	}{
+		{"archive.tar", true},
+		{"archive.tar.gz", true},
+		{"archive.deb", false},
+		{"archive", false},
+	}
+	for _, test := range tests {
+		got := isKnownArchive(test.name)
+		if got != test.want {
+			t.Errorf("isKnownArchive(%s) = %v, want %v", test.name, got, test.want)
+		}
+	}
+}
+
+func TestIsArtifactForPlatform(t *testing.T) {
+	tests := []struct {
+		name     string
+		platform types.Platform
+		want     bool
+	}{
+		{"linux-x86_64", types.NewPlatform("linux", "x86_64"), true},
+		{"linux_x86_64_1.1.0_SNAPSHOT-abcde12345", types.NewPlatform("linux", "x86_64"), true},
+		{"linux_x86_64-1.2.3-rc2.tar.gz", types.NewPlatform("linux", "amd64"), true},
+		{"no-os-no-arch", types.NewPlatform("", ""), false},
+		{"no-os-no-arch", types.NewPlatform("linux", "amd64"), false},
+		{"no_os_arm64", types.NewPlatform("linux", "arm64"), false},
+		{"linux_no-arch", types.NewPlatform("linux", "arm64"), false},
+		{"macos-universal-1.2.3", types.NewPlatform("darwin", "arm64"), true},
+		{"mac_386-1.2.3-snapshot_abcdef123456", types.NewPlatform("darwin", "386"), true},
+	}
+
+	for _, test := range tests {
+		got := isArtifactForPlatform(test.name, test.platform)
+		if got != test.want {
+			t.Errorf("isArtifactForPlatform(%s) = %v, want %v", test.name, got, test.want)
+		}
+	}
+}
+
+func TestFindArtifactForPlatform(t *testing.T) {
+	tests := []struct {
+		artifacts   []types.ArtifactMetadata
+		platform    types.Platform
+		want        bool
+		errTypeWant error
+	}{
+		{[]types.ArtifactMetadata{{Name: "mac-amd64.tar.gz"}}, types.NewPlatform("darwin", "amd64"), true, nil},
+		{[]types.ArtifactMetadata{{Name: "linux-amd64.deb"}}, types.NewPlatform("linux", "amd64"), false, types.ErrNoKnownArchive},
+		{[]types.ArtifactMetadata{{Name: "linux-amd64"}}, types.NewPlatform("linux", "amd64"), false, types.ErrNoKnownArchive},
+		{[]types.ArtifactMetadata{{Name: "darwin_arm64.tar"}}, types.NewPlatform("windows", "amd64"), false, types.ErrPlatformNotSupported},
+		{[]types.ArtifactMetadata{{Name: "darwin_arm64"}}, types.NewPlatform("windows", "amd64"), false, types.ErrPlatformNotSupported},
+		{[]types.ArtifactMetadata{{Name: "mac-amd64.tar.gz"}}, types.NewPlatform("", ""), false, types.ErrPlatformNotSupported},
+	}
+
+	for _, test := range tests {
+		got, err := findArtifactForPlatform(test.artifacts, test.platform)
+		if slices.Contains(test.artifacts, got) != test.want {
+			t.Errorf("findArtifactForPlatform() didn't return %v", got)
+		}
+		if !errors.Is(err, test.errTypeWant) {
+			t.Errorf("findArtifactForPlatform() returned %v error instead of %v", err, test.errTypeWant)
+		}
+	}
+}

pkg/runx/impl/types/errors.go

@@ -5,3 +5,4 @@ import "errors"
 var ErrPackageNotFound = errors.New("package not found")
 var ErrReleaseNotFound = errors.New("release not found")
 var ErrPlatformNotSupported = errors.New("package doesn't support platform")
+var ErrNoKnownArchive = errors.New("package doesn't come in a known archive")