Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter Lua deprecation warnings based on the originating rpm version #3270

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pmatilai
Copy link
Member

@pmatilai pmatilai commented Sep 3, 2024

Issuing deprecation warnings on packages built long time ago is just antisocial behavior when the user is powerless to do anything about it.

When running scripts, pass the builder rpm version to Lua through the registry, and filter out the warnings when the package was built with an rpm version where these functions were not yet deprecated.
Usage through rpmlua and macros always gets warnings: those are things that can technically be fixed by the user.

The first commit is prerequisite refactoring to make this possible.

Thoughts? This could be considered the beginnings of poor mans symbol versioning for rpm embedded Lua.
Tests would need to be added of course.

These fellas have enough arguments as it is, it's moderately hysterical
they don't actually accept the main struct as an argument. No functional
changes.
Issuing deprecation warnings on packages built long time ago is
just antisocial behavior when the user is powerless to do anything
about it.

When running scripts, pass the builder rpm version to Lua through the
registry, and filter out the warnings when the package was built with
an rpm version where these functions were not yet deprecated.
Usage through rpmlua and macros always gets warnings: those are things
that can technically be fixed by the user.
@ffesti
Copy link
Contributor

ffesti commented Sep 3, 2024

Anti social or not, the reason for these warning is that those packages won't work with the next RPM version. Not sure if keeping the users in the dark will do them any favors. Right now they can demand the vendor to fix their packages before it is too late.

@pmatilai
Copy link
Member Author

pmatilai commented Sep 3, 2024

I said this someplace else too, but: the thing is, I don't think we can actually proceed with the removal of these functions as originally planned. This stuff is used by very few packages but those very few happen to be corner stones of distros, and if we just remove the functions we make eg all current Fedora and RHEL versions uninstallable. That's just not acceptable really.

We can probably remove them in rpm 7...

@ffesti
Copy link
Contributor

ffesti commented Sep 3, 2024

if we don't actually remove the functions this is fine. We just need to issue the warnings in the last RPM version before removing them.

@pmatilai
Copy link
Member Author

pmatilai commented Sep 3, 2024

No, the point is that we need to warn on all new usages of these functions to scare people away from using them. Otherwise, we'll be in the same situation ten years from now and still can't remove those functions.

@ffesti
Copy link
Contributor

ffesti commented Sep 3, 2024

OK, I could have worded that better. Yes, we need to issue a warning for all new usages. We will need to issue a warning to all usages the release before we actually remove the functions - with the hope that old usage has died out til then.

@pmatilai
Copy link
Member Author

pmatilai commented Sep 3, 2024

Right, that I can agree with: we need to turn the warnings back to the always warning mode before actually removing them, probably more than one release in advance.

@pmatilai
Copy link
Member Author

pmatilai commented Sep 3, 2024

Hmm, probably there should be a switch to enable all deprecation warnings regardless of versions so it's possible to test whether you're affected or not. Maybe not necessary for the first version, but going forward.

@pmatilai
Copy link
Member Author

pmatilai commented Sep 3, 2024

Oh and to elaborate on the "poor mans symbol versioning" comment: what we could and probably should do is literally prevent the use of these functions in v6 packages. We now have (the beginnings of) means to track that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants