Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1936422
Summary: | rpm test transaction fails with upgrade of package that uses %pretrans to replace a symlink with a directory | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Luboš Uhliarik <luhliari> |
Component: | rpm | Assignee: | Michal Domonkos <mdomonko> |
Status: | POST --- | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | urgent | Docs Contact: | |
Priority: | urgent | ||
Version: | rawhide | CC: | bnater, igor.raits, mdomonko, mjw, packaging-team-maint, pmatilai, pmoravco, quantum.analyst, vmukhame |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | Type: | Bug | |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 1934919, 1936222, 1940412 |
Description
Luboš Uhliarik
2021-03-08 13:14:52 UTC
Old squid 4.13-3: # ls -la /usr/share/squid/errors/ |grep es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 ca-es -> ca drwxr-xr-x. 2 root root 4096 Mar 8 07:19 es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-ar -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-bo -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-cl -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-co -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-cr -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-do -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-ec -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-es -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-gt -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-hn -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-mx -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-ni -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-pa -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-pe -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-pr -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-py -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-sv -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-us -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-uy -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-ve -> es lrwxrwxrwx. 1 root root 2 Jan 28 22:22 es-xl -> es drwxr-xr-x. 2 root root 4096 Mar 8 07:19 templates New squid-5.0.5-3: # ls -la /usr/share/squid/errors/ |grep es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 ca-es -> ca drwxr-xr-x. 2 root root 4096 Mar 8 08:15 es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-ar -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-bo -> es lrwxrwxrwx. 1 root root 5 Mar 8 07:36 es-bz -> es-mx lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-cl -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-co -> es lrwxrwxrwx. 1 root root 5 Mar 8 07:36 es-cr -> es-mx lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-cu -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-do -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-ec -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-es -> es lrwxrwxrwx. 1 root root 5 Mar 8 07:36 es-gt -> es-mx lrwxrwxrwx. 1 root root 5 Mar 8 07:36 es-hn -> es-mx drwxr-xr-x. 2 root root 4096 Mar 8 08:15 es-mx lrwxrwxrwx. 1 root root 5 Mar 8 07:36 es-ni -> es-mx lrwxrwxrwx. 1 root root 5 Mar 8 07:36 es-pa -> es-mx lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-pe -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-pr -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-py -> es lrwxrwxrwx. 1 root root 5 Mar 8 07:36 es-sv -> es-mx lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-us -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-uy -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-ve -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 es-xl -> es lrwxrwxrwx. 1 root root 2 Mar 8 07:36 spq -> es drwxr-xr-x. 2 root root 4096 Mar 8 08:15 templates Haven't looked at details, but possibly a dupe of bug 1835424 (In reply to Panu Matilainen from comment #2) > Haven't looked at details, but possibly a dupe of bug 1835424 I thought it could be a dupe of the bug mentioned Comment 2, but it is mentioned there that it is a problem of files which are shared by two different packages. Is there any update on this? This issue is preventing to merge squid 5.0.5 to F34. Setting priority to urgent, since this is blocker for us. If this issue is not fixed soon, we won't have enough time to get squid v5 to RHEL-9 and test it properly. Apologies for not responding earlier; the root cause has been known to me for a while, but I got too invested in finding a way to fix it straight away, that I didn't bother sharing the details first and perhaps getting some feedback from others... Short version: So the issue here is that the directory "es-mx", which is replacing the same-named symlink pointing to "es", contains files with the same names (but different contents) as the original "es" directory. That generates a package self-conflict in RPM that we currently don't handle in any special way. Long version: Basically, whenever there are two file paths in a transaction that have different attributes or contents, RPM treats them as conflicting, and tries to either mitigate the conflict in some way or just aborts. Both the files in the transaction at hand, and the files recorded in the rpmdb for the already installed packages, are checked against each other prior to the transaction. Now, symlink replacement is a common enough type of file conflict that it gets special treatment in RPM. We have a simple heuristic [1] that assumes that the presence of a %pretrans scriptlet may somehow (hopefully) address the conflict in the real transaction, so we give it the benefit of the doubt and ignore the conflict in transaction test mode (remember, scriptlets aren't run in a test transaction). AIUI, this heuristic was specifically added [2] to handle some migration issues around the UsrMove in Fedora that happened in 2012. Since YUM runs a test transaction first (just like DNF does), there were similar failures in packages affected by the move (see [2] for more details). It was also documented in the Fedora Packaging Guidelines [3]. Basically, to make a symlink replacement work, all you need is remove the symlink in %pretrans using Lua, which is exactly what the updated squid.spec already does, so that's all correct, no issues there. The problem only appears when the new package ships files in both directories (the old symlinked and the new replacement) that share the same name but have different contents. This can easily happen if you have some kind of uniform directory structure where a set of file names is repeated in multiple subdirectories on the same level - just like the error pages in squid, which follow this pattern: /usr/share/squid/errors/<lang>/ERR_<TYPE> For example: /usr/share/squid/errors/es/ERR_ACCESS_DENIED What's happening with squid-5 is that the "es-mx" symlink which used to point to "es" is replaced with a standalone "es-mx" directory, along with a new set of ERR_* files inside of it, which for obvious reasons differ from the ones in "es". This happens because of the way we generate fingerprints for an internal hash table (which is later used to compare files for conflict detection, among other things) based on stuff like the basename and inode number; we use stat(2), as opposed to lstat(2), meaning that symlinks are followed. That way, on a package update, both the original file(s) and the new one(s) get the same fingerprint, and so we get a conflict later on. It is actually a self-conflict, i.e. *within* the same package (the updated one), since it effectively (from the system's PoV) ships two files at conflicting locations. For example, the following files are shipped by squid-5: /usr/share/squid/errors/es/ERR_ACCESS_DENIED /usr/share/squid/errors/es-mx/ERR_ACCESS_DENIED Due to "es-mx" still pointing to "es" on the system performing the transaction test, the second file gets the same fingerprint as the first one, due to the stat(2) call. There are multiple ways to make this work: 1) Extend the above heuristic also to package self-conflicts; this effectively means adding a similar check that we have in handleInstInstalledFile() also into handleOverlappedFiles(). That way, any self-conflicts would only be detected before the real transaction. The risk here is that, if a true self-conflict occurs, it would only be detected on the real transaction (meaning that e.g. %pretrans would have run by then). We could make the heuristic more accurate by only limiting it to a common path prefix that ends with the symlink in question, e.g. only conflicts underneath /usr/share/squid/errors/es-mx/* (knowing that "es-mx" is being replaced) could be ignored. However, that would bring other challenges such as what to do if we have multiple symlinks on the same path etc. It's probably not worth adding so much code complexity just to handle this little corner case. 2) Rework symlink handling in RPM. It's no secret that we have considered the symlink logic in RPM as some kind of technical debt for a long time. People have tried to fix it over the years, but it's a tough one. Certainly not something to be considered within the realms of this BZ, especially given its Critical severity... 3) A workaround in the packaging. This could actually be the quickest solution - just keep the "es-mx" symlink, but make it point to a new directory that contains the new translations. This means you'd have to maintain a downstream patch and possibly rebase it on every update, so it might not be as attractive of a solution overall, however it would allow us to approach this problem in a more holistic way perhaps (although the first solution above could achieve the same). So I'm a bit torn. In fact, I do have the first solution working in my local checkout, together with unit tests, so if nobody objects, I'll probably open a PR for that and call it a day. Otherwise, I'm all ears! [1] https://github.com/rpm-software-management/rpm/blob/a9678c2dce6abe4b9c7c86a363c999e215ff27bc/lib/transaction.c#L449 [2] https://bugzilla.redhat.com/show_bug.cgi?id=808750#c6 [3] https://docs.fedoraproject.org/en-US/packaging-guidelines/Directory_Replacement/#_scriptlet_to_replace_a_symlink_to_a_directory_with_a_directory Forgot to add: 4) Use lstat() instead of stat() for the fingerprinting. We can't do this unconditionally, though, since there's a reason we use the latter currently. But it's something to consider, too. Hi Michal, thanks for your wide explanation! As you mentioned, solution 1) or 2) would be best for me as a maintainer. Meanwhile this was not working, I decided for following solution: 5) Revert addition of es-mx translation. I reverted this translation, so squid v5 shipped in Fedora and RHEL-9 won't have it's es-mx language mutation (it will use spanish language instead as it was in squid v4). This is just a temporary solution, since there might be new language translations soon and we can not have it like this forever (taking car of dozens downstream patches only because of new added language translations). Thanks, Lubos, that makes things clear. I've opened a PR upstream, implementing option no. 1: https://github.com/rpm-software-management/rpm/pull/1684 This affects Fedora 33 (and presumably all releases) as well, cf the blocked bug report for R-shiny. |