package kit: Add pre-uninstall scripts feature.

Just like post-install scripts which run shortly after a package is
installed, pre-uninstall scripts are run just before a package is
removed.  Implements enhancement #13427

* Fix script exit code handling vs script launch errors.
* Bump package and repo file version numbers due to new attribute,
  unfortunately makes new .hpkg files not backwards compatible.
* Add pre-uninstall functionality, mostly cloning the post-install
  except in a few places.
* Discover that _RunQueuedScripts() is never called, a future TODO:?
* Update package documentation for pre-uninstall scripts, and use of
  the boot/post-install directory.

Change-Id: I45596255ce74bc102f6e5b606cbf83e4e4347a17
Reviewed-on: https://review.haiku-os.org/c/haiku/+/1504
Reviewed-by: Alexander G. M. Smith <[email protected]>
Reviewed-by: Adrien Destugues <[email protected]>

docs/develop/packages/BuildingPackages.rst

@@ -178,7 +178,11 @@ The supported attributes are:
 - ``groups``: A list of names of Unix groups the packaged software requires.
 - ``post-install-scripts``: A list of paths of files included in the package,
   which shall be executed on package activation. Each path must start with
-  "boot/post-install/".
+  "boot/post-install/". All the files in that directory are also run on first
+  boot after installing or copying the OS to a new disk.
+- ``pre-uninstall-scripts``: A list of paths of files included in the package,
+  which shall be executed on package deactivation. For consistency, each path
+  should start with "boot/pre-uninstall/".
 
 Version Strings
 ---------------

docs/develop/packages/FileFormat.rst

@@ -87,7 +87,7 @@ total_size
 
 minor_version
   The minor version of the HPKG format the file conforms to. The current minor
-  version is 0 (B_HPKG_MINOR_VERSION). Additions of new attributes to the
+  version is 1 (B_HPKG_MINOR_VERSION). Additions of new attributes to the
   attributes or TOC sections should generally only increment the minor version.
   When a file with a greater minor version is encountered, the reader should
   ignore unknown attributes.
@@ -915,7 +915,18 @@ B_HPKG_ATTRIBUTE_ID_PACKAGE_POST_INSTALL_SCRIPT ("package:post-install-script")
   :Value: Relative path of a script that shall be executed after package
     activation.
   :Allowed Values: Installation location relative path of a file included in the
-    package.
+    package.  Must start with "boot/post-install/", so besides being run after
+    package installation, it also gets run on the first boot after the OS is
+    installed.
+  :Child Attributes: none
+
+B_HPKG_ATTRIBUTE_ID_PACKAGE_PRE_UNINSTALL_SCRIPT ("package:pre-uninstall-script")
+  :Type: string
+  :Value: Relative path of a script that shall be executed before package
+    deactivation.
+  :Allowed Values: Installation location relative path of a file included in
+    the package.  For consistency, it is recommended to start with
+    "boot/pre-uninstall/".
   :Child Attributes: none
 
 Haiku Package Repository Format
@@ -994,8 +1005,8 @@ total_size
 
 minor_version
   The minor version of the HPKR format the file conforms to. The current minor
-  version is 0 (B_HPKG_REPO_MINOR_VERSION). Additions of new attributes to the
-  attributes section shouldgenerally only increment the minor version. When a
+  version is 1 (B_HPKG_REPO_MINOR_VERSION). Additions of new attributes to the
+  attributes section should generally only increment the minor version. When a
   file with a greater minor version is encountered, the reader should ignore
   unknown attributes.
 

docs/develop/packages/PackagingPolicy.rst

