On Wed 11 Nov 2009 at 03:41PM, Bart Smaalders wrote:
> A simpler one this time...
>
> http://cr.opensolaris.org/~barts/pkgmog/
>
> 12373 Need a programatic way of transforming manifests
Ok, I cheated by downloading your patch and playing with it.
As we all know, I'm cursed, so I found a few interesting edge
cases. I realize there's a lot here. Sorry to be a pain.
-dp
---
manpage: 16: pkgmod -> pkgmog
---
manpage 40: Suggest:
When processing the SPARC architecture, this macro would
be left unset, or set to the empty string. When processing
other architectures, this macro would be set to '#' (the comment
character) on the command line ...
---
manpage: 35 and 44 are two separate idioms... suggest maybe a bulleted
list, or some clearer way of delineating them?
---
manpage: 53: This says -I include_directory, but line 7 calls this a
searchdir.
---
manpage: 86: So it's not clear from this whether path and mode
are the only matchable attributes. Perhaps:
Matching criteria are of the form:
[action-type [action-type ...]] [attribute=<value-regexp>
[attribute=<value-regexp> ...]]
[
One of the action-types specified must match. All of the attributes
specified must match.
---
manpage: 87: I think a more complete sentence, outside of an example,
about the regexp syntax would be worthwhile. For example...
where would I go to learn this syntax?
It kind-of-sucks that our regexps will be bound to a specific language
platform. I also note from http://docs.python.org/dev/howto/regex.html
that there are two different python RE modules, so you might
need to be clear about which one. From reading the aforementioned,
I learned about all the RE flags which exist...
---
manpage 96: The behavior of replace not being specified is IMO
a little unintuitive-- and seems to prevent you from doing certain
transforms. It seems like this is standing in for a 'del attribute'
subcommand.
Here's my example. What if I wanted to delete some broken path prefix?:
$ pkgmog
<transform file -> edit path brokenness/ ''>
file brokenness/usr/bin path=brokenness/usr/bin/silly
You'd hope that this would yield:
file brokenness/usr/bin path=usr/bin/silly
But it will yield:
file brokenness/usr/bin
---
manpage: 105: Even more examples would be awesome.
---
manpage: 108: Just a note: this example seems to be different than what
we do in ON packages:
$ pkg contents -m SUNWcs | grep manifest-import|tail -1
file ... restart_fmri=svc:/system/manifest-import:default
---
Consider renaming def -> set-default (it will be rare, so making it more self
explantory might be good).
---
We previously discussed -v... at least file an RFE, please.
---
pkgmog.py: usage: Same comments as with the man page. Esp. the
refresh_fmri at 114.
---
[inputfile [outputfile]]
I thought sch had inhibitions or prohibitions about positional
parameters. Consider -o for output file.
---
276: I'm a little fuzzy on the behavior intended if $(sparc_ONLY) isn't
defined at all. If we find a macro which we cannot substitute, is that an
error? A warning?
---
310: Would it make sense to pass comments through?
---
376: My guess is you'll get a backtrace if the output file exists
and isn't writeable.
---
389: Please exit 99 if there's a backtrace.
---
t_pkgmog:
90 #shutil.rmtree(self.__test_dir)
?
Here are some tests I tried:
---
$ pkgmog
moop^D
Traceback (most recent call last):
File "/aux0/ws/pkg-build/proto/root_i386//usr/bin/pkgmog", line 386, in ?
main_func()
File "/aux0/ws/pkg-build/proto/root_i386//usr/bin/pkgmog", line 370, in
main_func
act = pkg.actions.fromstr(line)
File "/usr/lib/python2.4/vendor-packages/pkg/actions/__init__.py", line 161,
in fromstr
atype, ahash, attr_dict = _fromstr(string)
pkg.actions.MalformedActionError: Malformed action at position: 3:
doo
^
Catch this and supress the traceback?
---
$ pkgmog
set name=foo biz=baz
Traceback (most recent call last):
File "/aux0/ws/pkg-build/proto/root_i386//usr/bin/pkgmog", line 386, in ?
main_func()
File "/aux0/ws/pkg-build/proto/root_i386//usr/bin/pkgmog", line 370, in
main_func
act = pkg.actions.fromstr(line)
File "/usr/lib/python2.4/vendor-packages/pkg/actions/__init__.py", line 166,
in fromstr
action = types[atype](data=data, **attr_dict)
File "/usr/lib/python2.4/vendor-packages/pkg/actions/attribute.py", line 54,
in __init__
assert "value" in self.attrs
AssertionError
Perhaps a misbehavior in attribute.py?
---
I found this weird edge case:
$ pkgmog
set name=foo value=bar
<transform set name=foo -> drop>
None
I've dropped everything... I have None left :)
---
The grammar for transform doesn't error out on e.g.:
<transform set name=foo -> drop drop drop>
<transform -> drop>
---
Do errors leave the output file partially written? I don't know
what the best practice is.
---
$ pkgmog
<transform moop>
set name="foo"
^D
pkgmog: Missing -> in transform, file <stdin> line 0
Usage:
pkgmog [-I searchdir ...] [-A appendfile ...] [-D macro=value ...]
[inputfile [outputfile]]
<... complete usage message ...>
1) Complete usage really needed?
2) Line numbering should start at one?
---
There seems to be an undocumented behavior which is that pkgmog will
do some degree of auto-correction due to action processing:
$ pkgmog
set name=foo
set name=name value=foo
It'd be good to document whether the rules get applied before, or after the
transformation. Or, if this is a truly obscure special case I hit, never
mind, I guess.
---
Similarly:
$ pkgmog -Dsparc_ONLY='# Here is my comment.'
$(sparc_ONLY)
Did not do what is intuitively obvious. The presence of the # drops
the whole line, and it vanishes--- which is not obvious if you don't
know how actions are implemented. I think it's a documentation
issue-- this mentioned in the manpage but obliquely.
---
What is the order of processing? Macros first, then transforms?
When does -A processing happen? Maybe document the ordering.
Perhaps manpage lines 16-25 could be reworked to also explain
the order of operations.
---
$ pkgmog -D=
does not emit an error, and so $() becomes valid. Intended?
---
$ pkgmog
<include bizbaz>
^D
pkgmog: File not found "bizbaz"
Missing line number. Perhaps:
pkgmog: failed to find include "bizbaz" included from <stdin> at line 1
---
<include>: From the man page:
76 <include path>
77
78 This causes the named file to be searched for at the specified path,
79 prepending the specified search directories. When found, the
80 specified file is included.
So what's the intent if path is absolute? It appears that e.g.
<include /etc/motd> will... include /etc/motd.
I did eventually find an explanation in manifest_preprocessor.txt...
so maybe copy that into the man page.
In playing with this some, I'd be inclined to not allow absolute paths,
but I don't feel strongly I guess.
---
I also found that:
cd /etc
...
<include motd>
...
Works, and so also implies that "." is in the search path.
(not obvious from man page...). Is "." going to go first
or last in the search path if I also use -I?
---
t_pkgmog: Please add some negative tests:
Bad transforms
Bad command line options
Bad macros (if any?)
---
t_pkgmog: <include> has no test coverage.
-dp
--
Daniel Price, Solaris Kernel Engineering http://blogs.sun.com/dp
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
