fix: add test to cover behavior for invalid manifests

cmd/dbc/driver_list_test.go

@@ -23,11 +23,11 @@ func TestUnmarshalDriverList(t *testing.T) {
 		expected []dbc.PkgInfo
 		err      error
 	}{
-		{"basic", "[drivers]\nflightsql = {version = '1.7.0'}", []dbc.PkgInfo{
-			{Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.7.0")},
+		{"basic", "[drivers]\nflightsql = {version = '1.8.0'}", []dbc.PkgInfo{
+			{Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.8.0")},
 		}, nil},
-		{"less", "[drivers]\nflightsql = {version = '<=1.7.0'}", []dbc.PkgInfo{
-			{Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.7.0")},
+		{"less", "[drivers]\nflightsql = {version = '<=1.8.0'}", []dbc.PkgInfo{
+			{Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.8.0")},
 		}, nil},
 		{"greater", "[drivers]\nflightsql = {version = '>=1.7.0'}", []dbc.PkgInfo{
 			{Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.8.0")},

cmd/dbc/search_test.go

@@ -15,7 +15,8 @@ func (suite *SubcommandTestSuite) TestSearchCmd() {
 	suite.validateOutput("• test-driver-1 - This is a test driver\r\n"+
 		"• test-driver-2 - This is another test driver\r\n"+
 		"• test-driver-manifest-only - This is manifest-only driver\r\n"+
-		"• test-driver-no-sig - Driver manifest missing Files.signature entry\r\n\r ", "", suite.runCmd(m))
+		"• test-driver-no-sig - Driver manifest missing Files.signature entry\r\n"+
+		"• test-driver-invalid-manifest - This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.\r\n\r ", "", suite.runCmd(m))
 }
 
 func (suite *SubcommandTestSuite) TestSearchCmdWithInstalled() {
@@ -28,7 +29,8 @@ func (suite *SubcommandTestSuite) TestSearchCmdWithInstalled() {
 			downloadPkg: downloadTestPkg})
 	suite.validateOutput("• test-driver-1 - This is a test driver [installed: env=>1.1.0]\r\n"+
 		"• test-driver-2 - This is another test driver\r\n• test-driver-manifest-only - This is manifest-only driver\r\n"+
-		"• test-driver-no-sig - Driver manifest missing Files.signature entry\r\n\r ", "", suite.runCmd(m))
+		"• test-driver-no-sig - Driver manifest missing Files.signature entry\r\n"+
+		"• test-driver-invalid-manifest - This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.\r\n\r ", "", suite.runCmd(m))
 }
 
 func (suite *SubcommandTestSuite) TestSearchCmdVerbose() {
@@ -46,6 +48,9 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerbose() {
 		"Available Versions:\r\n    ╰── 1.0.0\r\n"+
 		"• test-driver-no-sig\r\n   Title: Test Driver No Signature\r\n   "+
 		"Description: Driver manifest missing Files.signature entry\r\n   License: Apache-2.0\r\n   "+
+		"Available Versions:\r\n    ╰── 1.0.0\r\n"+
+		"• test-driver-invalid-manifest\r\n   Title: Test Driver Invalid Manifest\r\n   "+
+		"Description: This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.\r\n   License: Apache-2.0\r\n   "+
 		"Available Versions:\r\n    ╰── 1.0.0\r\n\r ", "", suite.runCmd(m))
 }
 
@@ -75,5 +80,11 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerboseWithInstalled() {
 		"   Description: Driver manifest missing Files.signature entry\r\n"+
 		"   License: Apache-2.0\r\n"+
 		"   Available Versions:\r\n"+
+		"    ╰── 1.0.0\r\n"+
+		"• test-driver-invalid-manifest\r\n"+
+		"   Title: Test Driver Invalid Manifest\r\n"+
+		"   Description: This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.\r\n"+
+		"   License: Apache-2.0\r\n"+
+		"   Available Versions:\r\n"+
 		"    ╰── 1.0.0\r\n\r ", "", suite.runCmd(m))
 }

cmd/dbc/sync_test.go

@@ -45,6 +45,8 @@ func downloadTestPkg(pkg dbc.PkgInfo) (*os.File, error) {
 		return os.Open(filepath.Join("testdata", "test-driver-manifest-only.tar.gz"))
 	case "test-driver-no-sig":
 		return os.Open(filepath.Join("testdata", "test-driver-no-sig.tar.gz"))
+	case "test-driver-invalid-manifest":
+		return os.Open(filepath.Join("testdata", "test-driver-invalid-manifest.tar.gz"))
 	default:
 		return nil, fmt.Errorf("unknown driver: %s", pkg.Driver.Path)
 	}

cmd/dbc/testdata/test_index.yaml

@@ -68,6 +68,21 @@ drivers:
       - version: v1.0.0
         packages:
           - platform: linux_amd64
+          - platform: linux_arm64
           - platform: macos_amd64
           - platform: macos_arm64
-          - platform: windows_amd64
+          - platform: windows_amd64
+          - platform: windows_arm64
+  - name: Test Driver Invalid Manifest
+    description: This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.
+    license: Apache-2.0
+    path: test-driver-invalid-manifest
+    pkginfo:
+      - version: v1.0.0
+        packages:
+          - platform: linux_amd64
+          - platform: linux_arm64
+          - platform: macos_amd64
+          - platform: macos_arm64
+          - platform: windows_amd64
+          - platform: windows_arm64

cmd/dbc/uninstall_test.go

@@ -10,6 +10,7 @@ import (
 	"runtime"
 
 	"github.com/columnar-tech/dbc/config"
+	"github.com/pelletier/go-toml/v2"
 )
 
 func (suite *SubcommandTestSuite) TestUninstallNotFound() {
@@ -153,3 +154,49 @@ func (suite *SubcommandTestSuite) TestUninstallManifestOnlyDriver() {
 	}
 	suite.NoDirExists(filepath.Join(suite.tempdir, new_sidecar_path))
 }
+
+// See https://github.com/columnar-tech/dbc/issues/37
+func (suite *SubcommandTestSuite) TestUninstallInvalidManifest() {
+	if runtime.GOOS == "windows" {
+		suite.T().Skip()
+	}
+	m := InstallCmd{Driver: "test-driver-invalid-manifest", Level: config.ConfigEnv}.
+		GetModelCustom(baseModel{getDriverList: getTestDriverList, downloadPkg: downloadTestPkg})
+	suite.runCmd(m)
+	suite.FileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest.toml"))
+
+	// The installed manifest should have a Driver.shared set to a folder, not the .so
+	// We only need a partial struct definition to read in the Driver.shared table
+	type partialManifest struct {
+		Driver struct {
+			Shared map[string]string `toml:"shared"`
+		}
+	}
+	var invalidManifest partialManifest
+	f, err := os.Open(filepath.Join(suite.tempdir, "test-driver-invalid-manifest.toml"))
+	if err != nil {
+		suite.Error(err)
+	}
+	err = toml.NewDecoder(f).Decode(&invalidManifest)
+	if err != nil {
+		suite.Error(err)
+	}
+	value := invalidManifest.Driver.Shared[config.PlatformTuple()]
+	// Assert that it's a folder
+	suite.DirExists(value)
+	// and continue
+
+	m = UninstallCmd{Driver: "test-driver-invalid-manifest", Level: config.ConfigEnv}.GetModel()
+	output := suite.runCmd(m)
+
+	suite.validateOutput("Driver `test-driver-invalid-manifest` uninstalled successfully!\r\n\r\n\r ", "", output)
+
+	// Ensure we don't nuke the tempdir which is the original (major) issue
+	suite.DirExists(suite.tempdir)
+
+	// We do remove the manifest
+	suite.NoFileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest.toml"))
+	// But we don't remove the driver shared folder in this edge case, so we assert
+	// they're still around
+	suite.FileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest", "libadbc_driver_invalid_manifest.so"))
+}

config/config.go

@@ -331,7 +331,16 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error {
 		// 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 {
+			sharedDir := filepath.Dir(sharedPath)
+			// Edge case when manifest is ill-formed: if sharedPath is set to the
+			// folder containing the shared library instead of the shared library
+			// itself, sharedDir is cfg.Location and we definitely don't want to
+			// remove that
+			if sharedDir == cfg.Location {
+				continue
+			}
+
+			if err := os.RemoveAll(sharedDir); 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