Summary: | sys-apps/portage-2.1.4.4 unmerge broken symlink as it is a directory | ||
---|---|---|---|
Product: | Portage Development | Reporter: | Andrey <ahipp0> |
Component: | Core | Assignee: | Portage team <dev-portage> |
Status: | RESOLVED FIXED | ||
Severity: | normal | Keywords: | InVCS |
Priority: | High | ||
Version: | unspecified | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Bug Depends on: | |||
Bug Blocks: | 288499 | ||
Attachments: |
Proposed way to deal with unmerge-orphans. (not tested)
updated patch to ignore file's md5 sum |
Description
Andrey
2008-10-09 08:49:30 UTC
(In reply to comment #0) Let me demonstrate the behavior. 0) Create simple ebuild "test-bug/bug/bug-1.ebuild" src_install() { dodir /qqqq } 1) Install the package hippo_comp ~ # ebuild test-bug/bug/bug-1.ebuild merge * checking ebuild checksums ;-) ... [ ok ] * checking auxfile checksums ;-) ... [ ok ] * checking miscfile checksums ;-) ... [ ok ] >>> Unpacking source... >>> Source unpacked. >>> Compiling source in /var/tmp/portage/test-bug/bug-1/work ... >>> Source compiled. >>> Test phase [not enabled]: test-bug/bug-1 >>> Install bug-1 into /var/tmp/portage/test-bug/bug-1/image/ category test-bug >>> Completed installing bug-1 into /var/tmp/portage/test-bug/bug-1/image/ * checking 0 files for package collisions >>> Merging test-bug/bug-1 to / >>> /qqqq/ >>> test-bug/bug-1 merged. 2) Delete the directory and create broken symlink hippo_comp ~ # rm -r /qqqq/ hippo_comp ~ # ln -s /qaz /qqqq 3) Unmerge the package hippo_comp ~ # ebuild test-bug/bug/bug-1.ebuild unmerge No package files given... Grabbing a set. <<< dir /qqqq Notice the status and see, that there is no more symlink.:) If we create a symlink to a directory, we'll get: hippo_comp ~ # ebuild test-bug/bug/bug-1.ebuild unmerge No package files given... Grabbing a set. --- !empty dir /qqqq If we create a symlink to a file, we'll get: hippo_comp ~ # ebuild test-bug/bug/bug-1.ebuild unmerge No package files given... Grabbing a set. <<< dir /qqqq I think this is the same as: http://archives.gentoo.org/gentoo-alt/msg_2d3dfb3888ac4dcd4e64e1539ef039ff.xml and could possibly be fixed by r14248 (In reply to comment #2) > I think this is the same as: > http://archives.gentoo.org/gentoo-alt/msg_2d3dfb3888ac4dcd4e64e1539ef039ff.xml > and could possibly be fixed by r14248 Sounds good and looks similar. Will try to update my Prefix tomorrow and test the fix. Are there any chances in merging this to trunk or 2.1.6 branch upon successful testing? (In reply to comment #2) > I think this is the same as: > http://archives.gentoo.org/gentoo-alt/msg_2d3dfb3888ac4dcd4e64e1539ef039ff.xml > and could possibly be fixed by r14248 Thanks, I've merged that in r14250. (In reply to comment #3) > Are there any chances in merging this to trunk or 2.1.6 branch upon successful > testing? It will be in the next stable release which is due soon (it will be a new 2.1.7 branch). (In reply to comment #3) > Sounds good and looks similar. > Will try to update my Prefix tomorrow and test the fix. I haven't yet released a portage with that fix in Prefix. > Are there any chances in merging this to trunk or 2.1.6 branch upon successful > testing? The patch is easy to apply yourself so you can test it straight away: http://sources.gentoo.org/viewcvs.py/portage/main/trunk/pym/portage/dbapi/vartree.py?r1=14236&r2=14250&diff_format=u (In reply to comment #6) > The patch is easy to apply yourself so you can test it straight away: > http://sources.gentoo.org/viewcvs.py/portage/main/trunk/pym/portage/dbapi/vartree.py?r1=14236&r2=14250&diff_format=u I can confirm, that this changeset fixes this bug with FEATURES="-unmerge-orphans". But with FEATURES="unmerge-orphans" the symlink is still unmerged at line 2470 in vartree.py@14250. Again the incorrect behavior is in the following cases: 1) Symlink is broken 2) Symlink points to a file (symlink is created manually instead of directory after package merge simulating another package doing something nasty in postinst phase) hmmm, it looks to me as the unmerge-orphans kicks in too early, as it should use the same logic as the code right below it IMO, making sure only mtime changes are allowed, but not changes in type. Created attachment 204066 [details, diff]
Proposed way to deal with unmerge-orphans. (not tested)
(In reply to comment #8) > hmmm, it looks to me as the unmerge-orphans kicks in too early, as it should > use the same logic as the code right below it IMO, making sure only mtime > changes are allowed, but not changes in type. Is it better to remove the whole 'if unmerge_orphans' block and integrate its logic into the below 'if pkgfiles[objkey][0] == "dir"' code? I've attached a patch proposing such a change. It's not tested and the full logic isn't verified. (In reply to comment #7) > Again the incorrect behavior is in the following cases: > 1) Symlink is broken > 2) Symlink points to a file > (symlink is created manually instead of directory after package merge > simulating another package doing something nasty in postinst phase) I'm not sure that we should be leaving stuff like this intact with unmerge-orphans. The idea behind unmerge-orphans it to avoid leaving any orphans behind (whether they were created by a nasty postinst phase or not). (In reply to comment #9) > Created an attachment (id=204066) [edit] > Proposed way to deal with unmerge-orphans. (not tested) Aside from my reservation about leaving orphans when unmerge-orphans is enabled, the patch seems reasonable, except that it will also change unmerge-orphans behavior for regular files whose md5 differs from when they were originally installed. Also, I've just realized that the unmerge-orphans code has protection for symlinks like /lib -> lib64, and it seems like the change from r14248/r14250 might remove similar protection, so I'm thinking about reverting it. (In reply to comment #11) > Also, I've just realized that the unmerge-orphans code has protection for > symlinks like /lib -> lib64, and it seems like the change from r14248/r14250 > might remove similar protection, so I'm thinking about reverting it. Nevermind this, the code seems safe since 'not stat.S_ISDIR(lstatobj.st_mode)' should hold for lib -> lib64 symlinks. Created attachment 204789 [details, diff]
updated patch to ignore file's md5 sum
(In reply to comment #11) > (In reply to comment #9) > Aside from my reservation about leaving orphans when unmerge-orphans is > enabled, the patch seems reasonable, except that it will also change > unmerge-orphans behavior for regular files whose md5 differs from when they > were originally installed. Agree, I missed this in the first patch. Taking your remark into account I've attached an updated patch. (In reply to comment #11) > (In reply to comment #7) > > Again the incorrect behavior is in the following cases: > > 1) Symlink is broken > > 2) Symlink points to a file > > (symlink is created manually instead of directory after package merge > > simulating another package doing something nasty in postinst phase) > > I'm not sure that we should be leaving stuff like this intact with > unmerge-orphans. The idea behind unmerge-orphans it to avoid leaving any > orphans behind (whether they were created by a nasty postinst phase or not). Well, rethinking the portage behavior and taking into account it's collision-protect feature I now agree with you. But I'd prefer portage to inform me, that it noticed some inconsistencies. For example, "<<< !dir /qqqq" instead of "<<< dir /qqqq" or "<<< !mtime /foo" instead of "<<< obj /foo". (In reply to comment #4) > (In reply to comment #2) > > I think this is the same as: > > http://archives.gentoo.org/gentoo-alt/msg_2d3dfb3888ac4dcd4e64e1539ef039ff.xml > > and could possibly be fixed by r14248 > > Thanks, I've merged that in r14250. > Via gitweb: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=25d23593b4fa78f7e9694498ef71bca13acdb061 This was released in portage-2.1.7. Please file a new bug for any remaining issues. |