Handle extension-already-enabled output inside installer plugin

Rather than in bin/compile's attempt-native-installs-of-polyfill-provided-packages loop, we can figure out if a requested package doesn't actually trigger an install event inside the plugin.

This can happen when a polyfill declares that it provides a particular package, but that package is already there (e.g. because the extension is bundled with PHP and always enabled).

We previously had to handle this on the caller side in bin/compile by tee-ing the installer output to a special file we'd inspect afterwards, since a successful install of a ".native" variant of a package and a "nothing to install or update" outcome both resulted in a 0 exit status, so we could only tell the two apart by checking the output.

GUS-W-18557233

Author: dzuelke

bin/compile

@@ -512,14 +512,10 @@ else
 	EOF
 fi
 
-# close display output FD (we make new ones for each iteration next)
-exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}>&-
-
 export -n providedextensionslog_file_path # export no longer needed
 if [[ -s "$providedextensionslog_file_path" ]]; then
 	notice_inline "detected userland polyfill packages for PHP extensions"
 	notice_inline "now attempting to install native extension packages"
-	current_install_log=$(mktemp -t "currentinstall.log.XXXXXX" -u) # for capturing the single "composer install" output
 	# the platform installer recorded userland packages that "provide" a PHP extension
 	# for each occurrence (source package providing other package(s)) in the list,
 	# we will now attempt to install the real native extension packages (rest of the line)
@@ -531,21 +527,14 @@ if [[ -s "$providedextensionslog_file_path" ]]; then
 			ext_package=$(cut -d":" -f1 <<< "$extension") # extract name from "ext-foo:1.2.3" string
 			ext_name=${ext_package#"heroku-sys/"} # no "heroku-sys/" prefix for human output
 			# run a `composer require`, confined to the package we're attempting, allowing any version
-			# we're appending to install.log using tee, and using another new "display output" FD to redirect to a new single-step log (tee truncates for us)
-			# this log is checked later to see if a package install actually happened, or if it was already enabled (since both exit status 0)
-			exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}> >(tee "$current_install_log")
+			#   (the .native "replace"d variant in each extension matches the regular version, so the constraint for that regular version is enough)
 			# NO_COLOR=1 suppresses the download progress meter whose ANSI cursor codes trip up output in Heroku Dashboard
-			if NO_COLOR=1 composer require -d "$build_dir/.heroku/php" "${ext_package}.native:*" >> $build_dir/.heroku/php/install.log 2>&1; then
-				# package added successfully
-				[[ -s "$current_install_log" ]] || { echo "- ${ext_name} (already enabled)" | indent; } # if nothing was actually installed above, that means the dependency was already satisfied (i.e. extension is statically built into PHP); we need to echo something to that effect
-			else
+			if ! NO_COLOR=1 composer require -d "$build_dir/.heroku/php" "${ext_package}.native:*" >> $build_dir/.heroku/php/install.log 2>&1; then
 				# composer did not succeed; this means no package that matches all existing package's requirements was found
 				notice_inline "no suitable native version of ${ext_name} available"
 			fi
-			exec {PHP_PLATFORM_INSTALLER_DISPLAY_OUTPUT_FDNO}>&-
 		done
 	done {fd_num}< "$providedextensionslog_file_path" # use bash 4.1+ automatic file descriptor allocation (better than hardcoding e.g. "3<"), otherwise this loop's stdin (the lines of the file) will be consumed by programs called inside the loop
-	rm "$current_install_log"
 	exec {fd_num}>&-
 fi
 

support/installer/src/ComposerInstaller.php

