If pacman doesn't know the action is better to report a nice error rather than die with php error messages

Signed-off-by: emanuele <emanuele45@gmail.com>

Sources/Packages.php

@@ -338,7 +338,13 @@ function PackageInstallTest()
 		elseif ($action['type'] == 'redirect')
 			continue;
 		elseif ($action['type'] == 'error')
+		{
 			$context['has_failure'] = true;
+			if (isset($action['error_msg']) && isset($action['error_var']))
+				$context['failure_details'] = sprintf($txt['package_will_fail_' . $action['error_msg']], $action['error_var']);
+			elseif (isset($action['error_msg']))
+				$context['failure_details'] = isset($txt['package_will_fail_' . $action['error_msg']]) ? $txt['package_will_fail_' . $action['error_msg']] : $action['error_msg'];
+		}
 		elseif ($action['type'] == 'modification')
 		{
 			if (!file_exists($boarddir . '/Packages/temp/' . $context['base_path'] . $action['filename']))

Sources/Subs-Package.php

@@ -1179,142 +1179,152 @@ function parsePackageInfo(&$packageXML, $testing_only = true, $method = 'install
 				'type' => 'error',
 			);
 		}
-
-		$this_action = &$return[];
-		$this_action = array(
-			'type' => $actionType,
-			'filename' => $action->fetch('@name'),
-			'description' => $action->fetch('.')
-		);
-
-		// If there is a destination, make sure it makes sense.
-		if (substr($actionType, 0, 6) != 'remove')
-		{
-			$this_action['unparsed_destination'] = $action->fetch('@destination');
-			$this_action['destination'] = parse_path($action->fetch('@destination')) . '/' . basename($this_action['filename']);
-		}
-		else
+		elseif (in_array($actionType, array('require-file', 'remove-file', 'require-dir', 'remove-dir', 'move-file', 'move-dir', 'create-file', 'create-dir')))
 		{
-			$this_action['unparsed_filename'] = $this_action['filename'];
-			$this_action['filename'] = parse_path($this_action['filename']);
-		}
-
-		// If we're moving or requiring (copying) a file.
-		if (substr($actionType, 0, 4) == 'move' || substr($actionType, 0, 7) == 'require')
-		{
-			if ($action->exists('@from'))
-				$this_action['source'] = parse_path($action->fetch('@from'));
-			else
-				$this_action['source'] = $temp_path . $this_action['filename'];
-		}
+			$this_action = &$return[];
+			$this_action = array(
+				'type' => $actionType,
+				'filename' => $action->fetch('@name'),
+				'description' => $action->fetch('.')
+			);
 
-		// Check if these things can be done. (chmod's etc.)
-		if ($actionType == 'create-dir')
-		{
-			if (!mktree($this_action['destination'], false))
+			// If there is a destination, make sure it makes sense.
+			if (substr($actionType, 0, 6) != 'remove')
 			{
-				$temp = $this_action['destination'];
-				while (!file_exists($temp) && strlen($temp) > 1)
-					$temp = dirname($temp);
-
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $temp
-				);
+				$this_action['unparsed_destination'] = $action->fetch('@destination');
+				$this_action['destination'] = parse_path($action->fetch('@destination')) . '/' . basename($this_action['filename']);
 			}
-		}
-		elseif ($actionType == 'create-file')
-		{
-			if (!mktree(dirname($this_action['destination']), false))
+			else
 			{
-				$temp = dirname($this_action['destination']);
-				while (!file_exists($temp) && strlen($temp) > 1)
-					$temp = dirname($temp);
+				$this_action['unparsed_filename'] = $this_action['filename'];
+				$this_action['filename'] = parse_path($this_action['filename']);
+			}
 
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $temp
-				);
+			// If we're moving or requiring (copying) a file.
+			if (substr($actionType, 0, 4) == 'move' || substr($actionType, 0, 7) == 'require')
+			{
+				if ($action->exists('@from'))
+					$this_action['source'] = parse_path($action->fetch('@from'));
+				else
+					$this_action['source'] = $temp_path . $this_action['filename'];
 			}
 
-			if (!is_writable($this_action['destination']) && (file_exists($this_action['destination']) || !is_writable(dirname($this_action['destination']))))
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $this_action['destination']
-				);
-		}
-		elseif ($actionType == 'require-dir')
-		{
-			if (!mktree($this_action['destination'], false))
+			// Check if these things can be done. (chmod's etc.)
+			if ($actionType == 'create-dir')
 			{
-				$temp = $this_action['destination'];
-				while (!file_exists($temp) && strlen($temp) > 1)
-					$temp = dirname($temp);
+				if (!mktree($this_action['destination'], false))
+				{
+					$temp = $this_action['destination'];
+					while (!file_exists($temp) && strlen($temp) > 1)
+						$temp = dirname($temp);
 
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $temp
-				);
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $temp
+					);
+				}
 			}
