View Reviews


Submitter:	Ima Mechanique	Reviewers
Branch:		Groups:	viewer
Bugs:	OPEN-76	People:	alain_linden
Change Number:	None	Repository:	autobuild

Description:

When running a command such as:

autobuild configure --config-file ab-test.xml -c ReleaseOS

the configuration is only partially completed correctly. When the installables are processed, it does not use the correct configuration file. This is due to cmake calling autobuild install to do the actual install, but not passing the --config-file option. Autobuild falls back to the default autobuild.xml, because the correct configuration file is not saved at any point, which is not the intention and can cause configuration to be done with incorrect libraries. If autobuild.xml is missing or corrupt, an error occurs (which is how I discovered this), stopping the configure command.

This fix saves the current value of the configuration file into the AUTOBUILD_CONFIG_FILE environmental variable so that it becomes the default value should any subsequent command need it. I believe that something like this was the original intention when adding this environment variable as part of the default check, but was overlooked.

Testing Done:

Ran 'autobuild configure --config-file ab-test.xml -c ReleaseOS' successfully without any errors and was able to build the expected viewer afterwards.

Changes between revision 1 and 2

Jump to revision:	1 2
Changes between r1 and:	1 2

Files Changed:

autobuild/configfile.py:

         """
         path = None
         def __init__(self, path):
             self.version = AUTOBUILD_CONFIG_VERSION
             self.type = AUTOBUILD_CONFIG_TYPE
             self.installables = {}
             self.package_description = None
             self.__load(path)
             os.environ['AUTOBUILD_CONFIG_FILE'] = os.path.basename(self.path)
         def absolute_path(self, path):
             """
             Returns an absolute path derived from the input path rooted at the configuration file's
             directory when the input is a relative path.
             """
             if os.path.isabs(path):
                 return path
             else:
                 return os.path.abspath(os.path.join(os.path.dirname(self.path), path))
                 if saved_data['version'] == self.version:
                     if (not saved_data.has_key('type')) or (saved_data['type'] != 'autobuild'):
                         raise AutobuildError(self.path + ' not an autobuild configuration file')
                     package_description = saved_data.pop('package_description', None)
                     if package_description is not None:
                         self.package_description = PackageDescription(package_description)
                     installables = saved_data.pop('installables', {})
                     for (name, package) in installables.iteritems():
                         self.installables[name] = PackageDescription(package)
                     self.update(saved_data)
-                    os.environ['AUTOBUILD_CONFIG_FILE'] = os.path.basename(self.path)
                     logger.debug("Configuration file '%s'" % self.path)
                 else:
                     if saved_data['version'] in update.updaters:
                         update.updaters[saved_data['version']](saved_data, self)
                     else:
                         raise ConfigurationError("cannot update version %s file %s" %
                                                  (saved_data.version, self.path))
             elif not os.path.exists(self.path):
                 logger.warn("Configuration file '%s' not found" % self.path)
             else:

autobuild/configfile.py:

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

This change has been marked as submitted.

OPEN-76 Fix autobuild so that --config-file option is honoured by subsequent (possibly recursive) commands

Changes between revision 1 and 2