@@ -127,7 +127,14 @@ Post-Installation Scripts
 =========================
 A package may include one or more post-installation scripts. The scripts are
 executed whenever the package is activated (for the first time, but also after
-package updates). They shall be placed in "boot/post-install" and declared
-explicitly by the package (via POST_INSTALL_SCRIPTS in the build recipe). A
-post-install script should be considered the last resort. It should only be
-used, if there's no reasonable alternative.
+package updates and first boot of a newly installed OS). They shall be placed
+in "boot/post-install" and declared explicitly by the package (via
+POST_INSTALL_SCRIPTS in the build recipe). A post-install script should be
+considered the last resort. It should only be used, if there's no reasonable
+alternative.  A typical use would be to create a desktop icon that the user
+can move around or delete.
+
+Pre-Uninstallation Scripts
+=========================
+These undo the effects of a post-installation script and usually are put
+into "boot/pre-uninstall".  A typical use is to remove desktop icons.

headers/os/package/CommitTransactionResult.h

@@ -59,6 +59,9 @@ public:
 				B_POST_INSTALL_SCRIPT_NOT_FOUND,
 				B_STARTING_POST_INSTALL_SCRIPT_FAILED,
 				B_POST_INSTALL_SCRIPT_FAILED,
+				B_PRE_UNINSTALL_SCRIPT_NOT_FOUND,
+				B_STARTING_PRE_UNINSTALL_SCRIPT_FAILED,
+				B_PRE_UNINSTALL_SCRIPT_FAILED,
 			};
 
 public:

headers/os/package/PackageInfo.h

@@ -96,6 +96,7 @@ public:
 			const BStringList&	Groups() const;
 
 			const BStringList&	PostInstallScripts() const;
+			const BStringList&	PreUninstallScripts() const;
 
 			const BObjectList<BPackageResolvable>&	ProvidesList() const;
 			const BObjectList<BPackageResolvableExpression>&
@@ -159,6 +160,9 @@ public:
 			void				ClearPostInstallScripts();
 			status_t			AddPostInstallScript(const BString& path);
 
+			void				ClearPreUninstallScripts();
+			status_t			AddPreUninstallScript(const BString& path);
+
 			void				ClearProvidesList();
 			status_t			AddProvides(const BPackageResolvable& provides);
 
@@ -297,6 +301,7 @@ private:
 			BStringList			fGroups;
 
 			BStringList			fPostInstallScripts;
+			BStringList			fPreUninstallScripts;
 
 			ResolvableList		fProvidesList;
 

headers/os/package/PackageInfoAttributes.h

@@ -51,6 +51,8 @@ enum BPackageInfoAttributeID {
 								// list of (Unix) groups defined/needed
 	B_PACKAGE_INFO_POST_INSTALL_SCRIPTS,
 								// list of scripts to be executed post-install
+	B_PACKAGE_INFO_PRE_UNINSTALL_SCRIPTS,
+								// list of scripts to be run before uninstalling
 	//
 	B_PACKAGE_INFO_ENUM_COUNT,
 };

headers/os/package/hpkg/HPKGDefs.h