-		}
-		elseif ($actionType == 'require-file')
-		{
-			if ($action->exists('@theme'))
-				$this_action['theme_action'] = $action->fetch('@theme');
-
-			if (!mktree(dirname($this_action['destination']), false))
+			elseif ($actionType == 'create-file')
 			{
-				$temp = dirname($this_action['destination']);
-				while (!file_exists($temp) && strlen($temp) > 1)
-					$temp = dirname($temp);
+				if (!mktree(dirname($this_action['destination']), false))
+				{
+					$temp = dirname($this_action['destination']);
+					while (!file_exists($temp) && strlen($temp) > 1)
+						$temp = dirname($temp);
 
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $temp
-				);
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $temp
+					);
+				}
+
+				if (!is_writable($this_action['destination']) && (file_exists($this_action['destination']) || !is_writable(dirname($this_action['destination']))))
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $this_action['destination']
+					);
 			}
+			elseif ($actionType == 'require-dir')
+			{
+				if (!mktree($this_action['destination'], false))
+				{
+					$temp = $this_action['destination'];
+					while (!file_exists($temp) && strlen($temp) > 1)
+						$temp = dirname($temp);
 
-			if (!is_writable($this_action['destination']) && (file_exists($this_action['destination']) || !is_writable(dirname($this_action['destination']))))
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $this_action['destination']
-				);
-		}
-		elseif ($actionType == 'move-dir' || $actionType == 'move-file')
-		{
-			if (!mktree(dirname($this_action['destination']), false))
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $temp
+					);
+				}
+			}
+			elseif ($actionType == 'require-file')
 			{
-				$temp = dirname($this_action['destination']);
-				while (!file_exists($temp) && strlen($temp) > 1)
-					$temp = dirname($temp);
+				if ($action->exists('@theme'))
+					$this_action['theme_action'] = $action->fetch('@theme');
 
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $temp
-				);
+				if (!mktree(dirname($this_action['destination']), false))
+				{
+					$temp = dirname($this_action['destination']);
+					while (!file_exists($temp) && strlen($temp) > 1)
+						$temp = dirname($temp);
+
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $temp
+					);
+				}
+
+				if (!is_writable($this_action['destination']) && (file_exists($this_action['destination']) || !is_writable(dirname($this_action['destination']))))
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $this_action['destination']
+					);
 			}
+			elseif ($actionType == 'move-dir' || $actionType == 'move-file')
+			{
+				if (!mktree(dirname($this_action['destination']), false))
+				{
+					$temp = dirname($this_action['destination']);
+					while (!file_exists($temp) && strlen($temp) > 1)
+						$temp = dirname($temp);
 
-			if (!is_writable($this_action['destination']) && (file_exists($this_action['destination']) || !is_writable(dirname($this_action['destination']))))
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $this_action['destination']
-				);
-		}
-		elseif ($actionType == 'remove-dir')
-		{
-			if (!is_writable($this_action['filename']) && file_exists($this_action['filename']))
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $this_action['filename']
-				);
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $temp
+					);
+				}
+
+				if (!is_writable($this_action['destination']) && (file_exists($this_action['destination']) || !is_writable(dirname($this_action['destination']))))
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $this_action['destination']
+					);
+			}
+			elseif ($actionType == 'remove-dir')
+			{
+				if (!is_writable($this_action['filename']) && file_exists($this_action['filename']))
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $this_action['filename']
+					);
+			}
+			elseif ($actionType == 'remove-file')
+			{
+				if (!is_writable($this_action['filename']) && file_exists($this_action['filename']))
+					$return[] = array(
+						'type' => 'chmod',
+						'filename' => $this_action['filename']
+					);
+			}
 		}
-		elseif ($actionType == 'remove-file')
+		else
 		{
-			if (!is_writable($this_action['filename']) && file_exists($this_action['filename']))
-				$return[] = array(
-					'type' => 'chmod',
-					'filename' => $this_action['filename']
-				);
+			$return[] = array(
+				'type' => 'error',
+				'error_msg' => 'unknown_action',
+				'error_var' => $actionType
+			);
 		}
 	}
 

Themes/default/Packages.template.php

@@ -42,7 +42,8 @@ function template_view_package()
 		echo '
 		<div class="errorbox">
 			', $txt['package_will_fail_title'], '<br />
-			', $txt['package_will_fail_warning'], '
+			', $txt['package_will_fail_warning'], 
+			!empty($context['failure_details']) ? '<br /><br /><strong>' . $context['failure_details'] . '</strong>' : '', '
 		</div>';
 	}
 

Themes/default/languages/Packages.english.php

@@ -110,6 +110,7 @@ $txt['package_will_fail_title'] = 'Error in Package Installation';
 $txt['package_will_fail_warning'] = 'At least one error was encountered during a test installation of this package.
 	It is <strong>strongly</strong> recommended that you do not continue with installation unless you know what you are doing, and have made a backup very recently.<br />
 	This error may be caused by a conflict between the package you\'re trying to install and another package you have already installed, an error in the package, a package which requires another package that you don\'t have installed yet, or a package designed for another version of SMF.';
+$txt['package_will_fail_unknown_action'] = 'The package is trying to perform an unknown action: %1$s';
 // Don't use entities in the below string.
 $txt['package_will_fail_popup'] = 'Are you sure you wish to continue installing this modification, even though it will not install successfully?';
 $txt['package_will_fail_popup_uninstall'] = 'Are you sure you wish to continue uninstalling this modification, even though it will not uninstall successfully?';