On 4/6/06, Marius Mauch <[EMAIL PROTECTED]> wrote:
> On Thu, 06 Apr 2006 19:11:49 -0700
> Zac Medico <[EMAIL PROTECTED]> wrote:
> > The manifest code doesn't have very many use cases so I'd expect that
> > we would have hit most major problems by now (even with a small
> > sample). Any necessary changes are likely to be small patches. As
> > an alternative, we can cut the 2.1 branch at the point before
> > manifest2 was merged (2.1_pre7, essentially).
>
> Releasing 2.1 without manifest2 is a no go, it would significantly
> delay the deployment and transition. I'm not requesting to delay 2.1
> for another few months, just one more pre release so people get a
> chance to test it for one or two weeks.
The manifest2 code is still pretty raw from where I'm sitting and has
a few design problems.
Offhand,
1) usage of settings sucks (awareness of PORTAGE_ACTUAL_DISTDIR is an
indication right there that distdir should be passed in rather then
manifest code knowing of settings). Throwing around a big ole dict of
settings while easy, isn't exactly encapsulated/seperated all that
well (thus making any changes to config class that much more of a
mess).
2) triggering __init__ within create is pretty dodgy and definitely a
"wtf" trick
3) usual loading everything into memory rather then iterating over
objects (file objects in particular)
4) incomplete class; notably the sign chunks.
5) lack of protection in digest parsing; flipping through it, any
old/deviate from the norm digest's generated (say files//blah) the
code doesn't protect itself against.
6) impenetrable code.
This should be simple/easily grokable code- as is, I don't think it is.
fex.
cpvlist = [os.path.join(self.pkgdir.rstrip(os.sep).split(os.sep)[-2],
x[:-7]) for x in \
portage.listdir(self.pkgdir) if x.endswith(".ebuild")]
I'm not in front of a gentoo box right now, and the best I can figure
that code is doing is pulling the ebuild name and joining the ebuild
name + version to it- and I'm pretty sure that's not a correct
interpretation of it.
The code should be _readable_ by folks, fhashdict (fex) should have an
explanation in __init__ explaining it's structure. Else folks are
stuck popping open the interpretter, and inspecting the object to
figure out what the code sets it to- which sucks because if you have
to inspect the implementation to determine it's intentions, finding
bugs in the implementation is that much harder.
Realistially, tend to think the code would be cleaner/more readable if
split into manifest1 and manifest2 classes. Combine then via a
derivative class instead of making manifest2 code (which will live on)
nastier to read.
Aside from that, my usual rant about creating uneccessary lists, using
.keys() when should just be iterating over the dict all apply for this
code, in general efficient code (usually resulting in less lines)
applies.
Honestly? Main issue I have with the code is that while the previous
digest/manifest code was icky, it was at least known to be stable via
live testing/usage by users/devs (for better or worse). The new code,
while containing most functionality in one spot is roughly just as
dense/opaque in trying to eyeball, but it lacks a year+ of being in
actual testing. That combination right there makes it harder to view
as "good to go".
/me notes also that he applies same criteria to what he has in the
past tried to push into portage; beyond features/bug fixes, whats the
cost in terms of maintenance? Will someone else be able to actually
grok this code without having to use pdb or liter it with print
statements?
Not intending to be an ass about the code, just think it needs to be
simpler then it is. Effectively it's just marshalling code (yes it
triggers chksum calls, but that's minor); it's not that complex of a
thing and the implementation should reflect that (or at least be
heavily documented if not).
> > > The remaining feature I'd like to get into 2.1 is the
> > > tree-format-check issue, but that could probably be slipped in in
> > > the rc phase (don't really like that idea, but it's an option).
> >
> > I don't want to rush the development of new features such as
> > manifest2 or the tree-format-check. We have a 2.1 branch that, in
> > it's current state (2.1_pre7-r4, for example), provides significant
> > benefits over the 2.0.x branch. By delaying 2.1's release for the
> > addition of _new_ features, we run the risk of the release being
> > delayed indefinitely by "just one more feature" syndrome.
> > Personally, I'd rather have shorter release periods so that "just one
> > more feature" syndrome becomes less of an issue.
>
> Ehm, this is not "just one more feature", both manifest2 and
> the tree-format-check are things to improve forward compability (or for
> the latter even enable forward compability at all), so delaying them
> will hinder future development, not only for us.
> Also this isn't exactly news to you all as I sent my intentions already
> a while ago, and last I asked you all agreed with them, so is there any
> reason to rush this now?
We didn't all agree on tree-formal- last it was left at, there were
non-trivial criticisms of the implementation/design, and questions of
it's actual protection/gain- questions which need to be answered
before it's pushed in. Lack of compatibility checks sucks,
potentially broken compatibility checks suck far more. :)
Re: delaying support, tree-format isn't delaying anything tangible
yet, as such the push for it isn't quite as strong as manifest2.
~harring
--
gentoo-portage-dev@gentoo.org mailing list
