On 03/ 2/12 12:11 AM, Abhinandan Ekande wrote:
Made the changes to set owner as pkg5srv for /var/cache/pkg/sysrepo.
Added the test case also.
The updated webrev is located at :
https://cr.opensolaris.org/action/browse/pkg/saurabhv/CR-7120902-rev2/webrev/
src/sysrepo.py
line 293, don't hardcode the name of the cache directory - there's a
cache_dir variable passed to the method that you should use instead.
line 309, why are you changing the permissions?
line 310, you're chowning all files in the cache, which seems like
overkill, but you're also doing it 3 or 4 times. I think it's enough to
simply ensure the cachedir itself has the correct permissions.
src/tests/cli/t_sysrepo.py
line 484, if something goes wrong with the sysrepo, this could cause the
testsuite to hang indefinitely (which would be bad). It would be better
to look for the existence of the cache directory a fixed number of
times, before giving up and throwing a test failure.
cheers,
tim
Completed unit testing. As well ran the test suite which was successful.
Thanks,
Abhi.
On 02/24/12 16:18, Abhinandan Ekande wrote:
Thanks Brock and Tim for review.
I will make the changes to set owner as pkg5src for sysrepo directory
and post the webrev.
Thanks,
Abhi.
On 02/24/12 12:23, Tim Foster wrote:
On 02/24/12 07:20 PM, Brock Pytlik wrote:
On 02/23/12 21:03, Abhinandan Ekande wrote:
Folks,
Please review fix for CR :
7120902 var/cache/pkg/sysrepo recreated with broken perms
This change looks correct, but I'd like to see a test added to pkg's
test suite to check for this condition. In addition, I'm curious about
the user and group, shouldn't we make sure that the directory is
created
as pkg5srv, and not root as described in the bug?
Yep, it should be created 0755 as the pkg5srv user, just the way it
was packaged in the system repository package manifest:
dir group=bin mode=0755 owner=pkg5srv path=var/cache/pkg
dir group=bin mode=0755 owner=pkg5srv path=var/cache/pkg/sysrepo
The sysrepo.py script runs as root but chowns everything to pkg5srv,
since that's the user that Apache ultimately runs its processes as.
I missed the original bug, but deleting random packaged system
directories doesn't seem to be a good move on the whole. I agree it'd
be nice if we worked around every occurrence of that however.
cheers,
tim
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
