-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
[apr] fix CMake config via patch from the stable branch. #38674
Conversation
* Use patch with changes from the stable branch to fix cmake config instead of fix-configcmake.patch file. * Take windows part of the portfile.cmake file from stable branch of apr. * Remove the fix-configcmake.patch file. * Change cmake namespace from `unofficial-apr` to just `apr`, patch is now taken from the stable branch of apr.
This reverts commit 430a1fa.
Local attempt to apply upstream-diff, but failed. vcpkg_download_distfile(PATCH_APR_1917612_FROM_1909837
URLS "https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/CMakeLists.txt?r1=1917612&r2=1909837&view=patch"
FILENAME "PATCH_APR_1917612_FROM_1909837.diff"
SHA512 4bd8943d224bf90ec215c3f09d3c1ca44ba732db245ca2eb65092a71d1ce3b7aa9b444654c155857a8bfd26adb1934af2be1563bd4317518ebdceab0f816709b
)
file(COPY_FILE "${PATCH_APR_1917612_FROM_1909837}" "${CURRENT_BUILDTREES_DIR}/patches/PATCH_APR_1917612_FROM_1909837.diff"}
vcpkg_replace_string("${CURRENT_BUILDTREES_DIR}/patches/PATCH_APR_1917612_FROM_1909837.diff"
"apr/apr/branches/1.7.x/CMakeLists.txt"
"CMakeLists.txt"
)
vcpkg_extract_source_archive(SOURCE_PATH
ARCHIVE "${ARCHIVE}"
PATCHES
"${CURRENT_BUILDTREES_DIR}/patches/PATCH_APR_1917612_FROM_1909837.diff"
unglue.patch
) @rinrab Could you help apply patches by this method? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patch diffs are basically the same, and can be viewed via compare the pull-diff
https://github.com/microsoft/vcpkg/blob/fa826f088dded4b387a503ee6303ced0acbc7363/ports/apr/apr-r1917051%2Br1917283%2Br1917612.patch
and upstream-diff
https://svn.apache.org/viewvc/apr/apr/branches/1.7.x/CMakeLists.txt?r1=1917612&r2=1909837&view=patch
Edit: Newer changes, test invalid
The port usage tests pass with the following triplets:
x64-windows
@WangWeiLin-MV I checked this problem and I found that the patch works as well. I did the following:
I also tried to do it via git: Maybe you tried to apply this patch to the stable or trunk branch. |
@rinrab Sorry for not clarify, the patches in this PR can be applied. My local attempt is using the patch downloaded from the link mentioned. If possible, we prefer patches by downloading upstream patches like this rather than add patches to the port. |
Patches are generated via the following command: ```powershell 1917051,1917065,1917283,1917612 | ForEach-Object { svn-diff https://svn.apache.org/repos/asf/apr/apr/branches/1.7.x -c $_ -raw | Out-File "apr-r$_.patch" } ```
* apr-r1917051.patch: Replace ENDIF() with SET(install_bin_pdb) in context. * apr-r1917283.patch: Remove changes in the CHANGES file because it doesn't affect on the result.
I don't think that this solution is good because:
For better clarity I separated that patch by each revision. |
Your attempt failed because the context in patch was wrong. When patch is applying, it looks at the lines before and after. However they were changed in the '1.7.x' branch before these changes. It's required to fix them like I did here: f23193d (In github webiu it looks so strange. I don't know why) |
I just checked, in my environment it does work in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rinrab Therefore, it should change the variable corresponding to feature private-headers
to APR_INSTALL_PRIVATE_H
. Moreover, FEATURE_OPTIONS
clearly pass the options and it is recorded in the features of vcpkg.json
.
I committed these suggestions in e0e3454, however I prefer to follow the APR's upstream version of the portfile. |
These are significant patches. Could we |
Those have been released as APR of version 1.7.5. I opened a pull-request at #40742 with bumping the version and adding some features from this pull-request. |
Some of the changes made in patch which fixes CMake config in the apr port were moved to the upstream. I added these changes the port via patch, made from upstream.
unofficial-apr
to justapr
, patch is now taken from the stable branch of apr.