pkgman uninstall: improve correctness

... with respect to inter-installation-location dependencies. E.g.
uninstalling a package from common should only uninstall packages
depending on it, when system doesn't still provide those dependencies.

We don't consider uninstalling packages from more specific installation
locations when dependencies are uninstalled from a more general one yet.

src/bin/pkgman/PackageManager.cpp

@@ -31,10 +31,10 @@
 using namespace BPackageKit::BPrivate;
 
 
-// #pragma mark - Repository
+// #pragma mark - RemoteRepository
 
 
-PackageManager::Repository::Repository()
+PackageManager::RemoteRepository::RemoteRepository()
 	:
 	BSolverRepository()
 {
@@ -42,8 +42,8 @@ PackageManager::Repository::Repository()
 
 
 status_t
-PackageManager::Repository::Init(BPackageRoster& roster, BContext& context,
+PackageManager::RemoteRepository::Init(BPackageRoster& roster,
-	const char* name, bool refresh)
+	BContext& context, const char* name, bool refresh)
 {
 	// get the repository config
 	status_t error = roster.GetRepositoryConfig(name, &fConfig);
@@ -67,12 +67,46 @@ PackageManager::Repository::Init(BPackageRoster& roster, BContext& context,
 
 
 const BRepositoryConfig&
-PackageManager::Repository::Config() const
+PackageManager::RemoteRepository::Config() const
 {
 	return fConfig;
 }
 
 
+// #pragma mark - InstalledRepository
+
+
+PackageManager::InstalledRepository::InstalledRepository()
+	:
+	BSolverRepository(),
+	fDisabledPackages(10, true)
+{
+}
+
+
+void
+PackageManager::InstalledRepository::DisablePackage(BSolverPackage* package)
+{
+	if (fDisabledPackages.HasItem(package)) {
+		fprintf(stderr, "*** package %s already disabled\n",
+			package->VersionedName().String());
+		exit(1);
+	}
+
+	if (package->Repository() != this) {
+		fprintf(stderr, "*** package %s not in repository %s\n",
+			package->VersionedName().String(), Name().String());
+		exit(1);
+	}
+
+	// move to disabled list
+	if (!fDisabledPackages.AddItem(package))
+		DIE(B_NO_MEMORY, "*** failed to add package to list");
+
+	RemovePackage(package);
+}
+
+
 // #pragma mark - Solver
 
 
@@ -81,9 +115,9 @@ PackageManager::PackageManager(BPackageInstallationLocation location,
 	:
 	fLocation(location),
 	fSolver(NULL),
-	fSystemRepository(),
+	fSystemRepository(new (std::nothrow) InstalledRepository),
-	fCommonRepository(),
+	fCommonRepository(new (std::nothrow) InstalledRepository),
-	fHomeRepository(),
+	fHomeRepository(new (std::nothrow) InstalledRepository),
 	fInstalledRepositories(10),
 	fOtherRepositories(10, true),
 	fDecisionProvider(),
@@ -95,6 +129,11 @@ PackageManager::PackageManager(BPackageInstallationLocation location,
 	if (error != B_OK)
 		DIE(error, "failed to create solver");
 
+	if (fSystemRepository == NULL || fCommonRepository == NULL
+		|| fHomeRepository == NULL) {
+		DIE(B_NO_MEMORY, "failed to create repositories");
+	}
+
 	// add installation location repositories
 	if ((flags & ADD_INSTALLED_REPOSITORIES) != 0) {
 		// We add only the repository of our actual installation location as the
@@ -106,28 +145,28 @@ PackageManager::PackageManager(BPackageInstallationLocation location,
 		// one. Instead any requirement that is already installed in a more
 		// general installation location will turn up as to be installed as
 		// well. But we can easily filter those out.
-		RepositoryBuilder(fSystemRepository, "system")
+		RepositoryBuilder(*fSystemRepository, "system")
 			.AddPackages(B_PACKAGE_INSTALLATION_LOCATION_SYSTEM, "system")
 			.AddToSolver(fSolver, false);
-		fSystemRepository.SetPriority(-1);
+		fSystemRepository->SetPriority(-1);
 
 		bool installInHome = location == B_PACKAGE_INSTALLATION_LOCATION_HOME;
-		RepositoryBuilder(fCommonRepository, "common")
+		RepositoryBuilder(*fCommonRepository, "common")
 			.AddPackages(B_PACKAGE_INSTALLATION_LOCATION_COMMON, "common")
 			.AddToSolver(fSolver, !installInHome);
-	
+
-		if (!fInstalledRepositories.AddItem(&fSystemRepository)
+		if (!fInstalledRepositories.AddItem(fSystemRepository)
-			|| !fInstalledRepositories.AddItem(&fCommonRepository)) {
+			|| !fInstalledRepositories.AddItem(fCommonRepository)) {
 			DIE(B_NO_MEMORY, "failed to add installed repositories to list");
 		}
-	
+
 		if (installInHome) {
-			fCommonRepository.SetPriority(-2);
+			fCommonRepository->SetPriority(-2);
-			RepositoryBuilder(fHomeRepository, "home")
+			RepositoryBuilder(*fHomeRepository, "home")
 				.AddPackages(B_PACKAGE_INSTALLATION_LOCATION_HOME, "home")
 				.AddToSolver(fSolver, true);
-	
+
-			if (!fInstalledRepositories.AddItem(&fHomeRepository))
+			if (!fInstalledRepositories.AddItem(fHomeRepository))
 				DIE(B_NO_MEMORY, "failed to add home repository to list");
 		}
 	}
@@ -142,7 +181,7 @@ PackageManager::PackageManager(BPackageInstallationLocation location,
 	
 		int32 repositoryNameCount = repositoryNames.CountStrings();
 		for (int32 i = 0; i < repositoryNameCount; i++) {
-			Repository* repository = new(std::nothrow) Repository;
+			RemoteRepository* repository = new(std::nothrow) RemoteRepository;
 			if (repository == NULL || !fOtherRepositories.AddItem(repository))
 				DIE(B_NO_MEMORY, "failed to create/add repository object");
 	
@@ -166,6 +205,9 @@ PackageManager::PackageManager(BPackageInstallationLocation location,
 
 PackageManager::~PackageManager()
 {
+	delete fSystemRepository;
+	delete fCommonRepository;
+	delete fHomeRepository;
 }
 
 
@@ -201,7 +243,7 @@ PackageManager::Install(const char* const* packages, int packageCount)
 void
 PackageManager::Uninstall(const char* const* packages, int packageCount)
 {
-	// solve
+	// find the packages that match the specification
 	BSolverPackageSpecifierList packagesToUninstall;
 	for (int i = 0; i < packageCount; i++) {
 		if (!packagesToUninstall.AppendSpecifier(packages[i]))
@@ -209,8 +251,9 @@ PackageManager::Uninstall(const char* const* packages, int packageCount)
 	}
 
 	const BSolverPackageSpecifier* unmatchedSpecifier;
-	status_t error = fSolver->Uninstall(packagesToUninstall,
+	PackageList foundPackages;
-		&unmatchedSpecifier);
+	status_t error = fSolver->FindPackages(packagesToUninstall,
+		BSolver::B_FIND_INSTALLED_ONLY, foundPackages, &unmatchedSpecifier);
 	if (error != B_OK) {
 		if (unmatchedSpecifier != NULL) {
 			DIE(error, "failed to find a match for \"%s\"",
@@ -219,10 +262,44 @@ PackageManager::Uninstall(const char* const* packages, int packageCount)
 			DIE(error, "failed to compute packages to uninstall");
 	}
 
+	// determine the inverse base package closure for the found packages
+	InstalledRepository* installationRepository
+		= dynamic_cast<InstalledRepository*>(
+			foundPackages.ItemAt(0)->Repository());
+	bool foundAnotherPackage;
+	do {
+		foundAnotherPackage = false;
+		int32 count = installationRepository->CountPackages();
+		for (int32 i = 0; i < count; i++) {
+			BSolverPackage* package = installationRepository->PackageAt(i);
+			if (foundPackages.HasItem(package))
+				continue;
+
+			if (_FindBasePackage(foundPackages, package->Info()) >= 0) {
+				foundPackages.AddItem(package);
+				foundAnotherPackage = true;
+			}
+		}
+	} while (foundAnotherPackage);
+
+	// remove the packages from the repository
+	for (int32 i = 0; BSolverPackage* package = foundPackages.ItemAt(i); i++)
+		installationRepository->DisablePackage(package);
+
+	error = fSolver->VerifyInstallation(BSolver::B_VERIFY_ALLOW_UNINSTALL);
+	if (error != B_OK)
+		DIE(error, "failed to compute packages to uninstall");
+
 	_HandleProblems();
 
 	// install/uninstall packages
 	_AnalyzeResult();
+
+	for (int32 i = foundPackages.CountItems() - 1; i >= 0; i--) {
+		if (!fPackagesToDeactivate.AddItem(foundPackages.ItemAt(i)))
+			DIE(B_NO_MEMORY, "failed to add package to uninstall");
+	}
+
 	_PrintResult();
 	_ApplyPackageChanges();
 }
@@ -342,7 +419,8 @@ PackageManager::_AnalyzeResult()
 			case BSolverResultElement::B_TYPE_INSTALL:
 			{
 				PackageList& packageList
-					= fInstalledRepositories.HasItem(package->Repository())
+					= dynamic_cast<InstalledRepository*>(package->Repository())
+							!= NULL
 						? potentialBasePackages
 						: fPackagesToActivate;
 				if (!packageList.AddItem(package))
@@ -357,11 +435,6 @@ PackageManager::_AnalyzeResult()
 		}
 	}
 
-	if (fPackagesToActivate.IsEmpty() && fPackagesToDeactivate.IsEmpty()) {
-		printf("Nothing to do.\n");
-		exit(0);
-	}
-
 	// Make sure base packages are installed in the same location.
 	for (int32 i = 0; i < fPackagesToActivate.CountItems(); i++) {
 		BSolverPackage* package = fPackagesToActivate.ItemAt(i);
@@ -379,6 +452,11 @@ PackageManager::_AnalyzeResult()
 void
 PackageManager::_PrintResult()
 {
+	if (fPackagesToActivate.IsEmpty() && fPackagesToDeactivate.IsEmpty()) {
+		printf("Nothing to do.\n");
+		exit(0);
+	}
+
 	printf("The following changes will be made:\n");
 	for (int32 i = 0; BSolverPackage* package = fPackagesToActivate.ItemAt(i);
 		i++) {
@@ -419,8 +497,6 @@ PackageManager::_ApplyPackageChanges()
 	for (int32 i = 0; BSolverPackage* package = fPackagesToActivate.ItemAt(i);
 		i++) {
 		// get package URL and target entry
-		Repository* repository
-			= static_cast<Repository*>(package->Repository());
 
 		BString fileName(package->Info().CanonicalFileName());
 		if (fileName.IsEmpty())
@@ -431,12 +507,16 @@ PackageManager::_ApplyPackageChanges()
 		if (error != B_OK)
 			DIE(error, "failed to create package entry");
 
-		if (fInstalledRepositories.HasItem(repository)) {
+		RemoteRepository* remoteRepository
+			= dynamic_cast<RemoteRepository*>(package->Repository());
+		if (remoteRepository == NULL) {
 			// clone the existing package
-			_ClonePackageFile(repository, fileName, entry);
+			_ClonePackageFile(
+				dynamic_cast<InstalledRepository*>(package->Repository()),
+				fileName, entry);
 		} else {
 			// download the package
-			BString url = repository->Config().PackagesURL();
+			BString url = remoteRepository->Config().PackagesURL();
 			url << '/' << fileName;
 
 			DownloadFileRequest downloadRequest(fContext, url, entry,
@@ -486,14 +566,14 @@ PackageManager::_ApplyPackageChanges()
 
 
 void
-PackageManager::_ClonePackageFile(Repository* repository,
+PackageManager::_ClonePackageFile(InstalledRepository* repository,
 	const BString& fileName, const BEntry& entry) const
 {
 	// get the source and destination file paths
 	directory_which packagesWhich;
-	if (repository == &fSystemRepository) {
+	if (repository == fSystemRepository) {
 		packagesWhich = B_SYSTEM_PACKAGES_DIRECTORY;
-	} else if (repository == &fCommonRepository) {
+	} else if (repository == fCommonRepository) {
 		packagesWhich = B_COMMON_PACKAGES_DIRECTORY;
 	} else {
 		fprintf(stderr, "*** don't know packages directory path for "

src/bin/pkgman/PackageManager.h

@@ -26,8 +26,10 @@ using namespace BPackageKit;
 
 class PackageManager {
 public:
-			struct Repository;
+			struct RemoteRepository;
-			typedef BObjectList<Repository> RepositoryList;
+			struct InstalledRepository;
+			typedef BObjectList<RemoteRepository> RemoteRepositoryList;
+			typedef BObjectList<InstalledRepository> InstalledRepositoryList;
 
 			enum {
 				ADD_INSTALLED_REPOSITORIES	= 0x01,
@@ -44,15 +46,15 @@ public:
 			BSolver*			Solver() const
 									{ return fSolver; }
 
-			const BSolverRepository* SystemRepository() const
+			const InstalledRepository* SystemRepository() const
-									{ return &fSystemRepository; }
+									{ return fSystemRepository; }
-			const BSolverRepository* CommonRepository() const
+			const InstalledRepository* CommonRepository() const
-									{ return &fCommonRepository; }
+									{ return fCommonRepository; }
-			const BSolverRepository* HomeRepository() const
+			const InstalledRepository* HomeRepository() const
-									{ return &fHomeRepository; }
+									{ return fHomeRepository; }
-			const BObjectList<BSolverRepository>& InstalledRepositories() const
+			const InstalledRepositoryList& InstalledRepositories() const
 									{ return fInstalledRepositories; }
-			const RepositoryList& OtherRepositories() const
+			const RemoteRepositoryList& OtherRepositories() const
 									{ return fOtherRepositories; }
 
 			void				Install(const char* const* packages,
@@ -71,7 +73,8 @@ private:
 			void				_PrintResult();
 			void				_ApplyPackageChanges();
 
-			void				_ClonePackageFile(Repository* repository,
+			void				_ClonePackageFile(
+									InstalledRepository* repository,
 									const BString& fileName,
 							 		const BEntry& entry) const;
 			int32				_FindBasePackage(const PackageList& packages,
@@ -80,11 +83,11 @@ private:
 private:
 			BPackageInstallationLocation fLocation;
 			BSolver*			fSolver;
-			BSolverRepository	fSystemRepository;
+			InstalledRepository* fSystemRepository;
-			BSolverRepository	fCommonRepository;
+			InstalledRepository* fCommonRepository;
-			BSolverRepository	fHomeRepository;
+			InstalledRepository* fHomeRepository;
-			BObjectList<BSolverRepository> fInstalledRepositories;
+			InstalledRepositoryList fInstalledRepositories;
-			RepositoryList		fOtherRepositories;
+			RemoteRepositoryList fOtherRepositories;
 			DecisionProvider	fDecisionProvider;
 			JobStateListener	fJobStateListener;
 			BContext			fContext;
@@ -93,8 +96,8 @@ private:
 };
 
 
-struct PackageManager::Repository : public BSolverRepository {
+struct PackageManager::RemoteRepository : public BSolverRepository {
-								Repository();
+								RemoteRepository();
 
 			status_t			Init(BPackageRoster& roster, BContext& context,
 									const char* name, bool refresh);
@@ -106,4 +109,17 @@ private:
 };
 
 
+struct PackageManager::InstalledRepository : public BSolverRepository {
+								InstalledRepository();
+
+			void				DisablePackage(BSolverPackage* package);
+
+private:
+			typedef BObjectList<BSolverPackage> PackageList;
+
+private:
+			PackageList			fDisabledPackages;
+};
+
+
 #endif	// PACKAGE_MANAGER_H

src/bin/pkgman/command_uninstall.cpp

@@ -86,8 +86,7 @@ UninstallCommand::Execute(int argc, const char* const* argv)
 		? B_PACKAGE_INSTALLATION_LOCATION_HOME
 		: B_PACKAGE_INSTALLATION_LOCATION_COMMON;
 	PackageManager packageManager(location,
-		PackageManager::ADD_INSTALLED_REPOSITORIES
+		PackageManager::ADD_INSTALLED_REPOSITORIES);
-			| PackageManager::ADD_REMOTE_REPOSITORIES);
 	packageManager.Uninstall(packages, packageCount);
 
 	return 0;