On 06/ 8/10 03:32 PM, Danek Duvall wrote:
Shawn Walker wrote:
...
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.
Seems fine; I actually didn't quite expect you to go this far -- I was
merely inquiring about what I thought might be an implicit assertion.
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.
No, it was more of a "does it work?".
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.
That seems perfectly reasonable to me and helpful in the event we
introduce hashes for other action types or introduce a new action type
that has a hash attribute.
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.
LGTM.
Cheers,
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss