On Fri, Apr 18, 2008 at 01:54:51PM -0700, Dan Price wrote:

> The layering/implicit knowledge in client.py that something is
> going to be gzipped seems... sketchy IMO.

Yup.  My thought is to make the gunzip function check its input and
decompress if it finds gzip, spit out raw otherwise, possibly optionally.
It already tastes its input to make sure it's a valid gz stream, so that
wouldn't be too hard.  But this may not make this wad.

> Test cases needed for each of the failure modes at:
> 
> client.py: 686 (info: -l and -r may not be combined)
> client.py: 689 (nfo: must request remote info for specific packages)

Yep.  I'm not convinced all the exit codes are what they ought to be,
though.

> client.py: I personally really like a couple of lines of vertical
> whitespace between functions, esp. in python where there is no column
> zero closing brace to signify function end.  Oh well.

Hm.  My sense is the opposite.  :)

> More "get a list of things" sprawl.  Oh well.  We need to clean this up
> post-May.

Yeah.  I've some ideas for this, like I said.

> Can you post some sample output please?

Sure.  This is what's currently there.  The empty output from the first
example was what I was asking about, but I'm sure the messaging could be
improved for all the errors.  Here are the local infos:

    # In catalog, not installed (exit=0):
    $ pkg info SUNWzsh
    # In catalog, installed (exit=0):
    $ pkg info SUNWtcat
              Name: SUNWtcat
           Summary: Tomcat Servlet/JSP Container
             State: Installed
         Authority: opensolaris.org (preferred)
           Version: 5.5.25
     Build Release: 5.11
            Branch: 0.84
    Packaging Date: Wed Apr 16 22:03:47 2008
              Size: 23.7 MB
              FMRI: pkg:/[EMAIL PROTECTED],5.11-0.84:20080416T220347Z
    # Not in catalog (exit=1):
    $ pkg info SUNWnethack
    client.py: no matching packages found in catalog

Here are the remote infos.  Note that the SUNWtcat output is the same as
for the local case, because there's only one version to be had.  If there
were more than one, and I didn't have the latest installed, it would get
the information for the latest version from the server.  I believe that it
would say "Not installed" in that case, though we should probably figure
out how to make it say "Upgradable".

    # In catalog, not installed (exit=0):
    $ pkg info -r SUNWzsh
              Name: SUNWzsh
           Summary: Z shell (zsh)
             State: Not installed
         Authority: opensolaris.org (preferred)
           Version: 4.3.4
     Build Release: 5.11
            Branch: 0.84
    Packaging Date: Wed Apr 16 22:12:11 2008
              Size: 4.0 MB
              FMRI: pkg:/[EMAIL PROTECTED],5.11-0.84:20080416T221211Z
    # In catalog, installed (exit=0):
    $ pkg info -r SUNWtcat
              Name: SUNWtcat
           Summary: Tomcat Servlet/JSP Container
             State: Installed
         Authority: opensolaris.org (preferred)
           Version: 5.5.25
     Build Release: 5.11
            Branch: 0.84
    Packaging Date: Wed Apr 16 22:03:47 2008
              Size: 23.7 MB
              FMRI: pkg:/[EMAIL PROTECTED],5.11-0.84:20080416T220347Z
    # Not in catalog (exit=0):
    $ pkg info -r SUNWnethack
    pkg: no package matching 'SUNWnethack' could be found in current catalog
         suggest relaxing pattern, refreshing and/or examining catalogs

> Didn't totally understand the movement of gunzip stuff from generic to
> misc....

Stephen and I had talked about doing this a while back, and now that it's
no longer being used just by actions, it makes sense to make the move.

> client/image.py: Any deeper implications of this change?  Would
> we want to warn the customer that they're fetching manifests but
> they can't be stored locally?  For non-priv users, should we have
> a stash in /tmp or something?  I could see this banging on the
> server rather a lot...

Yeah, I dunno.  I'm pretty certain nothing will break because the manifest
can't be written.  Anything where you wouldn't have perms to install will
fail later now than it would have previously, but that's the only behavior
change there.  As for banging on the server a lot, yeah, I suppose.
Another wad?

> client/image.py: I know you didn't write this but "parse" is a really
> vague name and it took me some time to work out what this was doing.
> Would this code be clearer, shorter and faster as follows?
> 
>         mfst = self.get_manifest(fmri, filtered = True)
> 
>         for depend in mfst.gen_actions_by_type("depend"):
>                 required, min_fmri, max_fmri = depend.parse()
>                 if required == False:
>                         self.update_optional_dependency(min_fmri)

That's better, yeah.

> I couldn't work out why we needed a list comprehension.

It's a generator expression, actually, but I like yours better, anyway.

Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to