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

Fix building zchunk code if zchunk is enabled #302

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

ppisar
Copy link
Contributor

@ppisar ppisar commented Mar 8, 2024

Commit 66c99da (Change header files to match a configured ABI regarding a zchunk support) accidentally removed a definition of WITH_ZCHUNK C preprocessor symbol. That symbol controls including zchunk support into the library's code.

As a result, librepo library was always missing a zchunk support and ci-dnf-stack "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)" always failed.

This fix returns back the macro into CFLAGS.

Commit 66c99da (Change header files
to match a configured ABI regarding a zchunk support) accidentally
removed a definition of WITH_ZCHUNK C preprocessor symbol. That
symbol controls including zchunk support into the library's code.

As a result, librepo library was always missing a zchunk support and
ci-dnf-stack "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)" always failed.

This fix returns back the macro into CFLAGS.
@ppisar
Copy link
Contributor Author

ppisar commented Mar 8, 2024

@kontura, you reviewed a pull request which introduced a regression. Could you review this pull request which fixes the regression?

The desired output is that when building librepo (as configured in ci-dnf-stack), a compiler should have -DWITH_ZCHUNK option. I suspect that this fix will have an effect on ci-dfn-stack after building nightlies with the fix. Not immediately when merging this pull request. At least that's what we observed on the breaking pull request.

@kontura
Copy link
Contributor

kontura commented Mar 8, 2024

It is odd that the specific CI test passed on the earlier PR.

@kontura kontura self-assigned this Mar 8, 2024
@kontura
Copy link
Contributor

kontura commented Mar 8, 2024

The desired output is that when building librepo (as configured in ci-dnf-stack), a compiler should have -DWITH_ZCHUNK option. I suspect that this fix will have an effect on ci-dfn-stack after building nightlies with the fix. Not immediately when merging this pull request. At least that's what we observed on the breaking pull request.

That would suggest we are not actually using the build from the PR for the CI run.
I will look into it.

@kontura kontura merged commit e2abd5d into rpm-software-management:master Mar 8, 2024
4 of 6 checks passed
@ppisar
Copy link
Contributor Author

ppisar commented Mar 8, 2024

I looked at here failed "DNF4 Integration Tests". It updates librepo from copr:copr.fedorainfracloud.org:rpmsoftwaremanagement:dnf-nightly repository (1.17.0-20240308004650.6.g66c99da.fc39) instead of upgrading to the one build in "DNF4 Copr Build" task.

@ppisar
Copy link
Contributor Author

ppisar commented Mar 8, 2024

the one build in "DNF4 Copr Build" task.

That one is 1.17.0-20240308125436.7.gcdc88a7.fc39.

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

Successfully merging this pull request may close these issues.

2 participants