On 11/22/11 11:07, Brock Pytlik wrote:
On 11/22/11 10:02, Shawn Walker wrote:
On 11/10/11 15:51, Brock Pytlik wrote:
On 10/11/11 21:42, Brock Pytlik wrote:
Webrev:
https://cr.opensolaris.org/action/browse/pkg/bpytlik/18012-v1
Bug:
18012 cached manifests persist for packages not currently installed
even when copy in repository changes
Thanks for taking a look,
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
Updated webrev:
https://cr.opensolaris.org/action/browse/pkg/bpytlik/18012-v2
src/modules/client/api.py:
lines 80-81: Please don't use range here; that's overkill for a
simple list of numbers. I don't think it's that bothersome to
maintain the list and it's far simpler to read.
...
General:
I do not believe this is the right approach. The manifest verification
should be invisible to API consumers -- we shouldn't expose this sort
of internal implementation detail to consumers.
Given that whether or not we should reverify a manifest when doing pkg
contents depends on whether -r is used, behavior I thought we as a team
had agreed on, it's unclear how you not expose verification through the
api.
The current logic is also broken because it doesn't take into account
whether the manifest is for a package that's installed. If it is, then
it doesn't matter if the manifest is different than what the server
has now, it can't be replaced as that would invalidate the already
installed package.
Actually, it does take into account if the package is installed. See the
code in imageplan, among other places. It's just that the knowledge of
whether it's installed might be located differently than you'd prefer.
No, it doesn't. imageplan for example only bothers to do the check if
it's under newfmri. However, this doesn't account for the cases where
the fmri hasn't actually changed, but will be the same for both oldfmri
and newfmri (think pkg fix, facets, variants, mediators).
Unless I'm missing some other piece of logic that's actually limiting
this to not-yet-installed packages.
Here's what I think should be happening:
* pkgplan evaluation should trigger verification of manifests for new
packages; never an installed package
This happens with the code as provided. Please see the code in imageplan.
See above.
* manifests shouldn't be permanently cached unless a package is
actually installed; that's why manifests are automatically removed
when a package is uninstalled
So you want to change pkg contents -r so that it removes the manifest
after it shows it to the user?
Ideally, what would happen is that contents -r manifest retrieval would
never be done into /var/pkg. That's arguably a separate bug, so I would
be ok with not fixing the contents case as part of this one.
We do this already in the case that a user can't write to /var/pkg due
to permissions so that unprivileged users can use 'info', 'list', etc.
* the function to verify the manifest shouldn't be stuffed into
has_manifest or get_manifest, or at the very least, should not be
exposed through pkg.client.api
Well that sounds like a very different bug than the one that was filed.
If that's the case, please repurpose 18012 (or close it and file a new
bug) and I'll remove myself as the assigned engineer.
I don't see how; the exposure of verification through pkg.client.api
isn't a requirement for implementation and nowhere does it say to do
expose it in the original bug filed.
Whether you choose to work the bug further is up to you, I just don't
think we quite agree on the implementation chosen.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
