From 3293600a874bd89246acf6701db474d56075af9a Mon Sep 17 00:00:00 2001 From: Rebecca Mahany-Horton Date: Fri, 6 Sep 2024 11:12:25 -0400 Subject: [PATCH] Only set arch on bin dir when we are building for Windows (#1851) --- pkg/packaging/detectLauncherVersion.go | 15 +++-- pkg/packaging/detectLauncherVersion_test.go | 55 ++++++++++++++---- pkg/packaging/fetch.go | 6 +- pkg/packaging/packaging.go | 14 ++++- pkg/packaging/packaging_test.go | 62 +++++++++++++++++++++ 5 files changed, 130 insertions(+), 22 deletions(-) diff --git a/pkg/packaging/detectLauncherVersion.go b/pkg/packaging/detectLauncherVersion.go index 169e83f5c..58befc80e 100644 --- a/pkg/packaging/detectLauncherVersion.go +++ b/pkg/packaging/detectLauncherVersion.go @@ -19,7 +19,7 @@ func (p *PackageOptions) detectLauncherVersion(ctx context.Context) error { logger := log.With(ctxlog.FromContext(ctx), "library", "detectLauncherVersion") level.Debug(logger).Log("msg", "Attempting launcher autodetection") - launcherPath := p.launcherLocation(filepath.Join(p.packageRoot, p.binDir, string(p.target.Arch))) + launcherPath := p.launcherLocation() stdout, err := p.execOut(ctx, launcherPath, "-version") if err != nil { @@ -49,17 +49,20 @@ func (p *PackageOptions) detectLauncherVersion(ctx context.Context) error { // launcherLocation returns the location of the launcher binary within `binDir`. For darwin, // it may be in an app bundle -- we check to see if the binary exists there first, and then // fall back to the common location if it doesn't. -func (p *PackageOptions) launcherLocation(binDir string) string { +func (p *PackageOptions) launcherLocation() string { if p.target.Platform == Darwin { - // We want /usr/local/Kolide.app, not /usr/local/bin/universal/Kolide.app, so we use Dir to strip out `bin` - // and universal - appBundleBinaryPath := filepath.Join(filepath.Dir(filepath.Dir(binDir)), "Kolide.app", "Contents", "MacOS", "launcher") + // We want /usr/local/Kolide.app, not /usr/local/bin/Kolide.app, so we use Dir to strip out `bin` + appBundleBinaryPath := filepath.Join(p.packageRoot, filepath.Dir(p.binDir), "Kolide.app", "Contents", "MacOS", "launcher") if info, err := os.Stat(appBundleBinaryPath); err == nil && !info.IsDir() { return appBundleBinaryPath } } - return filepath.Join(binDir, p.target.PlatformBinaryName("launcher")) + if p.target.Platform == Windows { + return filepath.Join(p.packageRoot, p.binDir, string(p.target.Arch), p.target.PlatformBinaryName("launcher")) + } + + return filepath.Join(p.packageRoot, p.binDir, p.target.PlatformBinaryName("launcher")) } // formatVersion formats the version according to the packaging requirements of the target platform diff --git a/pkg/packaging/detectLauncherVersion_test.go b/pkg/packaging/detectLauncherVersion_test.go index 3f7e6f4a9..5ff51306d 100644 --- a/pkg/packaging/detectLauncherVersion_test.go +++ b/pkg/packaging/detectLauncherVersion_test.go @@ -85,19 +85,25 @@ func TestFormatVersion(t *testing.T) { } } -func TestLauncherLocation(t *testing.T) { +func TestLauncherLocation_Darwin(t *testing.T) { t.Parallel() - pDarwin := &PackageOptions{target: Target{Platform: Darwin}} + pDarwin := &PackageOptions{ + target: Target{ + Platform: Darwin, + }, + packageRoot: t.TempDir(), + binDir: "bin", + } // First, test that if the app bundle doesn't exist, we fall back to the top-level binary - require.Equal(t, filepath.Join("some", "dir", "launcher"), pDarwin.launcherLocation(filepath.Join("some", "dir"))) + expectedFallbackLauncherLocation := filepath.Join(pDarwin.packageRoot, pDarwin.binDir, "launcher") + require.Equal(t, expectedFallbackLauncherLocation, pDarwin.launcherLocation()) // Create a temp directory with an app bundle in it - tmpDir := t.TempDir() - binDir := filepath.Join(tmpDir, "bin", "universal") + binDir := filepath.Join(pDarwin.packageRoot, pDarwin.binDir) require.NoError(t, os.MkdirAll(binDir, 0755)) - baseDir := filepath.Join(tmpDir, "Kolide.app", "Contents", "MacOS") + baseDir := filepath.Join(pDarwin.packageRoot, "Kolide.app", "Contents", "MacOS") require.NoError(t, os.MkdirAll(baseDir, 0755)) expectedLauncherBinaryPath := filepath.Join(baseDir, "launcher") f, err := os.Create(expectedLauncherBinaryPath) @@ -106,13 +112,38 @@ func TestLauncherLocation(t *testing.T) { defer os.Remove(expectedLauncherBinaryPath) // Now, confirm that we find the binary inside the app bundle - require.Equal(t, expectedLauncherBinaryPath, pDarwin.launcherLocation(binDir)) + require.Equal(t, expectedLauncherBinaryPath, pDarwin.launcherLocation()) +} + +func TestLauncherLocation_Windows(t *testing.T) { + t.Parallel() + + pWindows := &PackageOptions{ + target: Target{ + Platform: Windows, + Arch: Amd64, + }, + packageRoot: t.TempDir(), + binDir: "bin", + } // No file check for windows, just expect the binary in the given location - pWindows := &PackageOptions{target: Target{Platform: Windows}} - require.Equal(t, filepath.Join("some", "dir", "launcher.exe"), pWindows.launcherLocation(filepath.Join("some", "dir"))) + expectedLauncherLocation := filepath.Join(pWindows.packageRoot, pWindows.binDir, string(pWindows.target.Arch), "launcher.exe") + require.Equal(t, expectedLauncherLocation, pWindows.launcherLocation()) +} + +func TestLauncherLocation_Linux(t *testing.T) { + t.Parallel() + + pLinux := &PackageOptions{ + target: Target{ + Platform: Linux, + }, + packageRoot: t.TempDir(), + binDir: "bin", + } - // Same as for windows: no file check, just expect the binary in the given location - pLinux := &PackageOptions{target: Target{Platform: Linux}} - require.Equal(t, filepath.Join("some", "dir", "launcher"), pLinux.launcherLocation(filepath.Join("some", "dir"))) + // No file check for Linux, just expect the binary in the given location + expectedLauncherLocation := filepath.Join(pLinux.packageRoot, pLinux.binDir, "launcher") + require.Equal(t, expectedLauncherLocation, pLinux.launcherLocation()) } diff --git a/pkg/packaging/fetch.go b/pkg/packaging/fetch.go index 837e44fb6..8e3396b76 100644 --- a/pkg/packaging/fetch.go +++ b/pkg/packaging/fetch.go @@ -38,9 +38,11 @@ func FetchBinary(ctx context.Context, localCacheDir, name, binaryName, channelOr return "", errors.New("empty cache dir argument") } - // put binaries in arch specific directory to avoid naming collisions in wix msi building + // put binaries in arch specific directory for Windows to avoid naming collisions in wix msi building // where a single destination will have multiple, mutally exclusive sources - localCacheDir = filepath.Join(localCacheDir, string(target.Arch)) + if target.Platform == Windows { + localCacheDir = filepath.Join(localCacheDir, string(target.Arch)) + } if err := os.MkdirAll(localCacheDir, fsutil.DirMode); err != nil { return "", fmt.Errorf("could not create cache directory: %w", err) diff --git a/pkg/packaging/packaging.go b/pkg/packaging/packaging.go index 8a1027370..a250243bc 100644 --- a/pkg/packaging/packaging.go +++ b/pkg/packaging/packaging.go @@ -379,7 +379,7 @@ func (p *PackageOptions) getBinary(ctx context.Context, symbolicName, binaryName // Create a symlink from / to the actual binary location within the app bundle target := filepath.Join("..", appBundleName, "Contents", "MacOS", binaryName) - symlinkFile := filepath.Join(p.packageRoot, p.binDir, binaryName) + symlinkFile := p.fullPathToBareBinary(binaryName) if err := os.Symlink(target, symlinkFile); err != nil { return fmt.Errorf("could not create symlink after copying app bundle: %w", err) } @@ -387,7 +387,7 @@ func (p *PackageOptions) getBinary(ctx context.Context, symbolicName, binaryName return nil } - binPath := filepath.Join(p.packageRoot, p.binDir, string(p.target.Arch), binaryName) + binPath := p.fullPathToBareBinary(binaryName) if err := os.MkdirAll(filepath.Dir(binPath), fsutil.DirMode); err != nil { return fmt.Errorf("could not create directory for binary %s: %w", binaryName, err) } @@ -402,6 +402,16 @@ func (p *PackageOptions) getBinary(ctx context.Context, symbolicName, binaryName return nil } +// fullPathToBareBinary returns the path to the binary (including arch only on Windows). +// On macOS, this location is a symlink to inside the app bundle. +func (p *PackageOptions) fullPathToBareBinary(binaryName string) string { + if p.target.Platform == Windows { + return filepath.Join(p.packageRoot, p.binDir, string(p.target.Arch), binaryName) + } + + return filepath.Join(p.packageRoot, p.binDir, binaryName) +} + func (p *PackageOptions) makePackage(ctx context.Context) error { ctx, span := trace.StartSpan(ctx, "packaging.makePackage") defer span.End() diff --git a/pkg/packaging/packaging_test.go b/pkg/packaging/packaging_test.go index 97d829a63..90b1ee20f 100644 --- a/pkg/packaging/packaging_test.go +++ b/pkg/packaging/packaging_test.go @@ -277,3 +277,65 @@ func testedTargets() []Target { }, } } + +func Test_fullPathToBareBinary(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + testCaseName string + binaryName string + identifier string + packageRoot string + binDir string + target Target + expectedPath string + }{ + { + testCaseName: "darwin", + binaryName: "launcher", + packageRoot: filepath.Join("test", "root"), + binDir: filepath.Join("usr", "local", "test-identifier", "bin"), + target: Target{ + Platform: Darwin, + Arch: Arm64, + }, + expectedPath: filepath.Join("test", "root", "usr", "local", "test-identifier", "bin", "launcher"), + }, + { + testCaseName: "linux", + binaryName: "launcher", + packageRoot: filepath.Join("test", "root"), + binDir: filepath.Join("usr", "local", "test-identifier", "bin"), + target: Target{ + Platform: Linux, + Arch: Amd64, + }, + expectedPath: filepath.Join("test", "root", "usr", "local", "test-identifier", "bin", "launcher"), + }, + { + testCaseName: "windows", + binaryName: "launcher.exe", + packageRoot: filepath.Join("test", "root"), + binDir: filepath.Join("Launcher-test-identifier", "bin"), + target: Target{ + Platform: Windows, + Arch: Amd64, + }, + expectedPath: filepath.Join("test", "root", "Launcher-test-identifier", "bin", "amd64", "launcher.exe"), + }, + } { + tt := tt + t.Run(tt.testCaseName, func(t *testing.T) { + t.Parallel() + + p := &PackageOptions{ + packageRoot: tt.packageRoot, + binDir: tt.binDir, + target: tt.target, + } + + actualPath := p.fullPathToBareBinary(tt.binaryName) + require.Equal(t, tt.expectedPath, actualPath) + }) + } +}