From 3cc75307e9a19caea56348cbeccad67f3ec74b01 Mon Sep 17 00:00:00 2001 From: "kjellander@chromium.org" Date: Wed, 19 Dec 2012 16:29:03 +0000 Subject: [PATCH] Fix toplevel_root parsing in trychange.py In https://codereview.chromium.org/11574007/ I attempted to make it possible to use the TRYSERVER_ROOT setting in codereview.settings. It seems like some checkouts ran into errors with it not finding the codereview.settings file when it was parsed. This is probably due to the fact that the following call chain reads the self.toplevel_root variable: _GclStyleSettings -> GetCodeReviewSetting -> ReadRootFile By moving _GclStyleSettings up before self.toplevel_root is set probably causes this. This CL attempts to correct this. BUG=none, but try job upload fails for some users. TEST=submitting try job to locally running try server. Review URL: https://chromiumcodereview.appspot.com/11571052 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@173928 0039d316-1c4b-4281-b951-d872f2087c98 --- trychange.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/trychange.py b/trychange.py index 7d2ccda0b..89920e5bb 100755 --- a/trychange.py +++ b/trychange.py @@ -149,8 +149,12 @@ class SCM(object): return self.codereview_settings.get(key, '') def _GclStyleSettings(self): - """Set default settings based on the gcl-style settings from the - repository.""" + """Set default settings based on the gcl-style settings from the repository. + + The settings in the self.options object will only be set if no previous + value exists (i.e. command line flags to the try command will override the + settings in codereview.settings). + """ settings = { 'port': self.GetCodeReviewSetting('TRYSERVER_HTTP_PORT'), 'host': self.GetCodeReviewSetting('TRYSERVER_HTTP_HOST'), @@ -162,14 +166,14 @@ class SCM(object): logging.info('\n'.join(['%s: %s' % (k, v) for (k, v) in settings.iteritems() if v])) for (k, v) in settings.iteritems(): + # Avoid overwriting options already set using command line flags. if v and getattr(self.options, k) is None: setattr(self.options, k, v) def AutomagicalSettings(self): """Determines settings based on supported code review and checkout tools. """ - self._GclStyleSettings() - # Try to find gclient or repo root. + # Try to find gclient or repo root first. if not self.options.no_search: self.toplevel_root = gclient_utils.FindGclientRoot(self.checkout_root) if self.toplevel_root: @@ -181,10 +185,17 @@ class SCM(object): logging.info('Found .repo dir at %s' % os.path.dirname(self.toplevel_root)) + # Parse TRYSERVER_* settings from codereview.settings before falling back + # on setting self.options.root manually further down. Otherwise + # TRYSERVER_ROOT would never be used in codereview.settings. + self._GclStyleSettings() + if self.toplevel_root and not self.options.root: assert os.path.abspath(self.toplevel_root) == self.toplevel_root self.options.root = gclient_utils.PathDifference(self.toplevel_root, self.checkout_root) + else: + self._GclStyleSettings() def ReadRootFile(self, filename): cur = self.checkout_root