| Summary: | sys-apps/portage-2.1.8.3: emerge does not properly replace a directory with a symlink | ||
|---|---|---|---|
| Product: | Portage Development | Reporter: | Andreas K. Hüttel <dilfridge> |
| Component: | Core | Assignee: | Portage team <dev-portage> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | chithanh, Martin.vGagern, pms |
| Priority: | High | Keywords: | InVCS |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| See Also: |
https://bugs.gentoo.org/show_bug.cgi?id=664940 https://bugs.gentoo.org/show_bug.cgi?id=668560 |
||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 789729, 373933, 376129, 409359 | ||
| Attachments: | Output of emerge --info | ||
|
Description
Andreas K. Hüttel
2010-07-02 21:50:14 UTC
Created attachment 237331 [details]
Output of emerge --info
Unfortunately, this case is silently ignored. When a regular file is blocked by a directory, it triggers an eerror message and the file is merged with a temporary name. We can do the same for symlinks. I can confirm this bug with app-text/kiwix. Additionally, I noticed that if you downgrade the package after it was properly installed, it will not replace the symlink with the directory but will follow the symlink and copy its files where the symlink points to. (In reply to comment #2) > Unfortunately, this case is silently ignored. When a regular file is blocked by > a directory, it triggers an eerror message and the file is merged with a > temporary name. We can do the same for symlinks. This is fixed in git: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=87df7332c631b96a2d238abd2526eb5bb1a6e4f6 (In reply to comment #3) > Additionally, I noticed that if you downgrade the package after it was properly > installed, it will not replace the symlink with the directory but will follow > the symlink and copy its files where the symlink points to. We can detect this case, but how to handle it is another matter. When you remove a directory symlink like that, two different paths which were once equivalent become different, and there may be mixing of contents that would be difficult to reverse. @Andreas: per comment #1, you realize that's directly banned in PMS right? """ \section{Symlinks} Symlinks are merged as symlinks onto the filesystem. The link destination for a merged link shall be the same as the link destination for the link under \t{D}, except as noted below. The method used to perform the merge is not specified, so long as the end result is correct; in particular, merging a symlink may alter or remove the symlink under \t{D}. Ebuilds must not attempt to merge a symlink on top of a directory. """ This isn't a confirmed bug; it's a violation of pms. Portage may not signal it correctly, but that still doesn't make the ebuild valid... @Zac: rev 87df73 needs to be bound to a specific EAPI; enabling it across all is embrace/extend of PMS EAPIs (meaning I yell). Change PMS if desired, but breaking from the spec in this manner doesn't fly. Options? I think it is ok to error out when the forbidden directory replacement by symlink is detected. Silently not merging files or even leaving orphaned directories is maybe not desirable. (In reply to comment #7) > I think it is ok to error out when the forbidden directory replacement by > symlink is detected. You're saying that an incompliant ebuild that claims it's compliant, and blows up half way through a 100 pkg merge is acceptable? Because most users would find that to be *not* acceptable. Plus any such ebuild wouldn't be allowed in the tree (and would likely get QA on their ass if they knowingly introduced the incompliance). Most annoyingly, if portage goes and accepts behaviour like this, people don't bitch about the ebuild author breaking spec- they bitch that the PM that is spec compliant is broken. Catch 22, and why the embrace/extend isn't going to continue. The manager should throw a hissy fit, but the ebuild shouldn't even be marked that EAPI in the first place- that's the kicker here. If portage goes and handles incompliant ebuilds, then the standard winds up being forced to that (and folks who write alternative managers again get kicked in the nuts for abiding to a spec). Either way; those really are the options. Mark the ebuild as a different EAPI; portage flips on the merging for that case (hell, get PMS EAPI5 to include it). Alternatively try to get the spec retroactively updated. embrace/extending the spec isn't an option however. (In reply to comment #8) > (In reply to comment #7) > > I think it is ok to error out when the forbidden directory replacement by > > symlink is detected. > > You're saying that an incompliant ebuild that claims it's compliant, and blows > up half way through a 100 pkg merge is acceptable? I think he meant to make it bail out at the collision detection phase. It already bails out if the directory is owned by another package. We can easily tweak it to bail out whenever a symlink is intended to replace a directory. Yes, what zmedico said. There is no way for the ebuild to properly detect this case in advance. The ebuild could even be a binpkg that was built way before the offending directory was installed in the place where the symlink goes. Now the collision-protect code makes it abort before pkg_preinst: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=a4a4d87786e6c0c34370870af2071904d6184a9d (In reply to comment #3) > Additionally, I noticed that if you downgrade the package after it was properly > installed, it will not replace the symlink with the directory but will follow > the symlink and copy its files where the symlink points to. I've made it so this case will trigger an eerror log message: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=4bb08136f073024c5d31dceb1618b6f4e7246369 At first glance, it looks like that _elog/eerror doesn't actually abort it (meaning it just whines, and continues to accept). What's the exact behaviour when this comes up? For directory replacing symlink, it preserves the symlink and triggers an eerror message. Does PMS say anything about this case? The change in comment #12 is in the unmerge code where it's already too late to abort. We could extend the collision detection code to check for directories colliding with symlinks. Right now it only checks for regular files and symlinks colliding with stuff. (In reply to comment #14) > For directory replacing symlink, it preserves the symlink and triggers an > eerror message. Does PMS say anything about this case? Namely that the ebuild's not allowed to try it; the eerror message I presume is logged, and it continues? (In reply to comment #15) > (In reply to comment #14) > > For directory replacing symlink, it preserves the symlink and triggers an > > eerror message. Does PMS say anything about this case? > > Namely that the ebuild's not allowed to try it; the eerror message I presume is > logged, and it continues? Right, it continues in this case, unlike the reverse direction. It's the symlinks that tend to cause problems, so maybe it's reasonable to keep it this way, as a one-way migration path away from symlinks that point to directories. (In reply to comment #16) > (In reply to comment #15) > > (In reply to comment #14) > > > For directory replacing symlink, it preserves the symlink and triggers an > > > eerror message. Does PMS say anything about this case? > > > > Namely that the ebuild's not allowed to try it; the eerror message I presume is > > logged, and it continues? > > Right, it continues in this case, unlike the reverse direction. It's the > symlinks that tend to cause problems, so maybe it's reasonable to keep it this > way, as a one-way migration path away from symlinks that point to directories. Suggest folk read the merging section... it's all there. Rules of the road: Symlink on dir: no bueno, not allowed; this means it's an error, not a warn and continue, it's a broken ebuild. Do not pass go, do not collect 200, and do not convert this into anything other than a stopping error without getting the spec changed (doing otherwise means it's embrace/extend breaking the spec). Dir on symlink: allowed, must merge such that the "end result is correct". Implied it's merging into the symlinks target, although per the vague wording, ebuilds rely on differing behaviour there. Standardizing it is obviously useful. Changing any of that behaviour requires mangling PMS. Realistically if an ebuild tries relying on a specific "dir on symlink" behaviour, that needs to be locked down in the spec. So them's the rules; how is portage behaving in light of those requirements? (In reply to comment #17) > Symlink on dir: no bueno, not allowed; this means it's an error, not a warn and > continue, it's a broken ebuild. Do not pass go, do not collect 200, and do not > convert this into anything other than a stopping error without getting the spec > changed (doing otherwise means it's embrace/extend breaking the spec). It aborts for this case as required by the spec, with this commit: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=a4a4d87786e6c0c34370870af2071904d6184a9d > Dir on symlink: allowed, must merge such that the "end result is correct". > Implied it's merging into the symlinks target, although per the vague wording, > ebuilds rely on differing behaviour there. Standardizing it is obviously > useful. The symlink is protected from being unmerged, so that the end result is as correct as possible, with this commit: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=4bb08136f073024c5d31dceb1618b6f4e7246369 > So them's the rules; how is portage behaving in light of those requirements? I think it fits the spec now. (In reply to comment #18) > I think it fits the spec now. Thank you. I'll stop being a pain in the ass about this issue now ;) @Andreas: if you want to change the spec behaviour in this case, I'd suggest opening a ticket for it. Might gain some traction, although it would likely be eapi5 and higher. This is fixed in 2.1.10.8 and 2.2.0_alpha48. *** Bug 315425 has been marked as a duplicate of this bug. *** This has not been fixed but simply worked around with an emerge abort (collision detect) when the OP's problem is detected. In it's current state, Portage *still* cannot upgrade a package where a directory is replaced with a symlink. (In reply to Rick Harris from comment #22) > This has not been fixed but simply worked around with an emerge abort > (collision detect) when the OP's problem is detected. > > In it's current state, Portage *still* cannot upgrade a package where a > directory is replaced with a symlink. Portage can't do that without violating PMS: https://dev.gentoo.org/~ulm/pms/head/pms.html#x1-14700013.4 > Ebuilds must not attempt to merge a symlink on top of a directory. |