Add el9toel10 actor to handle symlink -> directory with ruby IRB. #1304
Conversation
Thank you for contributing to the Leapp project!Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build.
Note that first time contributors cannot run tests automatically - they need to be started by a reviewer. It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported,
See other labels for particular jobs defined in the Please open ticket in case you experience technical problem with the CI. (RH internal only) Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra. |
|
review please @oamg/developers |
|
/packit copr-build |
That depends, are there other packages that stand to gain from such work? Is that a refactor you'd consider worthwhile endeavor? From my view of a packager who fixes the upgrade for just this problematic part of the package, I prefer simpler copy paste for these 2 upgrade cases, but I don't know if this is now more common problem or still applies only to a package or few. Ruby is back in line with Fedora with this and hopefully that's the end of the broken symlink story for ruby WRT LEAPP... I am also going off of the comment on bugzilla regarding not needing generic solution https://bugzilla.redhat.com/show_bug.cgi?id=2030627#c13 If the world around changed in the time being making that comment not accurate then we can figure something out. |
I think that you are speaking about something different than me. The comment in BZ is not related anyhow to what I am suggesting :) I am speaking only about ruby all the time. I am not speaking about any other packages. Let's sync via call. |
|
Discussed on the call. We will keep the actors separate, as the reason for both IPUs is different for this handling, but docstring should be updated in both actors to make it clear. |
The `/usr/share/ruby/irb` path is a symlink in RHEL 9, but a regular directory in RHEL 10. This puts us back in line with RHEL 8 and Fedora in terms of the path's file type regarding the rubygem-irb package. Since this was not handled on RPM level, handle it as actor again. This was copied and adjusted from same-named el8->el9 actor. We do not care about the validity or target of the symlink, we just remove it to allow DNF create the correct directory on upgrade. Without this workaround, the upgrade will fail in transaction test with reports of file conflicts on the directory path. Users should not expect to ever retain anything in this directory.
In RHEL 10, the directory is a regular directory again. The 2 actors are separate over creating a common solution for both. Expand in the docstring on the reason for the el8->el9 actor to differentiate them apart.
|
@pirat89 updated the docstrings.I did the docstring update for el8->el9 in separate commit as I don't see it as something hide-able in the el9toel10 commit. And squashing is easier than splitting imo... |
|
/packit copr-build |
|
@jackorp merged! thanks for the contribution!! |
pirat89
added
the
changelog-checked
The
/usr/share/ruby/irbpath is a symlink in RHEL 9, but a regular directory in RHEL 10.Simply remove the symlink to then let the RPM mechanism create the correct directory on update.
Users should not expect to ever retain anything in this directory.
Fix for OAMG-12187 .
instead of
readlinkpresent in el8->el9 actor, I just usedtest -Lsince here we only care if there is any symlink or not, contents or validity of where it is pointing is not important for the upgrade.I left the rest as-is from a copy paste of the actor, I think naming and such still pretty much fits.