On 01.06.2012 [08:04:31 -0300], Cleber Rosa wrote:
> On 05/31/2012 09:04 PM, Nishanth Aravamudan wrote:
> >On 30.05.2012 [19:55:20 -0300], Cleber Rosa wrote:
> >>On 05/30/2012 07:52 PM, Lucas Meneghel Rodrigues wrote:
> >>>On Wed, 2012-05-30 at 19:49 -0300, Cleber Rosa wrote:
> >>>>On 05/30/2012 06:48 PM, Lucas Meneghel Rodrigues wrote:
> >>>>>On Wed, 2012-05-30 at 14:41 -0700, Nishanth Aravamudan wrote:
> >>>>>>On 30.05.2012 [18:26:58 -0300], Lucas Meneghel Rodrigues wrote:
> >>>>>>>On Wed, 2012-05-30 at 14:23 -0700, Nishanth Aravamudan wrote:
> >>>>>>>>On 26.05.2012 [18:03:31 -0300], Lucas Meneghel Rodrigues wrote:
> >>>>>>>>>On Fri, 2012-05-25 at 16:31 -0700, Nishanth Aravamudan wrote:
> >>>>>>>>>>Sometimes there are firewalls between test machines and the greater
> >>>>>>>>>>Internet, so it seems unwise to depend upon external network access 
> >>>>>>>>>>in
> >>>>>>>>>>the boottool/grubby building code. But some sites won't have those
> >>>>>>>>>>restrictions. Add the ability to configure a local mirror for the 
> >>>>>>>>>>grubby
> >>>>>>>>>>tarball in the CLIENT section, but default to the external location.
> >>>>>>>>>The problem with this patch is that makes boottool dependent on 
> >>>>>>>>>autotest
> >>>>>>>>>libraries, when the script itself is sometimes used in a stand alone
> >>>>>>>>>fashion. Therefore, I can't accept this as is.
> >>>>>>>>Ah makes sense. I didn't realize boottool was used stand-alone, sorry.
> >>>>>>>>
> >>>>>>>>>Cleber, I believe we should try to locate and download boottool from 
> >>>>>>>>>the
> >>>>>>>>>copy present in the autotest tree, before trying to reach out to 
> >>>>>>>>>github.
> >>>>>>>>>What do you say?
> >>>>>>>>Ah I didn't even realize there was one in client/deps/grubby -- so,
> >>>>>>>>would we try and push it out with the rest of autotest? I'm not sure
> >>>>>>>>pulling will work, as the client/deps/grubby path isn't guaranteed 
> >>>>>>>>(nor
> >>>>>>>>is it setup to be, afaict) part of the web-exposed path.
> >>>>>>>This is what I'd like to do, find a way to ensure the grubby tarball
> >>>>>>>gets copied when the client is installed. This way we wouldn't ever 
> >>>>>>>have
> >>>>>>>to resort to an external copy.
> >>>>Well, did you guys miss the response I posted a couple of days ago?
> >>>>Copying it again:
> >>>>
> >>>>---
> >>>>
> >>>>On client mode that is definitely the best thing to do. But that would
> >>>>fail on server mode.
> >>>>
> >>>>I suggest that boottool looks for the grubby tarball on those locations:
> >>>>
> >>>>1) current directory (would solve server mode if we also send the
> >>>>tarball to the client)
> >>>>2) autotest source tree
> >>>>3) remote github uri
> >>>>
> >>>>How does that sound?
> >>>>
> >>>>---
> >>>>
> >>>>So, adding that to the rsync'd path list sounds like implementing #2. #1
> >>>>is still needed, and number #3 is a fallback that may be skipped.
> >>>>
> >>>>Are we all on the same page here?
> >>>Yes, we are, implementing 1) and 2) is exactly what I had in mind.
> >>>
> >>OK, great! I guess there was a bit of miscommunication my part. /me
> >>glad that we all share the same point of view.
> >So I started hacking at this, and it seems like
> >
> >diff --git a/client/tools/boottool b/client/tools/boottool
> >index c0b095c..924c7ae 100755
> >--- a/client/tools/boottool
> >+++ b/client/tools/boottool
> >@@ -1227,18 +1227,34 @@ class Grubby(object):
> >          Fetches and verifies the grubby source tarball
> >          '''
> >          tarball_name = os.path.basename(GRUBBY_TARBALL_URI)
> >-        tarball = os.path.join(topdir, tarball_name)
> >
> >          try:
> >-            urllib.urlretrieve(GRUBBY_TARBALL_URI, tarball)
> >+            # first look in the current directory
> >+            tarball = tarball_name
> 
> ^ I'm unsure if autoserv calls boottool with its current working
> directory set to the root path of boottool itself, or, if it calls
> it from $HOME, and executes /tmp/autoserv-xxxxx/boottool. If the
> later is true, AFAICT without testing it, this would not be
> sufficient, and we'd have to get bootool's __file__ base directory.

Ok, I will take a look on a live system.

> >+            f = open(tarball)
> >          except:
> >-            return False
> >-
> >-        tarball_md5 = md5.md5(open(tarball).read()).hexdigest()
> >+            try:
> >+                # then the autotest source directory
> >+                from autotest.client.shared import global_config
> >+                GLOBAL_CONFIG = global_config.global_config
> >+                tarball = os.path.join(
> >+                           GLOBAL_CONFIG.get_config_value('COMMON', 
> >'autotest_top_path'),
> >+                           tarball_name)
> >+                f = open(tarball)
> 
> ^ I'd replace or complement the open calls protected by try
> statements with something like os.path.exists + os.access, but
> that's purely a style thing.

Sure, I can do that.

> >+            except:
> >+                # then try to grab it from github
> >+                try:
> >+                    tarball = os.path.join(topdir, tarball_name)
> >+                    urllib.urlretrieve(GRUBBY_TARBALL_URI, tarball)
> >+                    f = open(tarball)
> >+                except:
> >+                    return None
> 
> ^ Same here.
> 
> >+
> >+        tarball_md5 = md5.md5(f.read()).hexdigest()
> >          if tarball_md5 != GRUBBY_TARBALL_MD5:
> >-            return False
> >+            return None
> >
> >-        return True
> >+        return tarball
> >
> >
> >      def grubby_build(self, topdir, tarball):
> >
> >
> >Should do what we want? Not tested, but wanted to see if it was sane.
> >Would need to change the caller to take the returned value as the path
> >to the tarball.
> 
> Yep, that looks pretty much like what we need. Whoever gets the
> change please test it (first).
> 
> And thanks for looking into this.

Not a problem.

Thanks,
Nish

-- 
Nishanth Aravamudan <n...@us.ibm.com>
IBM Linux Technology Center

_______________________________________________
Autotest mailing list
Autotest@test.kernel.org
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to