Import a required module. Address Issue #49

[vladimir-v-diaz]

Address Isssue #49.

softwareupdater.py raises an exception when it tries to update its local time via NTP during an update. The message logged is [do_rsync] Unable to update ntp time. Not updating, however, updating the time via NTP was found to be functioning correctly. After some tracing and troubleshooting, the root cause of the NTP exception was found to be an import issue: global name 'time_updatetime' is not defined. It appears softwareupdater.py has not been importing a required time.r2py module. It is unclear how this module has been working correctly until now. (perhaps repy2 changed the way imports are handled?) In addition to issues with exception handling, the softwareupdater.py unit tests produce different results after every invocation (temp files are not properly removed, processes killed, etc?)

[aaaaalbert]

dy_import_module_symbols should be used with extreme caution (i.e. usually be avoided), just like Python's from x import *. The proper patch should be this:

@@ -52,8 +52,9 @@ import portable_popen
 import servicelogger


-dy_import_module_symbols("signeddata.r2py")
-dy_import_module_symbols("sha.r2py")
+signeddata = dy_import_module("signeddata.r2py")
+sha = dy_import_module("sha.r2py")
+time = dy_import_module("time.r2py")

 # Armon: The port that should be used to update our time using NTP
 TIME_PORT = 51234
@@ -151,7 +152,7 @@ def get_file_hash(filename):
   filedata = fileobj.read()
   fileobj.close()

-  return sha_hexhash(filedata)
+  return sha.sha_hexhash(filedata)



@@ -295,7 +296,7 @@ def do_rsync(serverpath, destdir, tempdir):
   newmetafileobject.close()

   # Incorrectly signed, we don't update...
-  if not signeddata_issignedcorrectly(newmetafiledata, softwareupdatepublickey):
+  if not signeddata.signeddata_issignedcorrectly(newmetafiledata, softwareupdatepublickey):
     safe_log("[do_rsync] New metainfo not signed correctly. Not updating.")
     return []

@@ -311,10 +312,10 @@ def do_rsync(serverpath, destdir, tempdir):
   else:
     try:
       # Armon: Update our time via NTP, before we check the meta info
-      time_updatetime(TIME_PORT)
+      time.time_updatetime(TIME_PORT)
     except Exception:
       try:
-        time_updatetime(TIME_PORT_2)
+        time.time_updatetime(TIME_PORT_2)
       except Exception:
         # Steven: Sometimes we can't successfully update our time, so this is
         # better than generating a traceback.
@@ -322,7 +323,7 @@ def do_rsync(serverpath, destdir, tempdir):
         return []

     # they're both good.   Let's compare them...
-    shoulduse, reasons = signeddata_shouldtrust(oldmetafiledata,newmetafiledata,softwareupdatepublickey)
+    shoulduse, reasons = signeddata.signeddata_shouldtrust(oldmetafiledata,newmetafiledata,softwareupdatepublickey)

     if shoulduse == True:
       # great!   All is well...
@@ -350,13 +351,13 @@ def do_rsync(serverpath, destdir, tempdir):
         # we should distrust. - Brent
         reasons.remove('Public keys do not match')
         for comment in reasons:
-          if comment in signeddata_fatal_comments:
+          if comment in signeddata.signeddata_fatal_comments:
             # If there is a different fatal comment still there, still log it
             # and don't perform the update.
             safe_log("[do_rsync] Serious problem with signed metainfo: " + str(reasons))
             return []

-          if comment in signeddata_warning_comments:
+          if comment in signeddata.signeddata_warning_comments:
             # If there is a different warning comment still there, log the
             # warning.  We will take care of specific behavior shortly.
             safe_log("[do_rsync] " + str(comment))

[vladimir-v-diaz]

softwareupdater.py now avoids dy_import_module_symbols(). See vladimir-v-diaz@4185620.

I will correct dy_import_ module_symbols() calls as I come across them. For example, writemetainfo.py also uses them.

[asm582]

I tried to run unit tests but it seems to be hanging on windows system. Below is the trace:-

C:\Users\abhishek\tests\softwareupdater\RUNNABLE>python utf.py -a
Testing module: softwareupdaters
Running: ut_softwareupdaters_testupdaterlocal.py

[vladimir-v-diaz]

The unit tests also hang when I run them on Windows. I have to manually kill the Python process for the test results to complete/show, and was told this is a currently known issue:

"grep the Seattle sources for "close_fds" for a few mentions of a problem Windows has with closing file descriptors after subprocess.Popen().

See also SeattleTestbed/seash@37637ba and specifically line 17 of the patched file, SeattleTestbed/seash@37637ba#diff-95df8f5b319ba2c8d0967adf01b37d61R17

The gist seems to be that the subprocess doesn't start on Windows if you don't close stdin/stdout (yet it would be helpful to monitor them at times, which was the subject of the patch that commit 37637ba reverted.)"