Shawn,
Thank you for the comments.
Shawn Walker wrote:
history.py:
line 555: a tuple instead of a list?
My Python newness is showing. Either a list or a tuple works here. Why
is a tuple preferable to a list?
image.py:
lines 1132, 1545: a tuple instead of a list?
Same question as above.
line 1409: I understand that on GNU/Linux platforms we see EACCES
instead of EEXIST (on OpenSolaris) when trying to remove a non-empty
directory (your notes from bug 5576) that we don't have permission to
remove. However, it doesn't seem right to me that we would ignore a
permissions exception here. Given the comment in
update_installed_pkgs, it seems like this case should be handled in
the except block instead.
Agreed. I'll need to work out a different solution for this. The
update_installed_pkgs method is called from 3 places; the other two
places check if the "state/installed" directory exists before calling
it, so the os.rmdir will never fail for those cases. So the only time
update_installed_pkgs is called when the "state/installed" directory
already exists is from retrieve_catalogs. If it exists and has entries,
and you don't have permission to delete it, you do get an EACCES on
OpenSolaris, so the PermissionsException does get raised. For a
do-nothing "pkg refresh" command executed as non-root on the root image
(by do-nothing, I mean one that doesn't actually fetch any new catalog
info), this results in a internal error with a traceback.
Maybe the os.rmdir should be replaced with an os.path.isdir (without
worrying about the directory actually containing any entries).
cli/t_actuators.py:
lines 167, 172: let's not use /bin/sh. who knows what shell that
will be? instead, specify /usr/bin/bash or /usr/bin/ksh or
/usr/bin/ksh93. /bin/sh is undefined.
None of those shells exist on Linux either. Linux doesn't include any
shells in /usr/bin. I had thought that /bin/sh was a pretty standard way
to reference a POSIX shell.
cli/testutils.py:
line 35: given that you trap this import as you did pspawn, why not
check for the presence of pwd in globals as you did for pspawn first?
Because get_su_wrap_user is never called on systems that don't have pwd
(Windows). On Windows we only run the API tests. But the test framework
reads in the Python modules (including testutils.py) for all tests even
though they are not run. To get the CLI tests to run on Windows is a
much bigger task that I'm not taking on this this changeset.
Would it be better to localize the import of pwd to the get_su_wrap_user
method?
Tom
Cheers,
begin:vcard
fn:Tom Mueller
n:Mueller;Tom
org:Sun Microsystems, Inc.;Update Center Software
adr:;;21915 Hillandale Dr;Elkhorn;NE;68022;USA
email;internet:[email protected]
title:Senior Staff Engineer
tel;work:877-250-4011
tel;fax:877-250-4011
tel;home:402-916-9943
x-mozilla-html:TRUE
version:2.1
end:vcard
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
