On 08/21/12 10:12 AM, Brock Pytlik wrote:
On 08/17/12 16:23, Tim Foster wrote:
https://cr.opensolaris.org/action/browse/pkg/timf/pkglint-legacy/pkglint-legacy-webrev/
Thanks for taking a look!
engine.py:
nit 1029/1030: extra new line?
Yep, fixed.
1024/1026: What happens target.get_name doesn't match any package? It
looks to me like self.get_manifest will return None and then we'd get an
attribute error on line 1026.
That's true. The way this is being called today, that can't happen
since we're always looking for a specific manifest that we know exists
(because we're running lint checks on it) but there's no reason not to
be defensive here. I'll add that.
pkglint_action.py:
Maybe it's just that I don't really grok what the legacy action is
doing, but this comment doesn't make much sense to me. I think breaking
it up into 4 sentences, one per parameter might make things clearer.
Yes, I'll tidy that up. The pkglint legacy check tries to ensure that
if you have a 'pkg=SUNWfoo' attribute in your legacy action, then if a
user tries to do 'pkg install SUNWfoo', that should result in the
package containing the legacy action being installed (IPS follows
pkg.renamed packages, so pkglint needs to do the same thing in the
course of this check) If SUNWfoo doesn't exist, then we don't care.
This changeset makes us consider packages called SUNWfoo as well as
package-leaf names ..../SUNWfoo as satisfying our rules.
A more general question:
With a legacy action, are we guaranteed that the target will be a base
name? Could we (in the future perhaps) have a legacy action whose target
was something like "foo/bar"? In that case, I'm a bit concerned about
line 1022 of engine.py. If that's simply not something we care about
now, then feel free to ignore it :)
Target could certainly be 'foo/bar' eg.
set name=pkg.fmri value=pkg:/foo/[email protected]
legacy pkg=SUNWbar ....
we would expect that if SUNWbar exists (likely renamed) that the
rename-chain includes a dependency on pkg:/foo/bar.
How does line 1022 bother you here? In particular, we're adding that so
that the following package would be an acceptable use of the legacy action:
set name=pkg.fmri value=pkg:/foo/bar/[email protected]
legacy pkg=SUNWbar
by doing 'pkg install SUNWbar' we get pkg:/foo/bar/SUNWbar installed (or
a message saying it clashes with another package I've not included the
manifest for, pkg:/SUNWbar)
t_pkglint:
Line 2987 refers to "renamed_old" and "compat_renamed_old" but the
manifests read on 2993 are "renamed_old" and "compat_legacy" (and I
don't actually see compat_renamed_old" mentioned after that). Is the
comment wrong?
3005: commented line?
Yep good catch, I'll clean those up - thanks!
cheers,
tim
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
