Shawn Walker wrote:
> > http://cr.opensolaris.org/~dduvall/pkg-pkgmog/
>
> src/man/pkgmogrify.1.txt:
> lines 118-155: nit: Are these in a specific order intentionally? If
> not, I think it'd be more pleasing to the eye and easier to
> reference if they were listed in asciiebtical order.
I had preserved the order that was there, which seems to have been an
attempt to group operations by what they did and what they operated on.
I went ahead and sorted them.
> lines 177-183: so users will have to take care to always use sep, etc.
> just in case multi-valued attribute values contain spaces?
I went ahead and put in the same quoting function as in the generic
action's __str__() method, so that values with spaces (or other quotes)
will be quoted properly. That will allow you to say something like
<transform set name=something -> emit set name=somethingelse value=%(value)>
without worrying about whether the original action's value needed quoting.
Of course, that means that if, like in some of my tests, you do something
like
<transform set name=something -> print The value is "%(value)">
then you'll get a double set of quotes.
The other problem with the first construct above is again in the list case,
where you'd get a MalformedActionError by trying to emit
set name=somethingelse value=foo bar "baz and qux"
(where the values were "foo", "bar", and "baz and qux"). You could work
around this with the transform
<transform set name=something -> \
emit set name=somethingelse %(value;prefix="value=")>
but that then doesn't work with singly-valued list values. So I went ahead
and made that work (both prefix and suffix). And you still have a problem
with MalformedActionError if you want to say something like
<transform set name=something -> \
emit file %(value;prefix="path=usr/bin/")>
since
file path=usr/bin/"foo bar"
doesn't parse properly.
I think for the most part people are going to use this rarely, and even
more rarely with complex values, or situations where they don't know the
kind of content of the attribute. I'm happy to entertain other
possibilities, but I'd like for this to remain reasonably simple, at least
right now.
> lines 210ff: I wonder if it's worth mentioning that leading '/' on
> paths are ignored so I'm assuming transforms don't have to worry
> about leading vs. no leading '/' ?
Eventually, I think there will be a distinction and we won't ignore leading
slashes, so I'd rather not document this.
> src/tests/cli/t_pkgmogrify.py:
> lines 76-110: Does transformation work on an action with unicode?
I did a couple of quick tests, and it appears to, yes. Both in a match and
in a print operation. I can add those two tests, but if you want something
more comprehensive, please file a bug.
> line 200: s/specifeid/specified/
Fixed.
> src/util/publish/pkgmogrify.py:
> line 315: duplicate of line 314?
Fixed.
> line 340: action.hash is only valid for file, license, signature.
> should this use the roughly the same logic as action.key, others
> to handle notfound?
I can change this to
attr = getattr(action, "hash", d.get("notfound", None))
which would allow you to say
<transform set -> print %(action.hash;notfound=dummy)>
though I doubt most people wouldn't have a "file" action matcher for this.
Note that at the moment there's no way to see whether the action has an
unspecified hash since it's initialized to "NOHASH" by the constructor.
> line 378: if it's always "pkg", why not make this a class atribute
> instead? I think it will override the inherited one.
Changed.
I've uploaded a new webrev, as well as an incremental, to
http://cr.opensolaris.org/~dduvall/pkg-pkgmog-1-2/
http://cr.opensolaris.org/~dduvall/pkg-pkgmog-2/
which also includes an implementation for Dan's request to eliminate
duplicate emitted actions.
Thanks,
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
