-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
vartree: update owner and mode on replaced directories #1381
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Henning Schild <[email protected]>
This patch is likely not fully correct yet and the whole story might deserve a bug or i misunderstand something. Why do i propose this patch: When an existing directory is replaced its wanted owernship and permissions are simply kept, which might be wrong like in my case. And this can likely be called a bug. Or maybe we at least need some warning here, in case it is intentional. repro yourself:
So these ebuilds could in fact never update perms and owners if they needed to, i tried that as well by modifying one. And i also tried "acct-user/cron". In fact in the case of "acct-user/cron" and "acct-user/mail" we have other packaging dropper their keep files, but these spool dirs will fill up anyhow i.e. with emails. So the dirs will never be deleted and never be updated. expected result:
|
Please let me know how to proceed, probably open a bug in some category and provide more detail. I would be happy to write the fixing patch myself, should we come to the conclusion that we have a bug that needs fixing. |
mydstat.st_uid != mystat.st_uid | ||
or mydstat.st_gid != mystat.st_gid | ||
): | ||
os.chown(mydest, mystat[4], mystat[5]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and the later chown
can probably be done unconditionally, maybe even for symlinks
the code is already pretty wild and hard to read
but for now i opted for a conservative way where addition warning prints could be added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be rebased on top of #1382 and not use magic numbers
The way i found this was because a user receiving mails could not write to the spool, so perms for that case where too restrictive. I can imagine that in other scenarios where perms have been too loose it would in fact be hard (postinst) to adjust them later, and whatever one would specify in an ebuild would not have an effect on updates or reinstalls over keepdirs, just for fresh installs. |
It seems there is a whole series of bugs around that topic already, including a suggestion to add a FEATURE. From this bug https://bugs.gentoo.org/654138 one can find related bugs and duplicates. But i could not really parse why nothing was done so far. Especially i did not really find what the benefit of the current implementation is, it seems to be intentional but really weird and confusing. Looking through patches in i.e. acct-user/* i see several commits that likely did not work as intended for users who had the packages installed before. |
No description provided.