Merge lp:~zorba-coders/zorba/download-modules into lp:zorba

[Chris Hillery]

Error checking for DownloadModules.cmake; try three times on failures.

[Chris Hillery (ceejatec)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/download-modules-2012-02-16T10-26-00.482Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job download-modules-2012-02-16T10-26-00.482Z is finished. The final status was:

All tests succeeded!

[Zorba Build Bot (zorba-buildbot)]

Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1. Got: 1 Approve.

[Sorin Marian Nasoi (sorin.marian.nasoi)]

I think the changes are not O.K. , please see below for a brief description of the tests I did.
I have not approved the changes yet because I would need another opinion on the 2 issues I have raised below and about the fix proposed for the bug I found.

I have checked the following options:
Check 1)
cmake -Dallmodules=ON -Doutdir="MY_LOCATION" -P DownloadModules.cmake
I then compiled and run all tests successfully for all external modules (I ran "ctest -R _module").

Check 2)
cmake -Dmodname=email -Dnotags=ON -Doutdir="MY_LOCATION" -P DownloadModules.cmake
CMake Error at DownloadModules.cmake:128 (message):
  Failed to download 'data-cleaning' after 3 attempts, giving up.

Check 3)
cmake -Dmodname=email -Doutdir="MY_LOCATION" -P DownloadModules.cmake
CMake Error at DownloadModules.cmake:128 (message):
  Failed to download 'data-cleaning' after 3 attempts, giving up.

For this issue I will try to propose a fix.

Issue 1)
Paul reported that he had an issue with the email module that I could not reproduce.
Anyway, Juan (who managed to reproduce the error) said that he found an work around by changing the ExternalModule.config to have email module as the last module to branch out.

IMO, I would propose to include Juan's proposed fix in this merge as well.

Issue 2)
What is the status of the following external modules:
- lp:zorba/exi-module
- lp:zorba/queue-module
- lp:zorba/stack-module
Should they be included in the ExternalModule.config already?

AFAIK at least the last 2 external modules(queue, stack) are finished.

review: Needs Fixing

[lp:~zorba-coders/zorba/download-modules]

10663. By Sorin Marian Nasoi <email address hidden> on 2012-02-20

Fixed for the case where only some modules are downloaded using 'modname' option.

10664. By Sorin Marian Nasoi <email address hidden> on 2012-02-20

Added the fix proposed by Juan for the issue reported by Paul: see thread on @coders "Downloading modules fails".

[Sorin Marian Nasoi (sorin.marian.nasoi)]

The only question I have now is this:

What is the status of the following external modules:
- lp:zorba/exi-module
- lp:zorba/queue-module
- lp:zorba/stack-module
Should they be included in the ExternalModule.config already?

AFAIK at least the last 2 external modules(queue, stack) are finished.

review: Needs Information

[Matthias Brantner (matthias-brantner)]

> The only question I have now is this:
>
> What is the status of the following external modules:
> - lp:zorba/exi-module
> - lp:zorba/queue-module
> - lp:zorba/stack-module
> Should they be included in the ExternalModule.config already?
>
> AFAIK at least the last 2 external modules(queue, stack) are finished.
Neither of them is finished, yet. Including them seems orthogonal to this merge proposal.

[Sorin Marian Nasoi (sorin.marian.nasoi)]

> > The only question I have now is this:
> >
> > What is the status of the following external modules:
> > - lp:zorba/exi-module
> > - lp:zorba/queue-module
> > - lp:zorba/stack-module
> > Should they be included in the ExternalModule.config already?
> >
> > AFAIK at least the last 2 external modules(queue, stack) are finished.
> Neither of them is finished, yet. Including them seems orthogonal to this
> merge proposal.
I agree.

I am marking the changes as approved but I would need someone else to test the fixes I made before marking the merge as "Approved".

review: Approve

[Chris Hillery (ceejatec)]

Thanks for the fixes. I'm committing one minor change that just fixes the indentation.

[lp:~zorba-coders/zorba/download-modules]

10665. By Chris Hillery on 2012-02-24

Indentation fix.

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/download-modules-2012-02-24T02-18-00.276Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job download-modules-2012-02-24T02-18-00.276Z is finished. The final status was:

All tests succeeded!

=== modified file 'modules/DownloadModules.cmake'
--- modules/DownloadModules.cmake	2011-10-11 22:34:31 +0000
+++ modules/DownloadModules.cmake	2012-02-24 02:16:18 +0000
@@ -75,39 +75,58 @@
     endif (allmodules)
 
     # Download
-    if (_getmod)
+    if ("${_getmod}" EQUAL 1)
       message ("Downloading module '${_modname}'...")