@@ -18,11 +18,11 @@ namespace BHPKG {
 enum {
 	B_HPKG_MAGIC				= 'hpkg',
 	B_HPKG_VERSION				= 2,
-	B_HPKG_MINOR_VERSION		= 0,
+	B_HPKG_MINOR_VERSION		= 1,
 	//
 	B_HPKG_REPO_MAGIC			= 'hpkr',
 	B_HPKG_REPO_VERSION			= 2,
-	B_HPKG_REPO_MINOR_VERSION	= 0
+	B_HPKG_REPO_MINOR_VERSION	= 1
 };
 
 

headers/os/package/hpkg/PackageAttributes.h

@@ -82,3 +82,10 @@ B_DEFINE_HPKG_ATTRIBUTE(52, STRING,	"package:post-install-script",
 B_DEFINE_HPKG_ATTRIBUTE(53, UINT,	"package:is-writable-directory",
 	PACKAGE_IS_WRITABLE_DIRECTORY)
 B_DEFINE_HPKG_ATTRIBUTE(54, STRING,	"package",				PACKAGE)
+B_DEFINE_HPKG_ATTRIBUTE(55, STRING,	"package:pre-uninstall-script",
+	PACKAGE_PRE_UNINSTALL_SCRIPT)
+// Note: add new entries at the end to avoid breaking index numbers, which are
+// in previously built .hpkg files the build process downloads from elsewhere.
+// Also remember to bump B_HPKG_MINOR_VERSION and B_HPKG_REPO_MINOR_VERSION.
+// And yes, the build (on Haiku) runs packaging tools compiled using your code,
+// which makes it extra fun to debug :-)

src/bin/package/PackageInfoPrinter.h

@@ -96,9 +96,9 @@ public:
 		for (int32 i = 0; i < count; i++)
 			PrintPostInstallScript(info.PostInstallScripts().StringAt(i));
 
-		count = info.PostInstallScripts().CountStrings();
+		count = info.PreUninstallScripts().CountStrings();
 		for (int32 i = 0; i < count; i++)
-			PrintPostInstallScript(info.PostInstallScripts().StringAt(i));
+			PrintPreUninstallScript(info.PreUninstallScripts().StringAt(i));
 
 		if (!info.InstallPath().IsEmpty())
 			PrintInstallPath(info.InstallPath());
@@ -203,6 +203,10 @@ public:
 				PrintPostInstallScript(value.string);
 				break;
 
+			case B_PACKAGE_INFO_PRE_UNINSTALL_SCRIPTS:
+				PrintPreUninstallScript(value.string);
+				break;
+
 			case B_PACKAGE_INFO_INSTALL_PATH:
 				PrintInstallPath(value.string);
 				break;
@@ -379,7 +383,12 @@ public:
 
 	void PrintPostInstallScript(const char* script) const
 	{
-		printf("\tpost install script: %s\n", script);
+		printf("\tpost-install script: %s\n", script);
+	}
+
+	void PrintPreUninstallScript(const char* script) const
+	{
+		printf("\tpre-uninstall script: %s\n", script);
 	}
 
 	void PrintInstallPath(const char* path) const

src/kits/package/CommitTransactionResult.cpp

@@ -162,6 +162,18 @@ BTransactionIssue::ToString() const
 			messageTemplate = "The post-installation script "
 				" \"%path1%\" failed with exit code %exitCode%.";
 			break;
+		case B_PRE_UNINSTALL_SCRIPT_NOT_FOUND:
+			messageTemplate = "Failed to find pre-uninstall script "
+				" \"%path1%\": %error%.";
+			break;
+		case B_STARTING_PRE_UNINSTALL_SCRIPT_FAILED:
+			messageTemplate = "Failed to run pre-uninstall script "
+				" \"%path1%\": %error%.";
+			break;
+		case B_PRE_UNINSTALL_SCRIPT_FAILED:
+			messageTemplate = "The pre-uninstall script "
+				" \"%path1%\" failed with exit code %exitCode%.";
+			break;
 	}
 
 	BString message(messageTemplate);

src/kits/package/PackageInfo.cpp

@@ -55,7 +55,8 @@ const char* const BPackageInfo::kElementNames[B_PACKAGE_INFO_ENUM_COUNT] = {
 	"user-settings-files",
 	"users",
 	"groups",
-	"post-install-scripts"
+	"post-install-scripts",
+	"pre-uninstall-scripts"
 };
 
 
@@ -180,6 +181,7 @@ BPackageInfo::BPackageInfo()
 	fUsers(4, true),
 	fGroups(4),
 	fPostInstallScripts(4),
+	fPreUninstallScripts(4),
 	fProvidesList(20, true),
 	fRequiresList(20, true),
 	fSupplementsList(20, true),
@@ -204,6 +206,7 @@ BPackageInfo::BPackageInfo(BMessage* archive, status_t* _error)
 	fUsers(4, true),
 	fGroups(4),
 	fPostInstallScripts(4),
+	fPreUninstallScripts(4),
 	fProvidesList(20, true),
 	fRequiresList(20, true),
 	fSupplementsList(20, true),
@@ -237,6 +240,8 @@ BPackageInfo::BPackageInfo(BMessage* archive, status_t* _error)
 		&& (error = _ExtractStringList(archive, "groups", fGroups)) == B_OK
 		&& (error = _ExtractStringList(archive, "post-install-scripts",
 			fPostInstallScripts)) == B_OK
+		&& (error = _ExtractStringList(archive, "pre-uninstall-scripts",
+			fPreUninstallScripts)) == B_OK
 		&& (error = _ExtractResolvables(archive, "provides", fProvidesList))
 			== B_OK
 		&& (error = _ExtractResolvableExpressions(archive, "requires",
@@ -554,6 +559,13 @@ BPackageInfo::PostInstallScripts() const
 }
 
 
+const BStringList&
+BPackageInfo::PreUninstallScripts() const
+{
+	return fPreUninstallScripts;
+}
+
+
 const BObjectList<BPackageResolvable>&
 BPackageInfo::ProvidesList() const
 {
@@ -852,6 +864,13 @@ BPackageInfo::ClearPostInstallScripts()
 }
 
 
+void
+BPackageInfo::ClearPreUninstallScripts()
+{
+	fPreUninstallScripts.MakeEmpty();
+}
+
+
 status_t
 BPackageInfo::AddPostInstallScript(const BString& path)
 {
@@ -859,6 +878,13 @@ BPackageInfo::AddPostInstallScript(const BString& path)
 }
 
 
+status_t
+BPackageInfo::AddPreUninstallScript(const BString& path)
+{
+	return fPreUninstallScripts.Add(path) ? B_OK : B_NO_MEMORY;
+}
+
+
 void
 BPackageInfo::ClearProvidesList()
 {
@@ -992,6 +1018,7 @@ BPackageInfo::Clear()
 	fUsers.MakeEmpty();
 	fGroups.MakeEmpty();
 	fPostInstallScripts.MakeEmpty();
+	fPreUninstallScripts.MakeEmpty();
 	fRequiresList.MakeEmpty();
 	fProvidesList.MakeEmpty();
 	fSupplementsList.MakeEmpty();
@@ -1031,6 +1058,8 @@ BPackageInfo::Archive(BMessage* archive, bool deep) const
 		|| (error = archive->AddStrings("groups", fGroups)) != B_OK
 		|| (error = archive->AddStrings("post-install-scripts",
 			fPostInstallScripts)) != B_OK
+		|| (error = archive->AddStrings("pre-uninstall-scripts",
+			fPreUninstallScripts)) != B_OK
 		|| (error = _AddResolvables(archive, "provides", fProvidesList)) != B_OK
 		|| (error = _AddResolvableExpressions(archive, "requires",
 			fRequiresList)) != B_OK
@@ -1080,6 +1109,7 @@ BPackageInfo::GetConfigString(BString& _string) const
 		.Write("users", fUsers)
 		.Write("groups", fGroups)
 		.Write("post-install-scripts", fPostInstallScripts)
+		.Write("pre-uninstall-scripts", fPreUninstallScripts)
 		.Write("provides", fProvidesList)
 		.BeginRequires(fBasePackage)
 			.Write("requires", fRequiresList)

src/kits/package/PackageInfoContentHandler.cpp

@@ -149,6 +149,10 @@ BPackageInfoContentHandler::HandlePackageAttribute(
 		case B_PACKAGE_INFO_POST_INSTALL_SCRIPTS:
 			return fPackageInfo.AddPostInstallScript(value.string);
 
+		case B_PACKAGE_INFO_PRE_UNINSTALL_SCRIPTS:
+			fPackageInfo.AddPreUninstallScript(value.string);
+			break;
+
 		default:
 			if (fErrorOutput != NULL) {
 				fErrorOutput->PrintError(

src/kits/package/PackageInfoParser.cpp

@@ -1054,6 +1054,10 @@ BPackageInfo::Parser::_Parse(BPackageInfo* packageInfo)
 				_ParseStringList(&packageInfo->fPostInstallScripts);
 				break;
 
+			case B_PACKAGE_INFO_PRE_UNINSTALL_SCRIPTS:
+				_ParseStringList(&packageInfo->fPreUninstallScripts);
+				break;
+
 			case B_PACKAGE_INFO_PROVIDES:
 				_ParseResolvableList(&packageInfo->fProvidesList);
 				break;

src/kits/package/hpkg/ReaderImplBase.cpp

@@ -668,6 +668,11 @@ ReaderImplBase::PackageAttributeHandler::HandleAttribute(
 				value.string);
 			break;
 
+		case B_HPKG_ATTRIBUTE_ID_PACKAGE_PRE_UNINSTALL_SCRIPT:
+			fPackageInfoValue.SetTo(B_PACKAGE_INFO_PRE_UNINSTALL_SCRIPTS,
+				value.string);
+			break;
+
 		default:
 			if (context->ignoreUnknownAttributes)
 				break;

src/kits/package/hpkg/WriterImplBase.cpp

@@ -559,6 +559,10 @@ WriterImplBase::RegisterPackageInfo(PackageAttributeList& attributeList,
 	_AddStringAttributeList(B_HPKG_ATTRIBUTE_ID_PACKAGE_POST_INSTALL_SCRIPT,
 		packageInfo.PostInstallScripts(), attributeList);
 
+	// pre uninstall script list
+	_AddStringAttributeList(B_HPKG_ATTRIBUTE_ID_PACKAGE_PRE_UNINSTALL_SCRIPT,
+		packageInfo.PreUninstallScripts(), attributeList);
+
 	// checksum (optional, only exists in repositories)
 	_AddStringAttributeIfNotEmpty(B_HPKG_ATTRIBUTE_ID_PACKAGE_CHECKSUM,
 		packageInfo.Checksum(), attributeList);

src/servers/package/CommitTransactionHandler.cpp

@@ -391,13 +391,18 @@ CommitTransactionHandler::_ApplyChanges()
 	// move packages to activate to packages directory
 	_AddPackagesToActivate();
 
+	// run pre-uninstall scripts, before their packages vanish.
+	_RunPreUninstallScripts();
+
 	// activate/deactivate packages
 	_ChangePackageActivation(fAddedPackages, fRemovedPackages);
 
+	// run post-installation scripts
 	if (fVolumeStateIsActive) {
-		// run post-installation scripts
 		_RunPostInstallScripts();
 	} else {
+		// need to reboot to finish installation so queue up scripts as
+		// symbolic links in a work directory, which will run later.
 		_QueuePostInstallScripts();
 	}
 
@@ -1303,7 +1308,7 @@ CommitTransactionHandler::_RunPostInstallScripts()
 		const BStringList& scripts = package->Info().PostInstallScripts();
 		int32 count = scripts.CountStrings();
 		for (int32 i = 0; i < count; i++)
-			_RunPostInstallScript(package, scripts.StringAt(i));
+			_RunPostOrPreScript(package, scripts.StringAt(i), true);
 	}
 
 	fCurrentPackage = NULL;
@@ -1311,19 +1316,42 @@ CommitTransactionHandler::_RunPostInstallScripts()
 
 
 void
-CommitTransactionHandler::_RunPostInstallScript(Package* package,
-	const BString& script)
+CommitTransactionHandler::_RunPreUninstallScripts()
 {
+	// Note this runs in the correct order, so dependents get uninstalled before
+	// the packages they depend on.  No need for a reversed loop.
+	for (PackageSet::iterator it = fPackagesToDeactivate.begin();
+		it != fPackagesToDeactivate.end(); ++it) {
+		Package* package = *it;
+		fCurrentPackage = package;
+		const BStringList& scripts = package->Info().PreUninstallScripts();
+		int32 count = scripts.CountStrings();
+		for (int32 i = 0; i < count; i++)
+			_RunPostOrPreScript(package, scripts.StringAt(i), false);
+	}
+
+	fCurrentPackage = NULL;
+}
+
+
+void
+CommitTransactionHandler::_RunPostOrPreScript(Package* package,
+	const BString& script, bool postNotPre)
+{
+	const char *postOrPreInstallWording = postNotPre
+		? "post-installation" : "pre-uninstall";
 	BDirectory rootDir(&fVolume->RootDirectoryRef());
 	BPath scriptPath(&rootDir, script);
 	status_t error = scriptPath.InitCheck();
 	if (error != B_OK) {
-		ERROR("Volume::CommitTransactionHandler::_RunPostInstallScript(): "
-			"failed get path of post-installation script \"%s\" of package "
-			"%s: %s\n", script.String(), package->FileName().String(),
-			strerror(error));
-		_AddIssue(TransactionIssueBuilder(
-				BTransactionIssue::B_POST_INSTALL_SCRIPT_NOT_FOUND)
+		ERROR("Volume::CommitTransactionHandler::_RunPostOrPreScript(): "
+			"failed get path of %s script \"%s\" of package "
+			"%s: %s\n",
+			postOrPreInstallWording, script.String(),
+			package->FileName().String(), strerror(error));
+		_AddIssue(TransactionIssueBuilder(postNotPre
+				? BTransactionIssue::B_POST_INSTALL_SCRIPT_NOT_FOUND
+				: BTransactionIssue::B_PRE_UNINSTALL_SCRIPT_NOT_FOUND)
 			.SetPath1(script)
 			.SetSystemError(error));
 		return;
@@ -1332,19 +1360,21 @@ CommitTransactionHandler::_RunPostInstallScript(Package* package,
 	errno = 0;
 	int result = system(scriptPath.Path());
 	if (result != 0) {
-		ERROR("Volume::CommitTransactionHandler::_RunPostInstallScript(): "
-			"running post-installation script \"%s\" of package %s "
-			"failed: %d (errno: %s)\n", script.String(),
-			package->FileName().String(), result,
-			strerror(errno));
-		if (result < 0 && errno != 0) {
-			_AddIssue(TransactionIssueBuilder(
-					BTransactionIssue::B_POST_INSTALL_SCRIPT_FAILED)
+		ERROR("Volume::CommitTransactionHandler::_RunPostOrPreScript(): "
+			"running %s script \"%s\" of package %s "
+			"failed: %d (errno: %s)\n",
+			postOrPreInstallWording, script.String(),
+			package->FileName().String(), result, strerror(errno));
+		if (result < 0 || result == 127) { // bash shell returns 127 on failure.
+			_AddIssue(TransactionIssueBuilder(postNotPre
+					? BTransactionIssue::B_STARTING_POST_INSTALL_SCRIPT_FAILED
+					: BTransactionIssue::B_STARTING_PRE_UNINSTALL_SCRIPT_FAILED)
 				.SetPath1(BString(scriptPath.Path()))
 				.SetSystemError(errno));
-		} else {
-			_AddIssue(TransactionIssueBuilder(
-					BTransactionIssue::B_STARTING_POST_INSTALL_SCRIPT_FAILED)
+		} else { // positive is an exit code from the script itself.
+			_AddIssue(TransactionIssueBuilder(postNotPre
+					? BTransactionIssue::B_POST_INSTALL_SCRIPT_FAILED
+					: BTransactionIssue::B_PRE_UNINSTALL_SCRIPT_FAILED)
 				.SetPath1(BString(scriptPath.Path()))
 				.SetExitCode(result));
 		}

src/servers/package/CommitTransactionHandler.h

@@ -97,8 +97,10 @@ private:
 			void				_RevertUserGroupChanges();
 
 			void				_RunPostInstallScripts();
-			void				_RunPostInstallScript(Package* package,
-									const BString& script);
+			void				_RunPreUninstallScripts();
+			void				_RunPostOrPreScript(Package* package,
+									const BString& script,
+									bool postNotPre);
 
 			void				_QueuePostInstallScripts();
 

src/servers/package/Volume.h

@@ -160,7 +160,7 @@ private:
 			status_t			_InitLatestState();
 			status_t			_InitLatestStateFromActivatedPackages();
 			status_t			_GetActivePackages(int fd);
-			void				_RunQueuedScripts();
+			void				_RunQueuedScripts(); // TODO: Never called, fix?
 			bool				_CheckActivePackagesMatchLatestState(
 									PackageFSGetPackageInfosRequest* request);
 			void				_SetLatestState(VolumeState* state,