Our "mail server" decided not to send this - resending.
-------- Forwarded Message --------
From: Tim Foster <[email protected]>
To: Brock Pytlik <[email protected]>
Cc: [email protected]
Subject: Re: [pkg-discuss] Code review: pkgdepend bypass & runpaths
Date: Mon, 07 Feb 2011 16:02:07 +1300
Hi Brock,
On Thu, 2011-02-03 at 22:23 -0800, Brock Pytlik wrote:
> On 02/ 3/11 08:29 PM, Tim Foster wrote:
> > On Thu, 2011-02-03 at 19:14 -0800, Brock Pytlik wrote:
> >> publish/dependencies.py:
> >> [snip]
> >> 319-320:
> .. errors (and not take elist as an argument). The code at line 333 would
> then look something like:
> es = __verify_run_path(a_run_path_str)
> if not es:
> return deps, elist + es, missing, pkg_attrs
> I don't think this changes the semantics (nor do I want it to), it's
> just a personal preference for not modifying arguments inside a function
> when it's not necessary.
Oh sure, sorry, I understand what you mean now. I'll change that.
> > Does that make sense?
> >
> Yikes, yes, that does make sense :) Thanks for the example.
Yikes indeed - I believe I've spotted a flaw in my reasoning
unfortunately. In my example, we have:
> depend pkg.d.depend.file=passwd pkg.d.depend.file=shadow \
> pkg.d.depend.path=foo \
> pkg.d.depend.path=bar
>
> The following files would satisfy the dependency:
>
> foo/passwd
> bar/passwd
> foo/shadow
> bar/shadow
>
> But, say we want to bypass only "foo/passwd", we need to split off a
> new
> dependency so that we instead generate the two dependencies:
>
> depend pkg.d.depend.file=passwd pkg.d.depend.file=shadow \
> pkg.d.depend.path=bar
>
> depend pkg.d.depend.file=shadow \
> pkg.d.depend.path=foo \
> pkg.d.depend.path=bar
The problem is, that we want any *one* of
bar/passwd
foo/shadow
bar/shadow
to satisfy both dependencies, essentially grouping them together. At
the moment, without linking the dependencies, this will result in an
unsatisfied dependency if we happen to deliver bar/passwd but not one of
either foo/shadow or bar/shadow.
We're not seeing this problem in the ongoing work on the ON manifests
yet as we haven't had to bypass a dependency that generates multiple
pkg.debug.depend.file values. (python modules are the only dependencies
that do this at the moment, and even then I'd imagine the vast majority
of users would simply bypass all of "*/foo.py" "*/foo.pyo", "*/foo.pyc"
"*/foomodule.so" "*/foo.so" and the standard runpaths to locate
"foo/__init__.py", avoiding this scenario completely )
In order to fix this, I suggest a "pkg.debug.depend.group" attribute to
provide hints to pkgdepend resolve on how groups of file dependencies
are related.
Dependencies that get spawned from a common parent dependency via the
bypass-generate mechanism would get the same auto-generated group-tag.
At the end of 'pkgdepend resolve', we scan the err list looking for
errors tagged with group tags that have already been marked as
satisfied. (which means we need to add pkg.debug.depend.group fields to
each of the Exception types used by pkgdepend resolve)
I can't think of a use-case where we'd want to expose this functionality
to manifest authors, hence the use of the internal attribute name, but
I'm all ears if anyone has one?
Apologies for not catching this before seeking code review - I'm
investigating the fix for this at the moment, but wonder whether it's
something we really want fixed before this wad goes back?
In the meantime, I've got the 'generate' part of the fix done now - the
'resolve' side is a bit more involved (need to mark each error produced
with the pkg.debug.depend.group tag responsible, if any)
> >> 470-472:
> >> I can't quite figure out what this comment's trying to explain.
> > Ok, it's where a user has specified bypasses that overlap - that is, a
> > user wants to bypass foo/shadow and */shadow -- the wildcarded entry
> > already matches "foo/shadow", so there's no need to go to extra effort
> > dealing with the "foo/shadow" case now (splitting off a new dependency
> > as explained above) when we can simply drop the pkg.debug.depend.file
> > entry for "shadow"
> >
> Ah, now I get it. If you include the example above in the comments, you
> might add this bit to the comment on 470 as well.
Sure.
> >> 519: How could the basename not be in dep.base_names? (other than it
> >> being '*', but I think 448 ensures it can't be '*').
> >>
> >> 522: Same as 519 for dir and dep.run_paths
> > Hmm, I'll have a think about that.
> > [snip]
OK, that's if we've previously dropped it from this dependency as a
result of another bypass entry. That is, we have:
depend pkg.d.d.file=passwd pkg.d.d.file=shadow \
pkg.d.d.path=etc pkg.d.d.path=var path=opt
and we specify bypasses on "etc/passwd" and "var/passwd" - we drop it
from the dependency for the 'etc/passwd' bypass, no need to drop it
again for the 'var/passwd' dependency.
I'll out send an updated webrev this week if we could decide on how to
proceed on the dependency splitting bug (a corner case, I believe).
cheers,
tim
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
