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.
> + 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.
> + 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.
Cheers,
CR.
>
> Thanks,
> Nish
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
