On 11/ 6/10 06:23 PM, Danek Duvall wrote:
Shawn Walker wrote:
http://cr.opensolaris.org/~swalker/pkg-iformat/
license.py:
- Why support version 3 images in get_local_opener(), but nowhere else?
So that you can perform read-only operations such as viewing license
information for packages in a version 3 image. However, since I don't
allow installing or updating packages in a version 3 image, support
elsewhere isn't needed.
...
imageconfig.py:
- The image configuration is at version 3, but the image layout is at
version 4?
Correct.
- line 139: should the image config file have a version, too?
The pkg.config class has its own internal versioning property that
tracks this.
...
image.py:
- line 487: Can you explain why this is safe?
Because configuration classes provide a set of declarations that define
what each version of the image configuration looks like, and enforce
that view of the configuration on load.
...
- line 632: Can you explain why this is safe?
Based on the structure, I have no reason to believe otherwise.
- line 638: this can move out one level
I need it to reload in both cases. Otherwise, I'm not understanding.
- line 697: If self.version< 4 and self.__user_cache_dir, then
__write_cache_dir doesn't end up getting set. Ditto with
_incoming_cache_dir. Is that a problem?
I've reworked this again because I'm not sure what was intended either.
It seemed so clear at the time :/
- line 701: was the "__" prefix used for directory names? Why do we need
it now?
Don't remember; removed.
- line 799, 806, 816: should we set self.version to self.CURRENT_VERSION
successfully completing update_format()?
Not for 799 and 806; self.version reflects on-disk version.
For 816, self.__upgrade_image_format() handles this indirectly through
its call to find_root which reloads configuration.
- line 1036: should we handle EXDEV specially?
I wasn't aware we were going to support random splits of /var/pkg into
separate filesystems. And in particular, it seems odd that /var/pkg
would be a separate filesystem.
- line 1141: I thought I saw somewhere else that you'd moved whether a
publisher was disabled or not directly into the configuration file,
rather than continuing to have a separate one.
Correct; this just copies it somewhere else so that imageconfig can load
the old information, transform it, and then remove it.
- line 1168: How does this work if there are mountpoints beneath imgdir?
Are we affected by the recently-putback PSARC/2010/385?
See note about EXDEV above.
- line 1313: I'd be really careful about assuming we understand versions
later than ourselves. In fact, I'm not sure I'd do it at all.
My assumption is all newer image formats will use the same basic
structure. If the image format is newer than what is supported, then an
exception will be raised during __set_dirs so this path would never be
reached.
- line 1842: Why do we have to iterate over all installed packages,
rather than just iterating until we get to the one we care about? Is
there some chance we can run into a package we think is installed from
multiple publishers, and we choose the last?
No idea; changed.
- line 1850, 1864: Can you explain why?
Yes, because search indexes FMRIs without the publisher. So when it
asks for a manifest, it provides an FMRI without one. Since I know that
such consumers only care about installed packages, this is easily
accounted for.
- line 3038: Should we warn the user that what they're doing is not
necessarily representative of the command that would actually get run?
I don't know. If so, what do you want the message to be?
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
