package daemon: Fix manual package [de/]activation

CommitTransactionHandler: Since the VolumeState is now always copied
(including the contained packages), we can't just use the passed in
packagesAlready{Added,Removed} sets, since they refer to the original
packages.

src/servers/package/CommitTransactionHandler.cpp

@@ -33,20 +33,18 @@ using namespace BPackageKit::BPrivate;
 
 
 CommitTransactionHandler::CommitTransactionHandler(Volume* volume,
-	PackageFileManager* packageFileManager, VolumeState* volumeState,
+	PackageFileManager* packageFileManager)
-	bool isActiveVolumeState, const PackageSet& packagesAlreadyAdded,
-	const PackageSet& packagesAlreadyRemoved)
 	:
 	fVolume(volume),
 	fPackageFileManager(packageFileManager),
-	fVolumeState(volumeState->Clone()),
+	fVolumeState(NULL),
-	fVolumeStateIsActive(isActiveVolumeState),
+	fVolumeStateIsActive(false),
 	fPackagesToActivate(),
 	fPackagesToDeactivate(),
 	fAddedPackages(),
 	fRemovedPackages(),
-	fPackagesAlreadyAdded(packagesAlreadyAdded),
+	fPackagesAlreadyAdded(),
-	fPackagesAlreadyRemoved(packagesAlreadyRemoved),
+	fPackagesAlreadyRemoved(),
 	fOldStateDirectory(),
 	fOldStateDirectoryRef(),
 	fOldStateDirectoryName(),
@@ -76,6 +74,31 @@ CommitTransactionHandler::~CommitTransactionHandler()
 }
 
 
