Update Jenkins Build.py to check and upgrade svn version in working dir as necessary.

[sqlbyme]

Summary

When we upgrade Xcode versions on the Green Dragon bots we need to also upgrade the SVN version of the current working directory. This patch will allow build.py to do so automatically.

This patch also includes a bit of general cleanup and a typo fix.

[mehdi_amini]

zorg/jenkins/build.py

175

Can't we run svn upgrade unconditionally?
Why the version check?

907–908

Why this change?

967

I guess we don't care about exception handling?

[sqlbyme]

zorg/jenkins/build.py

175

If we run the svn upgrade command unconditionally it will return an error when run on a directory which does not need to be upgraded. When I worked on this with Chris M. internally we settled on this approach to ensure we only run the upgrade command on a working direct which actually requires it.

907–908

The context here is not great. This variable was originally assigned in a try block. If the try block bailed out for some reason the variable would never be assigned. It is used a few lines down in a logger function call. I figured it was better to have it assigned to an empty string just in case it somehow did not get assigned in the try block.

967

No, we want the subprocess to run and return what ever happens. We will let the calling function decide what to do with the error, if anything.

[mehdi_amini]

zorg/jenkins/build.py

175

OK but here you always run the upgrade when the svn install is 1.7.23 and higher, I don't see how it address the issue you're describing.

179

why not assert m here and get an easier control flow + indentation?

907–908

FYI: git diff -U99999 allows you to upload patch with context (or arc would do it as well).

I figured it was better to have it assigned to an empty string just in case it somehow did not get assigned in the try block.

Right, but is it related to this patch? Otherwise just commit this separately (along with the whitespace changes here and there if possible).

967

But subprocess.check_output can throw, and you won't restore the path.
Should be something like:

def run_collect_output(cmd, working_dir=None, stderr=None):
    """Run cmd, and return the output"""
    last = os.getcwd()
    try:
        if os.getenv("TESTING"):
            print 'TV: ' + ' '.join(cmd)
            return TEST_VALS[' '.join(cmd)]
        if working_dir:
            os.chdir(working_dir)
        out = subprocess.check_output(cmd, stderr=stderr)
    finally:
      os.chdir(last)

Using a contextmanager would even be cleaner but not sure if it is available everywhere on our installs? http://stackoverflow.com/questions/299446/how-do-i-change-directory-back-to-my-original-working-directory-with-python

[sqlbyme]

Based on the feedback I have received form Mehdi, I am going to abandon this revision and go refactor this so it makes more sense. Thanks for the feedback Mehdi!

zorg/jenkins/build.py

         self.device = None
         self._svn_url_cache = None
 
+        self.node_name = os.environ.get('NODE_NAME', 'Undefine_node_name')
+
         # Import all of the command line arguments into the config object
         self.__dict__.update(vars(args))
 

 conf = None  # type: Configuration
 
 
+def update_svn_checkouts():
+    """Check the svn version and upgrade if necessary."""
+    out = run_collect_output(["/usr/bin/xcrun", "svn", "--version"],
+                             working_dir=conf.srcdir())
+    m = re.search("version.(?P<version_string>[\d.]+)", out, re.MULTILINE)
+    if m:
+        version_str = m.group("version_string")
+        version_list = [int(x) for x in version_str.split(".")]
+        if version_list > [1, 7, 22]:
+            try:
+                next_section("SVN upgrade")
+                out = run_collect_output(["/usr/bin/xcrun", "svn", "upgrade"],
+                                         working_dir=conf.srcdir())
+            except subprocess.CalledProcessError as e:
+                msg = """Process return code: {}\n
+                      Error: on {} SVN upgrade failed.\n
+                      The error was: {}.\n"""
+                msg = msg.format(e.returncode, conf.node_name, out)
+                print msg
+    else:
+        assert False, "Unable to parse the svn version: " + out
+
+
 def cmake_builder(target):
     check_repo_state(conf.workspace)
+    update_svn_checkouts()
 
     env = []
     dyld_path = ""

 def clang_builder(target):
     """Build to set of commands to compile and test apple-clang"""
     check_repo_state(conf.workspace)
+    update_svn_checkouts()
+
     # get rid of old archives from prior builds
-
     run_ws(['sh', '-c', 'rm -rfv *gz'])
 
     if target == "all" or target == "build":

                                './Build',
                                './Root'])
             install_prefix = conf.installdir()
-            
             # Infer which CMake cache file to use. If ThinLTO we select a specific one.
             cmake_cachefile_thinlto = ''
             if conf.thinlto:

 
 def check_repo_state(path):
     """Check the SVN repo at the path has all the
-    nessessary repos checked out.  Check this by
+    necessary repos checked out.  Check this by
     looking for the README.txt in each repo. """
 
     if os.environ.get('TESTING', False):

         return
 
     logging.info("Detecting repos in {}".format(path))
-    for r in ['llvm', 'clang', 'clang-tools-extra', 'debuginfo-tests', \
+    for r in ['llvm', 'clang', 'clang-tools-extra', 'debuginfo-tests',
               'compiler-rt', 'libcxx', 'debuginfo-tests']:
         detected_path = derived_path('llvm', tree_path(tree='llvm', repo=r))
         readme = os.path.join(path, detected_path, readme_name(repo=r))

     footer()
 
 
-def derive_llvm(repos=['llvm', 'clang', 'libcxx', 'clang-tools-extra', \
+def derive_llvm(repos=['llvm', 'clang', 'libcxx', 'clang-tools-extra',
                        'compiler-rt', 'debuginfo-tests']):
     """Build a derived src tree for LLVM"""
     derive(tree='llvm', repos=repos)

     sys.stdout.flush()
 
     start_time = datetime.datetime.now()
+    result = ""
     if not os.environ.get('TESTING', False):
         try:
             os.chdir(working_dir)

              }
 
 
-def run_collect_output(cmd):
+def run_collect_output(cmd, working_dir=None, stderr=None):
     """Run cmd, and return the output"""
+    last = os.getcwd()
     if os.getenv("TESTING"):
         print 'TV: ' + ' '.join(cmd)
         return TEST_VALS[' '.join(cmd)]
+    if working_dir:
+        os.chdir(working_dir)
+    out = subprocess.check_output(cmd, stderr=stderr)
+    if working_dir:
+        os.chdir(last)
+    return out
 
-    return subprocess.check_output(cmd)
 
-
 def query_sys_tool(sdk_name, tool_name):
     """Get the path of system tool