Change header files to match a configured ABI regarding a zchunk support #299
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If librepo was built with -DWITH_ZCHUNK=ON but an application included <librepo/librepo.h> without defining -DWITH_ZCHUNK, struct LrDownloadTarget mismatched. The same problem happened vice versa. Following the correct ABI was a burden for librepo applications and a mistake there could lead to a memory corruption. This patch replaces the WITH_ZCHUNK macros in the public header files with a constant so that the installed header files do not depend on the macro and always match the installed library. This patch also adds a missing RPM dependency on zchunk-devel to librepo-devel if built with enabled zchunk. (Some header files include <zck.h>.) An implementation note: CMakeLists.txt appends the header search paths with both a top-level build directory as well as with a librepo subdirectory. The latter is for "util.h" inclusions from the C files, the former is for <librepo/util.h> inclusions from other librepo header files. Without the former a C preprocesor would use already installed <librepo/util.h>. We do not want to infect the being built library with old and possible nonmatching system header files. Fixes: rpm-software-management#298
|
Thank you! The failing tests are not related. |
|
After merging this pull request, "dnf/zchunk.feature:141 using mirror wihtout ranges supports and zchunk results in only two GET requests for primary (the first try is with range specified)" scenario from ci-dnf-stack started to fail. I'm investigating it. |
A fix proposed in PR #302. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If librepo was built with -DWITH_ZCHUNK=ON but an application included <librepo/librepo.h> without defining -DWITH_ZCHUNK, struct LrDownloadTarget mismatched. The same problem happened vice versa. Following the correct ABI was a burden for librepo applications and a mistake there could lead to a memory corruption.
This patch replaces the WITH_ZCHUNK macros in the public header files with a constant so that the installed header files do not depend on the macro and always match the installed library.
This patch also adds a missing RPM dependency on zchunk-devel to librepo-devel if built with enabled zchunk. (Some header files include <zck.h>.)
An implementation note: CMakeLists.txt appends the header search paths with both a top-level build directory as well as with a librepo subdirectory. The latter is for "util.h" inclusions from the C files, the former is for <librepo/util.h> inclusions from other librepo header files. Without the former a C preprocesor would use already installed <librepo/util.h>. We do not want to infect the being built library with old and possible nonmatching system header files.
Fixes: #298