-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
libdeflate: conan v2 support #13759
libdeflate: conan v2 support #13759
Conversation
This comment has been minimized.
This comment has been minimized.
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.
Please, remove .CMakeLists.txt.swp
Co-authored-by: Uilian Ries <uilianries@gmail.com>
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
LGTM
env.generate() | ||
|
||
if is_msvc(self) or self._is_clangcl: | ||
vc = VCVars(self) |
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.
Does VirtualBuildEnv
automatically handle VCVars?
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.
I don't think so.
https://docs.conan.io/en/latest/reference/conanfile/tools/microsoft.html?highlight=vcvars#msbuild
It only handles msbuild stuff. VirtualBuildEnv is more related to environment variables.
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.
How does building most packages work then, when I'm not in a Visual Studio Developer Command Prompt? I figured they would fail.
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.
The msbuild helper does that step 😉
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.
@jwillikers one of the reasons I want to upgrade these recipes is so I can build everything in my git-bash-SDK-msys terminal on windows. And so far, the new recipes seem to be doing it well.
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.
theres some good comments above
Co-authored-by: Jordan Williams <jordan@jwillikers.com>
This comment has been minimized.
This comment has been minimized.
recipes/libdeflate/all/conanfile.py
Outdated
with chdir(self, self.source_folder): | ||
# TODO: replace by autotools.install() once https://github.com/conan-io/conan/issues/12153 fixed | ||
# Note that PREFIX=/ also appears to be required on Linux. | ||
autotools.install(args=["PREFIX=/", f"DESTDIR={unix_path(self, self.package_folder)}"]) |
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.
@jwillikers @uilianries continuing from other comment,
I get messages like:
install -m755 libdeflate.so.0 /build/conandata/conan-data/libdeflate/1.14/ccitest/test1/package/0582ee0920dad9fef631620d3ef80b963208d3a1/usr/local/lib;
Note it installs to /usr/local/lib
and then, [HOOK - conan-center.py] post_package(): ERROR: [DEFAULT PACKAGE LAYOUT (KB-H013)] Unknown folder 'usr' in the package (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H013)
and CMake Error at build/generators/cmakedeps_macros.cmake:39 (message): Library 'deflate' not found in package. If 'deflate' is a system library, declare it with 'cpp_info.system_libs' property Call Stack (most recent call first):
I know autotools adds PREFIX=/ to configure calls, but it didn't seem to be done this time.
Weirdly, it works on Windows without PREFIX=/
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.
Oh wait, this is a Makefile project and not actually Autotools, right? AutotoolsToolchain passes --prefix
to the Autotools configure script. You wouldn't have that here so you'd have to use PREFIX
or whatever the Makefile might expect. You can probably drop the DESTDIR
then. I'm sorry for the confusion. We might need a dedicated generator for Makefile projects that doesn't require using things with Autotools
in the name.
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.
Ok, dropped and just use PREFIX
This comment has been minimized.
This comment has been minimized.
Conan v1 pipelineAll green in build 8 (
|
bad bad bad Again, migration of recipes relying on |
fix here: #14028 |
Finishing the v2 upgrade