-      if (${_modvc} STREQUAL "svn")
-        if (NOT svn)
-          message (FATAL_ERROR
-            "Subversion client not found - required for ${_modname} module!")
-        endif (NOT svn)
-        # QQQ Ridiculous and slow hack, but Sourceforge has been
-        # incredibly unreliable lately so this is the best choice I've
-        # got to make the remote queue semi-stable
-        foreach (s 1 2)
+      # We try three times, to account for network weirdnesses
+      foreach (i 1 2 3)
+	# First delete the output directory, in case there's a partial
+	# download from a previous attempt
+	file (REMOVE_RECURSE "${outdir}/${_modname}")
+
+	set (_status)
+
+	if (${_modvc} STREQUAL "svn")
+          if (NOT svn)
+            message (FATAL_ERROR
+              "Subversion client not found - required for ${_modname} module!")
+          endif (NOT svn)
           execute_process (COMMAND "${svn}" checkout "${_modurl}" "${_modname}"
-            WORKING_DIRECTORY "${outdir}" TIMEOUT 60)
-        endforeach (s 1 2)
-
-      elseif (${_modvc} STREQUAL "bzr")
-        if (NOT bzr)
-          message (FATAL_ERROR
-            "Bazaar client not found - required for ${_modname} module!")
-        endif (NOT bzr)
-
-        set (_modtagargs)
-        if (_modtag AND NOT notags)
-          set (_modtagargs "-r" "${_modtag}")
-        endif (_modtag AND NOT notags)
-        execute_process (COMMAND "${bzr}" branch "${_modurl}" "${_modname}"
-          ${_modtagargs} WORKING_DIRECTORY "${outdir}" TIMEOUT 60)
-
-      else (${_modvc} STREQUAL "svn")
-        message (FATAL_ERROR "Unknown vc-type '${_modvc}' for module "
-          "'${_modname}' in modules/ExternalModules.conf!")
-
-      endif (${_modvc} STREQUAL "svn")
-    endif (_getmod)
+            WORKING_DIRECTORY "${outdir}" TIMEOUT 60 RESULT_VARIABLE _status)
+
+	elseif (${_modvc} STREQUAL "bzr")
+          if (NOT bzr)
+            message (FATAL_ERROR
+              "Bazaar client not found - required for ${_modname} module!")
+          endif (NOT bzr)
+
+          set (_modtagargs)
+          if (_modtag AND NOT notags)
+            set (_modtagargs "-r" "${_modtag}")
+          endif (_modtag AND NOT notags)
+          execute_process (COMMAND "${bzr}" branch "${_modurl}" "${_modname}"
+            ${_modtagargs} WORKING_DIRECTORY "${outdir}" TIMEOUT 60
+	    RESULT_VARIABLE _status)
+
+	else (${_modvc} STREQUAL "svn")
+          message (FATAL_ERROR "Unknown vc-type '${_modvc}' for module "
+            "'${_modname}' in modules/ExternalModules.conf!")
+
+	endif (${_modvc} STREQUAL "svn")
+
+	if ("${_status}" EQUAL 0)
+	  # Success
+	  break ()
+	else ("${_status}" EQUAL 0)
+	  message (WARNING "Attempt ${i}: Failed to download '${_modname}' (${_status})")
+	endif ("${_status}" EQUAL 0)
+
+      endforeach (i)
+
+      # Ensure we successfully downloaded something good
+      if (NOT EXISTS "${outdir}/${_modname}/CMakeLists.txt")
+	message (FATAL_ERROR "Failed to download '${_modname}' after 3 attempts, giving up.")
+      endif (NOT EXISTS "${outdir}/${_modname}/CMakeLists.txt")
+
+    endif ("${_getmod}" EQUAL 1)
   endif (modline)
 endforeach (modline)

=== modified file 'modules/ExternalModules.conf'
--- modules/ExternalModules.conf	2012-02-23 21:57:52 +0000
+++ modules/ExternalModules.conf	2012-02-24 02:16:18 +0000
@@ -29,7 +29,6 @@
 data-cleaning   bzr  lp:zorba/data-cleaning-module       1.0
 data-converters bzr  lp:zorba/data-converters-module     
 data-formatting bzr  lp:zorba/data-formatting-module     zorba-2.1
-email           bzr  lp:zorba/email-module               zorba-2.1
 excel           bzr  lp:zorba/excel-module               zorba-2.1
 geo             bzr  lp:zorba/geo-module                 zorba-2.1
 http-client     bzr  lp:zorba/http-client-module         
@@ -40,3 +39,4 @@
 security        bzr  lp:zorba/security-module            zorba-2.1
 system          bzr  lp:zorba/system-module              zorba-2.1
 xqxq            bzr  lp:zorba/xqxq-module                zorba-2.1
+email           bzr  lp:zorba/email-module               zorba-2.1