diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 79727615..c77a5258 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,7 +16,7 @@ jobs: snapshot: runs-on: ubuntu-latest environment: snapshot - if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' && !github.event.pull_request.head.repo.fork steps: - uses: actions/checkout@v5 with: diff --git a/cmd/dbc/install_test.go b/cmd/dbc/install_test.go index 3e488b1e..5bd943ef 100644 --- a/cmd/dbc/install_test.go +++ b/cmd/dbc/install_test.go @@ -120,7 +120,7 @@ func (suite *SubcommandTestSuite) TestInstallManifestOnlyDriver() { m := InstallCmd{Driver: "test-driver-manifest-only"}. GetModelCustom(baseModel{getDriverList: getTestDriverList, downloadPkg: downloadTestPkg}) suite.validateOutput("\r[✓] searching\r\n[✓] downloading\r\n[✓] installing\r\n[✓] verifying signature\r\n"+ - "\r\nInstalled test-driver-manifest-only 0.1.0 to "+suite.tempdir+"\r\n"+ + "\r\nInstalled test-driver-manifest-only 1.0.0 to "+suite.tempdir+"\r\n"+ "\r\nMust have libtest_driver installed to load this driver\r\n", suite.runCmd(m)) if runtime.GOOS != "windows" { suite.FileExists(filepath.Join(suite.tempdir, "test-driver-manifest-only.toml")) diff --git a/cmd/dbc/search_test.go b/cmd/dbc/search_test.go index 5037269a..82a20957 100644 --- a/cmd/dbc/search_test.go +++ b/cmd/dbc/search_test.go @@ -41,7 +41,7 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerbose() { "Available Versions:\r\n ├── 2.0.0\r\n ╰── 2.1.0\r\n"+ "• test-driver-manifest-only\r\n Title: Test Driver Manifest Only\r\n "+ "Description: This is manifest-only driver\r\n License: Apache-2.0\r\n "+ - "Available Versions:\r\n ╰── 0.1.0\r\n\r ", suite.runCmd(m)) + "Available Versions:\r\n ╰── 1.0.0\r\n\r ", suite.runCmd(m)) } func (suite *SubcommandTestSuite) TestSearchCmdVerboseWithInstalled() { @@ -64,5 +64,5 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerboseWithInstalled() { " Description: This is manifest-only driver\r\n"+ " License: Apache-2.0\r\n"+ " Available Versions:\r\n"+ - " ╰── 0.1.0\r\n\r ", suite.runCmd(m)) + " ╰── 1.0.0\r\n\r ", suite.runCmd(m)) } diff --git a/cmd/dbc/testdata/test_manifest.yaml b/cmd/dbc/testdata/test_manifest.yaml index c394740a..ba182fe3 100644 --- a/cmd/dbc/testdata/test_manifest.yaml +++ b/cmd/dbc/testdata/test_manifest.yaml @@ -54,7 +54,7 @@ drivers: license: Apache-2.0 path: test-driver-manifest-only pkginfo: - - version: v0.1.0 + - version: v1.0.0 packages: - platform: linux_amd64 - platform: macos_amd64 diff --git a/cmd/dbc/uninstall_test.go b/cmd/dbc/uninstall_test.go index f0a5dfe1..7b5f70b0 100644 --- a/cmd/dbc/uninstall_test.go +++ b/cmd/dbc/uninstall_test.go @@ -3,6 +3,7 @@ package main import ( + "fmt" "os" "path" "path/filepath" @@ -124,3 +125,31 @@ func (suite *SubcommandTestSuite) TestUninstallMultipleLocationsNonDefault() { suite.FileExists(filepath.Join(suite.tempdir, "test-driver-1.toml")) suite.NoFileExists(filepath.Join(installModel.cfg.Location, "test-driver-1.toml")) } + +func (suite *SubcommandTestSuite) TestUninstallManifestOnlyDriver() { + m := InstallCmd{Driver: "test-driver-manifest-only"}. + GetModelCustom(baseModel{getDriverList: getTestDriverList, downloadPkg: downloadTestPkg}) + suite.validateOutput("\r[✓] searching\r\n[✓] downloading\r\n[✓] installing\r\n[✓] verifying signature\r\n"+ + "\r\nInstalled test-driver-manifest-only 1.0.0 to "+suite.tempdir+"\r\n"+ + "\r\nMust have libtest_driver installed to load this driver\r\n", suite.runCmd(m)) + if runtime.GOOS != "windows" { + suite.FileExists(filepath.Join(suite.tempdir, "test-driver-manifest-only.toml")) + } + + // Verify the sidecar folder exists before we uninstall + new_sidecar_path := fmt.Sprintf("test-driver-manifest-only_%s_v1.0.0", config.PlatformTuple()) + err := os.Rename(filepath.Join(suite.tempdir, "test-driver-manifest-only"), filepath.Join(suite.tempdir, new_sidecar_path)) + if err != nil { + suite.Fail(fmt.Sprintf("Failed to rename sidecar folder. Something is wrong with this test: %v", err)) + } + suite.DirExists(filepath.Join(suite.tempdir, new_sidecar_path)) + + // Now uninstall and verify we clean up + m = UninstallCmd{Driver: "test-driver-manifest-only"}. + GetModelCustom(baseModel{getDriverList: getTestDriverList, downloadPkg: downloadTestPkg}) + suite.validateOutput("Driver `test-driver-manifest-only` uninstalled successfully!\r\n\r\n\r ", suite.runCmd(m)) + if runtime.GOOS != "windows" { + suite.NoFileExists(filepath.Join(suite.tempdir, "test-driver-manifest-only.toml")) + } + suite.NoDirExists(filepath.Join(suite.tempdir, new_sidecar_path)) +} diff --git a/config/config.go b/config/config.go index bc42a870..a5b6a2b8 100644 --- a/config/config.go +++ b/config/config.go @@ -11,7 +11,6 @@ import ( "io/fs" "maps" "os" - "path" "path/filepath" "runtime" "slices" @@ -201,7 +200,7 @@ func InstallDriver(cfg Config, shortName string, downloaded *os.File) (Manifest, if loc, err = EnsureLocation(cfg); err != nil { return Manifest{}, fmt.Errorf("could not ensure config location: %w", err) } - base := strings.TrimSuffix(path.Base(downloaded.Name()), ".tar.gz") + base := strings.TrimSuffix(filepath.Base(downloaded.Name()), ".tar.gz") finalDir := filepath.Join(loc, base) if err := os.MkdirAll(finalDir, 0o755); err != nil { @@ -315,3 +314,60 @@ func decodeManifest(r io.Reader, driverName string, requireShared bool) (Manifes return result, nil } + +// Common, non-platform-specific code for uninstalling a driver. Called by +// platform-specific UninstallDriver function. +func UninstallDriverShared(cfg Config, info DriverInfo) error { + for sharedPath := range info.Driver.Shared.Paths() { + // Run filepath.Clean on sharedPath mainly to catch inner ".." in the path + sharedPath = filepath.Clean(sharedPath) + + // Don't remove anything that isn't contained withing the found driver's + // config directory (i.e., avoid malicious driver manifests) + if !strings.HasPrefix(sharedPath, cfg.Location) { + continue + } + + // dbc installs drivers in a folder, other tools may not so we handle each + // differently. + if info.Source == "dbc" { + if err := os.RemoveAll(filepath.Dir(sharedPath)); err != nil { + // Ignore only when not found. This supports manifest-only drivers. + // TODO: Come up with a better mechanism to handle manifest-only drivers + // and remove this continue when we do + if errors.Is(err, fs.ErrNotExist) { + continue + } + return fmt.Errorf("error removing driver %s: %w", info.ID, err) + } + } else { + if err := os.Remove(sharedPath); err != nil { + // Ignore only when not found. This supports manifest-only drivers. + // TODO: Come up with a better mechanism to handle manifest-only drivers + // and remove this continue when we do + if errors.Is(err, fs.ErrNotExist) { + continue + } + return fmt.Errorf("error removing driver %s: %w", info.ID, err) + } + } + } + + // Manifest only drivers can come with extra files such as a LICENSE and we + // create a folder next to the driver manifest to store them, same as we'd + // store the actual driver shared library. Above, we find the path of this + // folder by looking at the Driver.shared path. For manifest-only drivers, + // Driver.shared is not a valid path (it's just a name), so this trick doesn't + // work. We do want to clean this folder up so here we guess what it is and + // try to remove it e.g., "somedriver_macos_arm64_v1.2.3." + extra_folder := fmt.Sprintf("%s_%s_v%s", info.ID, platformTuple, info.Version) + extra_folder = filepath.Clean(extra_folder) + extra_path := filepath.Join(cfg.Location, extra_folder) + finfo, err := os.Stat(extra_path) + if err == nil && finfo.IsDir() && extra_path != "." { + _ = os.RemoveAll(extra_path) + // ignore errors + } + + return nil +} diff --git a/config/dirs_unixlike.go b/config/dirs_unixlike.go index 98c32943..0486ea0c 100644 --- a/config/dirs_unixlike.go +++ b/config/dirs_unixlike.go @@ -5,9 +5,7 @@ package config import ( - "errors" "fmt" - "io/fs" "maps" "os" "path/filepath" @@ -90,36 +88,14 @@ func CreateManifest(cfg Config, driver DriverInfo) (err error) { } func UninstallDriver(cfg Config, info DriverInfo) error { - if info.Source == "dbc" { - for sharedPath := range info.Driver.Shared.Paths() { - if err := os.RemoveAll(filepath.Dir(sharedPath)); err != nil { - // Ignore only when not found. This supports manifest-only drivers. - // TODO: Come up with a better mechanism to handle manifest-only drivers - // and remove this continue when we do - if errors.Is(err, fs.ErrNotExist) { - continue - } - return fmt.Errorf("error removing driver %s: %w", info.ID, err) - } - } - } else { - for sharedPath := range info.Driver.Shared.Paths() { - if err := os.Remove(sharedPath); err != nil { - // Ignore only when not found. This supports manifest-only drivers. - // TODO: Come up with a better mechanism to handle manifest-only drivers - // and remove this continue when we do - if errors.Is(err, fs.ErrNotExist) { - continue - } - return fmt.Errorf("error removing driver %s: %w", info.ID, err) - } - } - } - manifest := filepath.Join(cfg.Location, info.ID+".toml") if err := os.Remove(manifest); err != nil { return fmt.Errorf("error removing manifest %s: %w", manifest, err) } + if err := UninstallDriverShared(cfg, info); err != nil { + return fmt.Errorf("failed to delete driver shared object: %w", err) + } + return nil } diff --git a/config/dirs_windows.go b/config/dirs_windows.go index 6e4ba793..1d157a48 100644 --- a/config/dirs_windows.go +++ b/config/dirs_windows.go @@ -5,7 +5,6 @@ package config import ( "errors" "fmt" - "io/fs" "log" "maps" "os" @@ -275,40 +274,20 @@ func CreateManifest(cfg Config, driver DriverInfo) (err error) { } func UninstallDriver(cfg Config, info DriverInfo) error { - k, err := registry.OpenKey(cfg.Level.key(), regKeyADBC, registry.WRITE) - if err != nil { - return err - } - defer k.Close() + if cfg.Level != ConfigEnv { + k, err := registry.OpenKey(cfg.Level.key(), regKeyADBC, registry.WRITE) + if err != nil { + return err + } + defer k.Close() - if err := registry.DeleteKey(k, info.ID); err != nil { - return fmt.Errorf("failed to delete driver registry key: %w", err) + if err := registry.DeleteKey(k, info.ID); err != nil { + return fmt.Errorf("failed to delete driver registry key: %w", err) + } } - if info.Source == "dbc" { - for sharedPath := range info.Driver.Shared.Paths() { - if err := os.RemoveAll(filepath.Dir(sharedPath)); err != nil { - // Ignore only when not found. This supports manifest-only drivers. - // TODO: Come up with a better mechanism to handle manifest-only drivers - // and remove this continue when we do - if errors.Is(err, fs.ErrNotExist) { - continue - } - return fmt.Errorf("error removing driver %s: %w", info.ID, err) - } - } - } else { - for sharedPath := range info.Driver.Shared.Paths() { - if err := os.Remove(sharedPath); err != nil { - // Ignore only when not found. This supports manifest-only drivers. - // TODO: Come up with a better mechanism to handle manifest-only drivers - // and remove this continue when we do - if errors.Is(err, fs.ErrNotExist) { - continue - } - return fmt.Errorf("error removing driver %s: %w", info.ID, err) - } - } + if err := UninstallDriverShared(cfg, info); err != nil { + return fmt.Errorf("failed to delete driver shared object: %w", err) } return nil