rocksdb/6.29.5: add patch to ignore thirdparty.inc #23090
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
This comment has been minimized.
This comment has been minimized.
|
My local compilation logs after patch: Abbreviated logs before patching which error out: Does this look ok to you @thejohnfreeman? :) |
|
@RubenRBS yes, I think that's right. |
conan-center-bot
added
Failed
Unexpected Error
and removed
Service Under Maintenance
labels
This comment has been minimized.
This comment has been minimized.
|
Why can't I see the error? |
|
@thejohnfreeman That's was an internal error in the CI, but will be restarted asap. Sorry the inconvenient. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Conan v1 pipeline ✔️All green in build 5 (
Conan v2 pipeline ✔️
All green in build 7 ( |
This was referenced Mar 20, 2024
valgur
suggested changes
Mar 24, 2024
There was a problem hiding this comment.
You can get away with a save(self, os.path.join(self.source_folder, "thirdparty.inc", "") in _patch_sources(), which is more stable across versions and easier to maintain.
Regarding WITH_GFLAGS, there's no need to remove it, probably. However, the recipe needs a tc.cache_variables["CMAKE_POLICY_DEFAULT_CMP0077"] = "NEW" for the current tc.variables to work correctly for CMake option()-s.
There was a problem hiding this comment.
That change won't fix the problem. The branch that calls find_package() for each optional dependency will still be skipped because MSVC is truthy. My patch effectively removes the other branch and sends all builds through the path that calls find_package().
I remove this call to option(WITH_GFLAGS) because it is superceded by a later call.
ghost
mentioned this pull request
|
This patch is still needed, and also needed for newer versions as well. How should we proceed with getting this in and working in general? |
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.
rocksdb/6.29.5
RocksDB has special instructions for compiling on Windows that asks builders to edit the special file
thirdparty.incto set variables pointing to all of the optional dependencies they want to enable. The mainCMakeLists.txtfollows the best practice of callingfind_package()and linking to imported targets, but only for non-MSVC generators.Thus, the latest RocksDB recipe in Conan Center builds with MSVC, but does not correctly link to any optional dependencies that are enabled.
thirdparty.incis included with nonsense paths and the build fails when it cannot find third-party headers.The old RocksDB recipe in Conan Center used a CMake subproject model. It had its own
CMakeLists.txtthat first calledconan_basic_setup()before callingadd_subdirectory()for the project. I don't know how, but it worked.This patch removes the branch on MSVC that includes
thirdparty.incinstead of callingfind_package(), but only for version 6.29.5 (the one I need for now).