@@ -16,7 +16,9 @@ public function getInstallPath(PackageInterface $package)
 
 	public static function formatHerokuSysName(string $name): string
 	{
-		return sscanf($name, "heroku-sys/%s")[0] ?? $name;
+		// strip a "heroku-sys/" prefix if it exists, and in that case, also a ".native" postfix
+		// this turns our internal "heroku-sys/ext-foobar.native" or "heroku-sys/php" names into "ext-foobar" or "php" for display output
+		return preg_replace('#^(heroku-sys/)(.+?)(?(1).native)?$#', '$2', $name);
 	}
 
 	/**

support/installer/src/ComposerInstallerPlugin.php

@@ -8,9 +8,9 @@
 use Symfony\Component\Console\Helper\{HelperSet,ProgressBar};
 use Symfony\Component\Console\Input\ArrayInput;
 use Symfony\Component\Console\Output\StreamOutput;
-use Composer\Plugin\{PluginEvents,PluginInterface,PreFileDownloadEvent,PostFileDownloadEvent};
+use Composer\Plugin\{PluginEvents,PluginInterface,PreFileDownloadEvent,PostFileDownloadEvent,PrePoolCreateEvent};
 use Composer\EventDispatcher\EventSubscriberInterface;
-use Composer\Installer\{PackageEvent,PackageEvents};
+use Composer\Installer\{InstallerEvent,InstallerEvents,PackageEvent,PackageEvents};
 use Composer\Package\PackageInterface;
 use Composer\Util\Filesystem;
 
@@ -120,14 +120,57 @@ public function uninstall(Composer $composer, IOInterface $io)
 	public static function getSubscribedEvents()
 	{
 		return [
+			PluginEvents::PRE_POOL_CREATE => 'onPrePoolCreate',
+			InstallerEvents::PRE_OPERATIONS_EXEC => 'onPreOperationsExec',
 			PluginEvents::PRE_FILE_DOWNLOAD => 'onPreFileDownload',
 			PluginEvents::POST_FILE_DOWNLOAD => 'onPostFileDownload',
 			PackageEvents::PRE_PACKAGE_INSTALL => 'onPrePackageInstall',
 			PackageEvents::POST_PACKAGE_INSTALL => 'onPostPackageInstall',
 		];
 	}
 
-	// this fires when a download starts
+	// This does not fire on initial install, as the plugin gets installed as part of that, but the event fires before the plugin install.
+	// Just what we want, since the logic in here is for the "ext-foobar.native" install attempts after the main packages installation.
+	// For those invocations, the plugin is already enabled, and this event handler fires.
+	public function onPrePoolCreate(PrePoolCreateEvent $event)
+	{
+		// the list of explicitly requested packages from e.g. a 'composer require ext-foobar.native:*'
+		// we remember this for later, so we can output a message about already-enabled extensions
+		// this will be e.g. ["heroku-sys/ext-mbstring.native"]
+		$this->requestedPackages = $event->getRequest()->getUpdateAllowList();
+	}
+	
+	// This does not fire on initial install, as the plugin gets installed as part of that, but the event fires before the plugin install.
+	// Just what we want, since the logic in here is for the "ext-foobar.native" install attempts after the main packages installation.
+	// For those invocations, the plugin is already enabled, and this event handler fires.
+	public function onPreOperationsExec(InstallerEvent $event)
+	{
+		// From the list of operations, we are getting all packages due for install.
+		// For each package, we check the "replaces" declarations.
+		// For instance, "heroku-sys/ext-mbstring" will declare that it replaces "heroku-sys/ext-mbstring.native".
+		// The "replaces" array is keyed by "replacement destination", so it's e.g.:
+		//   {"heroku-sys/ext-mbstring.native": {Composer\Package\Link(source="heroku-sys/ext-mbstring",target="heroku-sys/ext-mbstring.native")}}
+		// For any package found here that is in the requestAllowList made in onPrePoolCreate, this means a regular installation,
+		// the Downloader will print install progress in this case.
+		// What we are really looking for, though, is packages in requestAllowList that are not in our list of operations,
+		// that means the extension is already enabled (either installed previously, or enabled in PHP by default).
+		// Because no installer event will fire in that case (nothing gets installed, after all), we want to output a message here.
+		$installs = [];
+		foreach($event->getTransaction()->getOperations() as $operation) {
+			// add the package itself, just for completeness
+			if ($operation->getOperationType() == "install") {
+				$installs[] = $operation->getPackage()->getPrettyName();
+			}
+			// add all "replace" declarations from the package
+			$installs = array_merge($installs, array_keys($operation->getPackage()->getReplaces()));
+		}
+		foreach(array_diff($this->requestedPackages, $installs) as $requestedPackageNotInstalled) {
+			$this->displayIo->write(sprintf('<indent>-</indent> <info>%s</info> (already enabled)', ComposerInstaller::formatHerokuSysName($requestedPackageNotInstalled)));
+		}
+	}
+	
+	// Because our plugin declares "plugin-modifies-downloads", Composer installs it first.
+	// After that, all other package downloads trigger this event.
 	public function onPreFileDownload(PreFileDownloadEvent $event)
 	{
 		$package = $event->getContext();
@@ -146,7 +189,9 @@ public function onPreFileDownload(PreFileDownloadEvent $event)
 		}
 	}
 
-	// this fires when a download finishes, so we can output progress info
+	// Because our plugin declares "plugin-modifies-downloads", Composer installs it first.
+	// After that, all other package downloads trigger this event.
+	// We use it to output progress info when a download finishes
 	// (Downloader::download returns a promise for parallel downloads, so would be as useless as onPreFileDownload above)
 	public function onPostFileDownload(PostFileDownloadEvent $event)
 	{
@@ -158,8 +203,9 @@ public function onPostFileDownload(PostFileDownloadEvent $event)
 		}
 	}
 
-	// this fires for every package install
-	// nothing to do for us here except to clear a progress bar if it exists
+	// Because our plugin declares "plugin-modifies-install-path", Composer installs it first.
+	// After that, all other package installs trigger this event.
+	// Nothing to do for us here except to clear a progress bar if it exists
 	public function onPrePackageInstall(PackageEvent $event)
 	{
 		// clear our progress bar, since we're done with downloads
@@ -176,6 +222,9 @@ public function onPrePackageInstall(PackageEvent $event)
 		}
 	}
 
+	// Because our plugin declares "plugin-modifies-install-path", Composer installs it first.
+	// After that, all other package installs trigger this event.
+	// Here we do a lot of the "heavy lifting"
 	public function onPostPackageInstall(PackageEvent $event)
 	{
 		// first, load all platform requirements from all operations