[date] Add ICU backend for timezone info behind a flag

tools/testrunner/local/commands.py

 # Copyright 2012 the V8 project authors. All rights reserved.
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 #       notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
 #       copyright notice, this list of conditions and the following
 #       disclaimer in the documentation and/or other materials provided
@@ skipping 32 matching lines @@
   prev_error_mode = SEM_INVALID_VALUE
   try:
     import ctypes
     prev_error_mode = \
         ctypes.windll.kernel32.SetErrorMode(mode)  #@UndefinedVariable
   except ImportError:
     pass
   return prev_error_mode
 
 
-def RunProcess(verbose, timeout, args, **rest):

[Michael Achenbach, 2017/04/07 13:02:10]

Why not keep **rest? Might be that it's used from somewhere.

[Dan Ehrenberg, 2017/04/07 14:06:09]

I didn't see any usages in the tree, but anyway, added it back in.

+def RunProcess(verbose, timeout, args, additionalEnv):

[Michael Achenbach, 2017/04/07 13:02:10]

nit: additional_env

[Dan Ehrenberg, 2017/04/07 14:06:09]

Done.

   if verbose: print "#", " ".join(args)
   popen_args = args
   prev_error_mode = SEM_INVALID_VALUE
   if utils.IsWindows():
     popen_args = subprocess.list2cmdline(args)
     # Try to change the error mode to avoid dialogs on fatal errors. Don't
     # touch any existing error mode flags by merging the existing error mode.
     # See http://blogs.msdn.com/oldnewthing/archive/2004/07/27/198410.aspx.
     error_mode = SEM_NOGPFAULTERRORBOX
     prev_error_mode = Win32SetErrorMode(error_mode)
     Win32SetErrorMode(error_mode | prev_error_mode)
 
   env = os.environ.copy()
+  env.update(additionalEnv)
   # GTest shard information is read by the V8 tests runner. Make sure it
   # doesn't leak into the execution of gtests we're wrapping. Those might
   # otherwise apply a second level of sharding and as a result skip tests.
   env.pop('GTEST_TOTAL_SHARDS', None)
   env.pop('GTEST_SHARD_INDEX', None)
 
   try:
     process = subprocess.Popen(
       args=popen_args,
       stdout=subprocess.PIPE,
       stderr=subprocess.PIPE,
-      env=env,
-      **rest
+      env=env
     )
   except Exception as e:
     sys.stderr.write("Error executing: %s\n" % popen_args)
     raise e
 
   if (utils.IsWindows() and prev_error_mode != SEM_INVALID_VALUE):
     Win32SetErrorMode(prev_error_mode)
 
   def kill_process(process, timeout_result):
     timeout_result[0] = True
@@ skipping 29 matching lines @@
 
   return output.Output(
       process.returncode,
       timeout_result[0],
       stdout.decode('utf-8', 'replace').encode('utf-8'),
       stderr.decode('utf-8', 'replace').encode('utf-8'),
       process.pid,
   )
 
 
-def Execute(args, verbose=False, timeout=None):
+def Execute(args, verbose=False, timeout=None, env={}):

[Michael Achenbach, 2017/04/07 13:02:10]

env=None
and then pass
env or {}

[Dan Ehrenberg, 2017/04/07 14:06:09]

Done.

[Michael Achenbach, 2017/04/11 08:38:36]

You didn't do the second part, hence the test errors in buildbot. The line in
the end should look like:

return RunProcess(verbose, timeout, args, env or {})

often folks also put this into an extra line like:
env = env or {}
but I'm usually happy with one-liners.

Note that in your first version, {} is evaluated at parse-time and all functions
would share the same dict object. In case it's mutated, it results in
hard-to-find errors.

[Dan Ehrenberg, 2017/04/11 11:31:51]

This was a result of an overconfident search of some of the test code for
callers without this arg... anyway, that did the trick. Thanks.

   args = [ c for c in args if c != "" ]
-  return RunProcess(verbose, timeout, args=args)
+  return RunProcess(verbose, timeout, args, env)