| Summary: | collision-protect does not handle symlinks | ||
|---|---|---|---|
| Product: | Portage Development | Reporter: | Thomas Bettler <thomas.bettler> |
| Component: | Core - Configuration | Assignee: | Portage team <dev-portage> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | aklhfex, ansla80, askwar, cedk, corsair, dmakovey, gentoo, gentoo, jieryn, m.langer798, pupeno, roma1390, sascha-gentoo-bugzilla, shrdlu, SourLover, tcunha |
| Priority: | High | Keywords: | InVCS |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.gentoo.org/show_bug.cgi?id=927640 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 193766, 147007, 149110 | ||
| Attachments: |
collision-samefile-check.patch
avoid unnecessary collisions via device and i-node numbers |
||
|
Description
Thomas Bettler
2005-02-05 04:27:07 UTC
There will always be problems with collision-protect, and symlinks are a nasty issue. The problems with following symlinks are circular symlinks and symlinks pointing to top level directories (and probably a few more). Depending how symlinks are used by the individual package you need one way or the other, but for implementation you have to pick one of them, and generally the current solution causes less problems. The third option would be to completely ignore all symlinks (and their subtrees). But won't it be an acceptable solution for collision-protect to check if the two different paths point to the same inode on the same filesystem? In this manner that bug could be resolved AFAIK and collision-protect would become a usable feature again. Well, they won't point to the same inode (that's only only true for hardlinks). Note for softlinks: This will also aply for softlinks, but only if they are on a upper (parent) level - and that's exactly the issue I mean. But if you insist we close this bug again....and imagine there is no problem. *** Bug 85066 has been marked as a duplicate of this bug. *** So it might be wise to dereference the links (as it is done by 'ls -L') generally not a good idea to CC random developers, especially when they are already on the alias that the bug is assigned to ;) *** Bug 117363 has been marked as a duplicate of this bug. *** Pardon, I did't realize that. Created attachment 76091 [details, diff]
collision-samefile-check.patch
After messing round some hours in the portage code I got this.
For me it works fine for the described scenario.
Can you please review my patch? I'm not a coder.
Thank you very much - hope it is of use
thomas, can you fire this off to gentoo-portage-dev@gentoo.org ml and shop it around for some feedback? Bugs is high traffic with only portage devs on the alias, I'd like to see this patch tested by devs/users a bit for feedback. Thanks *** Bug 134215 has been marked as a duplicate of this bug. *** *** Bug 137644 has been marked as a duplicate of this bug. *** *** Bug 137767 has been marked as a duplicate of this bug. *** *** Bug 138239 has been marked as a duplicate of this bug. *** *** Bug 145063 has been marked as a duplicate of this bug. *** Looking at the proposed patch, it looks pretty CPU intensive for packages with thousands of files. Wouldn't a better approach (though more intrusive) be to only store real paths and then also use real paths when searching for the duplicates? This would only have a O(n) complexity compared to the O(n*n) of the suggested patch. Thank you for this concept. Sounds great - feel free to get in touch with the developers and integrate it. Just a short note to my patch: It checks all the others package's files (n) only for the files not matching the recorded file (m). Conclusion: So it won't be (n*n) - it is (n*m) where m is the count of files moved from the original path and linked back. [But yes, it will be n==m if all the files have moved] Anyway, I prefer your proposed optimal solution, as I like the idea of recording [and comparing] only the absolute path and not any symlinks. As this needs only a change in the recording, it seems compatible with earlier and later versions of portage. Go for it... (In reply to comment #18) > Go for it... Ahem, ok, maybe I'll give it a try one of this days, though my experience with python (and portage) programming is practically non existent. But there must be a first time for everything, just don't hold your breath waiting for the patch :) (In reply to comment #17) > Looking at the proposed patch, it looks pretty CPU intensive for packages with > thousands of files. Wouldn't a better approach (though more intrusive) be to > only store real paths and then also use real paths when searching for the > duplicates? It's not necessary to store all the real paths in order to improve performance. We can resolve all the real paths on the fly and cache them for the duration of the collision-protect phase of he merge. That should be more robust and less intrusive that storing the real paths. (In reply to comment #20) > It's not necessary to store all the real paths in order to improve performance. Yes, good point. This will eliminate the need for a `FEATURES=-collision-protect emerge -e world` to update all the paths in the database. Created attachment 96534 [details, diff]
avoid unnecessary collisions via device and i-node numbers
This is fixed in svn r4431 (patch applies against 2.1.1). The concept is a identical to Thomas's patch except that device and inode numbers are cached for performance reasons.
Great! I love to see this getting fixed. Thank you. *** Bug 147711 has been marked as a duplicate of this bug. *** This has been released in 2.1.2_pre1. Haven't followed this bug closely, bu I really hope you don't follow absolute symlinks as that causes a very nasty performance hit (and was one of the reasons why symlink checking was disabled in the first place). (In reply to comment #26) > Haven't followed this bug closely, bu I really hope you don't follow absolute > symlinks as that causes a very nasty performance hit (and was one of the > reasons why symlink checking was disabled in the first place). The performance hit of the chosen implementation is negligible. It uses st_dev and st_ino pairs as unique file identifiers in the dblink.isowner() method. The st_dev and st_ino pair is exactly what os.path.samefile() uses to check if two files are the same (neglecting path). The st_dev and st_ino pairs (from stat results) are cached inside of the dblink class so that the stat calls are reduced to a minimum (and us a set for fast lookup). *** Bug 148476 has been marked as a duplicate of this bug. *** *** Bug 148512 has been marked as a duplicate of this bug. *** *** Bug 150574 has been marked as a duplicate of this bug. *** *** Bug 172341 has been marked as a duplicate of this bug. *** |
