On 02/01/12 20:11, Shawn Walker wrote:
On 02/01/12 20:01, Brock Pytlik wrote:
On 02/01/12 19:33, Brock Pytlik wrote:
On 02/01/12 18:41, Shawn Walker wrote:
On 02/01/12 17:56, Brock Pytlik wrote:
Webrev:
https://cr.opensolaris.org/action/browse/pkg/bpytlik/7136706-v1

Bugs:
7136706 pkg operation planning can traceback for unprivileged users if
conflict cache files don't exist
7140772 image._load_actdict can find of isn't set when expected

So, I have a test case for this attached. It's almost identical to
the test case for the other bug we discussed. It'd be great if you
add it / verify it.

Your changes look right to me, although the wording of the synopsis
for the second one is a little awkward ("can find of" ?).

-Shawn
That test case doesn't cause this exception to be raised. It causes a
different traceback addressed by 7136705 (which is out for review and
includes that test case). The traceback in the bug can only happen if
a) 3 temporary files are created b) the 3 temporary files can be
removed c) an exception happens after the 3 temporary files are
created but before exiting that try block. I couldn't figure a way to
construct a test that did this in the test suite. I was able to
produce this exception by, for example, typoing a print command that
caused a traceback in the appropriate place. If you have an idea for
how to create such a scenario in the test suite, please let me know.
Short of adding a debugging flag solely to raise an exception in one
test case, something neither I nor Danek thought was reasonable, I'm
stumped.

Thanks,
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
I realized on the drive home that there's another way to trigger this
stack trace. One of the renames would need to throw an EnvironmentError
that wasn't an EACCES or EROFS. I don't see how to make that happen
either, but it seems like it might be slightly more possible than
satisfying the other set of conditions.

I suspect that's the key here -- I removed the /var/pkg/cache directory and then ran the pkg command as an unprivileged user. More than likely the issue is that we don't ensure that the /var/pkg/cache directory exists in this code before attempting to rename temporary files into it.

Hence, we're probably encountering ENOENT. (A quick test of that confirmed.)

It is legitimate to nuke the cache directory, although I admit it's not something that should generally be encountered in the course of normal execution. But the package system should be able to function fully without the presence of the /var/pkg/cache directory (that was the intent anyway).

-Shawn

Ok, I think we're on the page now. In any case, here's a new webrev which includes the test you provided. There are some other changes made to image.py that having this test made necessary. With the code as it, the test now produces the same error as 7136705, so I'll land this set of changes after that one lands. (7136705 is out for review and still needs someone to look at it.)

Webrev:
https://cr.opensolaris.org/action/browse/pkg/bpytlik/7136706-v2

Thanks,
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to