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

Remove deprecated no-op setInterruptSafety() call #52

Closed
wants to merge 1 commit into from

Conversation

dmnks
Copy link
Contributor

@dmnks dmnks commented May 18, 2023

This function no longer has any effect in RPM since the removal of the Berkeley DB backend in RPM 4.16. The Python binding has been deprecated for a while now, and is completely gone in RPM 4.19, so let's move with the times.

More info here:

rpm-software-management/rpm#1946
rpm-software-management/rpm#1992
rpm-software-management/rpm#1994

@xsuchy
Copy link
Owner

xsuchy commented May 19, 2023

This will not be easy. It was added as a fix for 66b896e to solve https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=1236722
I am on holiday now, will check this after a week.

@dmnks
Copy link
Contributor Author

dmnks commented May 20, 2023

OK, now I realize this PR was a bit rushed on my side, I somehow mixed up the chronology of the commits linked above and thought the Python binding had been no-op for a long time. Whereas, it really was only deprecated and made no-op in 4.18 which came out last year. 🤦 Apologies for that.

That said, the functionality itself might indeed be redundant now that we no longer use BDB, but depending on your release model, you may want to keep the code compatible with older RPM versions, too. In that case, we could perhaps apply this patch downstream only (in Rawhide).

Anyway, let's revisit when you're back.

This function became a no-op [1] in RPM 4.18 where it was also
deprecated [2], and is completely gone in RPM 4.19 [3].  Adjust the
version check accordingly.

Note that the function itself seems to have no effect in RPM 4.16
already where the Berkeley DB backend was swapped for sqlite, but don't
risk breaking stuff here and just keep the call in place for those older
RPM versions.

More info here:

[1] https://github.com/rpm-software-management/rpm/commit/
    baeb71141744bb0db6bd72c580bfaf929b7b4fb2
[2] rpm-software-management/rpm#1994
[3] rpm-software-management/rpm#1992
@dmnks
Copy link
Contributor Author

dmnks commented May 22, 2023

I've updated the PR with a backward compatible version of the same, hopefully this is now better.

Whether or not there's a regression in terms of 66b896e with RPM 4.18 I'm not sure but if so, that has to be fixed separately. This PR is just to make rpmconf work with RPM 4.19 the same way as it did with 4.18. IOW, to adapt rpmconf to the deprecation in 4.18.

@xsuchy
Copy link
Owner

xsuchy commented Jul 7, 2023

Closing in favour of #53

@xsuchy xsuchy closed this Jul 7, 2023
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.

None yet

2 participants