Switch to using Unix chmod values for file permissions instead of values from Qt's QFile::Permission enum

The use of QFile::Permission flag values is a legacy from the old Qt-based updater.
Unix chmod values are used by the native system calls so it makes more sense
to use them instead.

 * Update the test examples and the test update script.
 * Update the package building tool to populate the permissions field with octal chmod values.

src/FileUtils.cpp

@@ -72,13 +72,12 @@ bool FileUtils::fileExists(const char* path) throw (IOException)
 #endif
 }
 
-void FileUtils::setQtPermissions(const char* path, int qtPermissions) throw (IOException)
+void FileUtils::chmod(const char* path, int mode) throw (IOException)
 {
 #ifdef PLATFORM_UNIX
-	int mode = toUnixPermissions(qtPermissions);
+	if (::chmod(path,mode) != 0)
-	if (chmod(path,mode) != 0)
 	{
-		throw IOException("Failed to set permissions on " + std::string(path) + " to " + intToStr(qtPermissions));
+		throw IOException("Failed to set permissions on " + std::string(path) + " to " + intToStr(mode));
 	}
 #else
 	// TODO - Not implemented under Windows - all files
@@ -361,32 +360,6 @@ std::string FileUtils::canonicalPath(const char* path)
 #endif
 }
 
-template <class InFlags, class OutFlags>
-void addFlag(InFlags inFlags, int testBit, OutFlags& outFlags, int setBit)
-{
-	if (inFlags & testBit)
-	{
-		outFlags |= setBit;
-	}
-}
-
-#ifdef PLATFORM_UNIX
-int FileUtils::toUnixPermissions(int qtPermissions)
-{
-	mode_t result = 0;
-	addFlag(qtPermissions,ReadUser,result,S_IRUSR);
-	addFlag(qtPermissions,WriteUser,result,S_IWUSR);
-	addFlag(qtPermissions,ExecUser,result,S_IXUSR);
-	addFlag(qtPermissions,ReadGroup,result,S_IRGRP);
-	addFlag(qtPermissions,WriteGroup,result,S_IWGRP);
-	addFlag(qtPermissions,ExecGroup,result,S_IXGRP);
-	addFlag(qtPermissions,ReadOther,result,S_IROTH);
-	addFlag(qtPermissions,WriteOther,result,S_IWOTH);
-	addFlag(qtPermissions,ExecOther,result,S_IXOTH);
-	return result;
-}
-#endif
-
 std::string FileUtils::toUnixPathSeparators(const std::string& str)
 {
 	std::string result = str;

src/FileUtils.h

@@ -34,23 +34,6 @@ class FileUtils
 				int m_errno;
 		};
 
-		/** Reproduction of Qt's QFile::Permission enum. */
-		enum QtFilePermission
-		{
-			ReadOwner  = 0x4000,
-			WriteOwner = 0x2000,
-			ExecOwner  = 0x1000,
-			ReadUser   = 0x0400,
-			WriteUser  = 0x0200,
-			ExecUser   = 0x0100,
-			ReadGroup  = 0x0040,
-			WriteGroup = 0x0020,
-			ExecGroup  = 0x0010,
-			ReadOther  = 0x0004,
-			WriteOther = 0x0002,
-			ExecOther  = 0x0001
-		};
-
 		/** Remove a file.  Throws an exception if the file
 		 * could not be removed.
 		 *
@@ -62,10 +45,10 @@ class FileUtils
 		 */
 		static void removeFile(const char* src) throw (IOException);
 
-		/** Set the permissions of a file using a combination of flags
+		/** Set the permissions of a file.  @p permissions uses the standard
-		  * from the QtFilePermission enum.
+		  * Unix mode_t values.
 		  */
-		static void setQtPermissions(const char* path, int permissions) throw (IOException);
+		static void chmod(const char* path, int permissions) throw (IOException);
 		static bool fileExists(const char* path) throw (IOException);
 		static void moveFile(const char* src, const char* dest) throw (IOException);
 		static void mkdir(const char* dir) throw (IOException);
@@ -116,8 +99,5 @@ class FileUtils
 		  * @p basePath should be absolute.
 		  */
 		static std::string makeAbsolute(const char* path, const char* basePath);
-
-	private:
-		static int toUnixPermissions(int qtPermissions);
 };
 

src/UpdateInstaller.cpp

@@ -235,7 +235,7 @@ void UpdateInstaller::installFile(const UpdateScriptFile& file)
 		FileUtils::extractFromZip(packageFile.c_str(),file.path.c_str(),destPath.c_str());
 
 		// set the permissions on the newly extracted file
-		FileUtils::setQtPermissions(destPath.c_str(),file.permissions);
+		FileUtils::chmod(destPath.c_str(),file.permissions);
 	}
 	else
 	{

src/UpdateScript.cpp

@@ -109,7 +109,10 @@ UpdateScriptFile UpdateScript::parseFile(const TiXmlElement* element)
 	UpdateScriptFile file;
 	file.path = elementText(element->FirstChildElement("name"));
 	file.package = elementText(element->FirstChildElement("package"));
-	file.permissions = atoi(elementText(element->FirstChildElement("permissions")).c_str());
+
+	std::string modeString = elementText(element->FirstChildElement("permissions"));
+	sscanf(modeString.c_str(),"%i",&file.permissions);
+
 	file.linkTarget = elementText(element->FirstChildElement("target"));
 	file.isMainBinary = strToBool(elementText(element->FirstChildElement("is-main-binary")));
 	return file;

src/UpdateScript.h

@@ -40,8 +40,13 @@ class UpdateScriptFile
 
 		std::string path;
 		std::string package;
-		int permissions;
 		std::string linkTarget;
+
+		/** The permissions for this file, specified
+		  * using the standard Unix mode_t values.
+		  */
+		int permissions;
+
 		bool isMainBinary;
 
 		bool operator==(const UpdateScriptFile& other) const

src/tests/TestUpdateScript.cpp

@@ -4,6 +4,7 @@
 #include "UpdateScript.h"
 
 #include <iostream>
+#include <algorithm>
 
 void TestUpdateScript::testV2Script()
 {
@@ -19,10 +20,31 @@ void TestUpdateScript::testV2Script()
 	TEST_COMPARE(newFormat.filesToUninstall(),oldFormat.filesToUninstall());
 }
 
+void TestUpdateScript::testPermissions()
+{
+	UpdateScript script;
+	script.parse("file_list.xml");
+
+	for (std::vector<UpdateScriptFile>::const_iterator iter = script.filesToInstall().begin();
+	     iter != script.filesToInstall().end();
+	     iter++)
+	{
+		if (iter->isMainBinary)
+		{
+			TEST_COMPARE(iter->permissions,0755);
+		}
+		if (!iter->linkTarget.empty())
+		{
+			TEST_COMPARE(iter->permissions,0);
+		}
+	}
+}
+
 int main(int,char**)
 {
 	TestList<TestUpdateScript> tests;
 	tests.addTest(&TestUpdateScript::testV2Script);
+	tests.addTest(&TestUpdateScript::testPermissions);
 	return TestUtils::runTest(tests);
 }
 

src/tests/TestUpdateScript.h

@@ -4,5 +4,6 @@ class TestUpdateScript
 {
 	public:
 		void testV2Script();
+		void testPermissions();
 };
 

src/tests/file_list.xml

@@ -20,7 +20,7 @@
    <name>$APP_FILENAME</name>
    <hash>$UPDATED_APP_HASH</hash>
    <size>$UPDATED_APP_SIZE</size>
-   <permissions>30549</permissions>
+   <permissions>0755</permissions>
    <package>app-pkg</package>
    <is-main-binary>true</is-main-binary>
   </file>

src/tests/v2_file_list.xml

@@ -36,7 +36,7 @@
    <name>$APP_FILENAME</name>
    <hash>$UPDATED_APP_HASH</hash>
    <size>$UPDATED_APP_SIZE</size>
-   <permissions>30549</permissions>
+   <permissions>0755</permissions>
    <package>app-pkg</package>
    <is-main-binary>true</is-main-binary>
   </file>

tools/create-packages.rb

@@ -43,8 +43,7 @@ end
 class UpdateScriptFile
 	# path - The path of the file relative to the installation directory
 	# hash - The SHA-1 hash of the file
-	# permissions - The permissions of the file expressed using
+	# permissions - The permissions of the file (See File::stat)
-	#               flags from the QFile::Permission enum in Qt
 	# size - The size of the file in bytes
 	# package - The name of the package containing this file
 	attr_reader :path,:hash,:permissions,:size,:package,:target,:is_main_binary
@@ -127,7 +126,7 @@ class UpdateScriptGenerator
 				file.target = File.readlink(path)
 			else
 				file.hash = file_sha1(path)
-				file.permissions = get_file_permissions(path)
+				file.permissions = File.stat(path).mode
 				file.size = File.size(path)
 				file.package = @config.package_for_file(file.path)
 
@@ -215,7 +214,7 @@ class UpdateScriptGenerator
 				attributes["target"] = file.target
 			else
 				attributes["size"] = file.size.to_s
-				attributes["permissions"] = file.permissions.to_s
+				attributes["permissions"] = file_mode_string(file.permissions)
 				attributes["hash"] = file.hash
 				attributes["package"] = file.package
 			end
@@ -229,53 +228,9 @@ class UpdateScriptGenerator
 		return install_elem
 	end
 
-	# Unix permission flags
+	def file_mode_string(mode)
-	# from <sys/stat.h>
+		mode_octal = (mode & 0777).to_s(8)
-	S_IRUSR = 0400
+		return "0#{mode_octal}"
-	S_IWUSR = 0200
-	S_IXUSR = 0100
-	S_IRGRP = (S_IRUSR >> 3)
-	S_IWGRP = (S_IWUSR >> 3)
-	S_IXGRP = (S_IXUSR >> 3)
-	S_IROTH = (S_IRGRP >> 3)
-	S_IWOTH = (S_IWGRP >> 3)
-	S_IXOTH = (S_IXGRP >> 3)
-
-	# Qt permission flags
-	# (taken from QFile::Permission)
-	QT_READ_OWNER  = 0x4000
-	QT_WRITE_OWNER = 0x2000
-	QT_EXEC_OWNER  = 0x1000
-	QT_READ_USER   = 0x0400
-	QT_WRITE_USER  = 0x0200
-	QT_EXEC_USER   = 0x0100
-	QT_READ_GROUP  = 0x0040
-	QT_WRITE_GROUP = 0x0020
-	QT_EXEC_GROUP  = 0x0010
-	QT_READ_OTHER  = 0x0004
-	QT_WRITE_OTHER = 0x0002
-	QT_EXEC_OTHER  = 0x0001
-
-	def get_file_permissions(path)
-		unix_to_qt = {
-			S_IRUSR => QT_READ_USER  | QT_READ_OWNER,
-			S_IWUSR => QT_WRITE_USER | QT_WRITE_OWNER,
-			S_IXUSR => QT_EXEC_USER  | QT_EXEC_OWNER,
-			S_IRGRP => QT_READ_GROUP,
-			S_IWGRP => QT_WRITE_GROUP,
-			S_IXGRP => QT_EXEC_GROUP,
-			S_IROTH => QT_READ_OTHER,
-			S_IWOTH => QT_WRITE_OTHER,
-			S_IXOTH => QT_EXEC_OTHER
-		}
-
-		qt_permissions = 0
-		unix_permissions = File.stat(path).mode
-		unix_to_qt.each do |unix_flag,qt_flags|
-			qt_permissions |= qt_flags if ((unix_permissions & unix_flag) != 0)
-		end
-
-		return qt_permissions.to_i
 	end
 end