+void
+CommitTransactionHandler::Init(VolumeState* volumeState,
+	bool isActiveVolumeState, const PackageSet& packagesAlreadyAdded,
+	const PackageSet& packagesAlreadyRemoved)
+{
+	fVolumeState = volumeState->Clone();
+	if (fVolumeState == NULL)
+		throw std::bad_alloc();
+
+	fVolumeStateIsActive = isActiveVolumeState;
+
+	for (PackageSet::const_iterator it = packagesAlreadyAdded.begin();
+			it != packagesAlreadyAdded.end(); ++it) {
+		Package* package = fVolumeState->FindPackage((*it)->FileName());
+		fPackagesAlreadyAdded.insert(package);
+	}
+
+	for (PackageSet::const_iterator it = packagesAlreadyRemoved.begin();
+			it != packagesAlreadyRemoved.end(); ++it) {
+		Package* package = fVolumeState->FindPackage((*it)->FileName());
+		fPackagesAlreadyRemoved.insert(package);
+	}
+}
+
+
 void
 CommitTransactionHandler::HandleRequest(BMessage* request, BMessage* reply)
 {
@@ -118,20 +141,15 @@ CommitTransactionHandler::HandleRequest(
 
 
 void
-CommitTransactionHandler::HandleRequest(const PackageSet& packagesAdded,
+CommitTransactionHandler::HandleRequest()
-	const PackageSet& packagesRemoved)
 {
-	// Copy package sets to fPackagesToActivate/fPackagesToDeactivate. The
+	for (PackageSet::const_iterator it = fPackagesAlreadyAdded.begin();
-	// given sets are assumed to be identical to the ones specified in the
+		it != fPackagesAlreadyAdded.end(); ++it) {
-	// constructor invocation (fPackagesAlreadyAdded,
-	// fPackagesAlreadyRemoved).
-	for (PackageSet::const_iterator it = packagesAdded.begin();
-		it != packagesAdded.end(); ++it) {
 		if (!fPackagesToActivate.AddItem(*it))
 			throw std::bad_alloc();
 	}
 
-	fPackagesToDeactivate = packagesRemoved;
+	fPackagesToDeactivate = fPackagesAlreadyRemoved;
 
 	_ApplyChanges(NULL);
 }
@@ -263,9 +281,6 @@ CommitTransactionHandler::_ReadPackagesToActivate(
 void
 CommitTransactionHandler::_ApplyChanges(BMessage* reply)
 {
-	if (fVolumeState == NULL)
-		throw std::bad_alloc();
-
 	// create an old state directory
 	_CreateOldStateDirectory(reply);
 

src/servers/package/CommitTransactionHandler.h

@@ -25,20 +25,22 @@ typedef std::set<std::string> StringSet;
 class CommitTransactionHandler {
 public:
 								CommitTransactionHandler(Volume* volume,
-									PackageFileManager* packageFileManager,
+									PackageFileManager* packageFileManager);
-									VolumeState* volumeState,
+								~CommitTransactionHandler();
+
+			void				Init(VolumeState* volumeState,
 									bool isActiveVolumeState,
 									const PackageSet& packagesAlreadyAdded,
 									const PackageSet& packagesAlreadyRemoved);
-								~CommitTransactionHandler();
 
 			void				HandleRequest(BMessage* request,
 									BMessage* reply);
 			void				HandleRequest(
 									const BActivationTransaction& transaction,
 									BMessage* reply);
-			void				HandleRequest(const PackageSet& packagesAdded,
+			void				HandleRequest();
-									const PackageSet& packagesRemoved);
+									// uses packagesAlreadyAdded and
+									// packagesAlreadyRemoved from Init()
 
 			void				Revert();
 
@@ -145,8 +147,8 @@ private:
 			PackageSet			fPackagesToDeactivate;
 			PackageSet			fAddedPackages;
 			PackageSet			fRemovedPackages;
-			const PackageSet&	fPackagesAlreadyAdded;
+			PackageSet			fPackagesAlreadyAdded;
-			const PackageSet&	fPackagesAlreadyRemoved;
+			PackageSet			fPackagesAlreadyRemoved;
 			BDirectory			fOldStateDirectory;
 			node_ref			fOldStateDirectoryRef;
 			BString				fOldStateDirectoryName;

src/servers/package/Volume.cpp

@@ -526,11 +526,11 @@ Volume::HandleCommitTransactionRequest(BMessage* message)
 		return;
 
 	// perform the request
-	PackageSet dummy;
+	CommitTransactionHandler handler(this, fPackageFileManager);
-	CommitTransactionHandler handler(this, fPackageFileManager, fLatestState,
-		fLatestState == fActiveState, dummy, dummy);
 	int32 error;
 	try {
+		PackageSet dummy;
+		handler.Init(fLatestState, fLatestState == fActiveState, dummy, dummy);
 		handler.HandleRequest(message, &reply);
 		_SetLatestState(handler.DetachVolumeState(),
 			handler.IsActiveVolumeState());
@@ -709,12 +709,12 @@ Volume::ProcessPendingPackageActivationChanges()
 		return;
 
 	// perform the request
-	CommitTransactionHandler handler(this, fPackageFileManager, fLatestState,
+	CommitTransactionHandler handler(this, fPackageFileManager);
-		fLatestState == fActiveState, fPackagesToBeActivated,
-		fPackagesToBeDeactivated);
 	int32 error;
 	try {
-		handler.HandleRequest(fPackagesToBeActivated, fPackagesToBeDeactivated);
+		handler.Init(fLatestState, fLatestState == fActiveState,
+			fPackagesToBeActivated, fPackagesToBeDeactivated);
+		handler.HandleRequest();
 		_SetLatestState(handler.DetachVolumeState(),
 			handler.IsActiveVolumeState());
 		error = B_DAEMON_OK;
@@ -797,11 +797,11 @@ Volume::CommitTransaction(const BActivationTransaction& transaction,
 	BDaemonClient::BCommitTransactionResult& _result)
 {
 	// perform the request
-	CommitTransactionHandler handler(this, fPackageFileManager, fLatestState,
+	CommitTransactionHandler handler(this, fPackageFileManager);
-		fLatestState == fActiveState, packagesAlreadyAdded,
-		packagesAlreadyRemoved);
 	int32 error;
 	try {
+		handler.Init(fLatestState, fLatestState == fActiveState,
+			packagesAlreadyAdded, packagesAlreadyRemoved);
 		handler.HandleRequest(transaction, NULL);
 		_SetLatestState(handler.DetachVolumeState(),
 			handler.IsActiveVolumeState());