libx264: conan v2, and to make it work on Windows msys (build for msvc) #15353
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
|
Can I please get some guidance on this PR ? |
1 task
|
I've done some more, but I really don't know what to do with the compiler_wrapper and other such things that other recipes seem to perform. |
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 7b7a9bblibx264/20191217 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 26c59d8libx264/20191217libx264/cci.20220602libx264/20190605 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
StellaSmith
suggested changes
Feb 10, 2023
Thanks to StellaSmith! Co-authored-by: Stella Smith <40411082+StellaSmith@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
SpaceIm
suggested changes
Feb 21, 2023
|
Thanks @SpaceIm, I've been buried in other things recently, I'll get back to this soon. |
Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
|
@paulharris Could you close/open this PR to see whether it passes CI? libx264 is a dependency of ffmpeg and it would be nice to have a conan v2 compatible recipe for this one ;) |
SpaceIm
reviewed
Mar 28, 2023
recipes/libx264/all/conanfile.py
Outdated
Comment on lines
85
to
90
| if self.options.shared: | ||
| args.append("--enable-shared") | ||
| args["--enable-shared"] = "" | ||
| args["--disable-static"] = None # --disable-static is not understood | ||
| else: | ||
| args.append("--enable-static") | ||
| args["--enable-static"] = "" | ||
| args["--disable-shared"] = None # --disable-shared is not understood |
There was a problem hiding this comment.
--disable-static can be moved outside this branching I think, otherwise it' a little bit confusing I think.
SpaceIm
reviewed
Mar 28, 2023
recipes/libx264/all/conanfile.py
Outdated
Comment on lines
119
to
120
| env.define("AS", os.environ["CC"]) | ||
| ndk_root = unix_path(self, os.environ["NDK_ROOT"]) |
There was a problem hiding this comment.
CC should come from tools.build:compiler_executables config and fallback eventually to CC from VirtualBuildEnv, not os.environ
Same for NDK_ROOT, I think there is tools.android:ndk_path config for this one.
There was a problem hiding this comment.
I gave it a try but I don't build Android so hard to test.
I referred to git log -p for hints on usage patterns.
Conan v1 pipeline ✔️All green in build 17 (
Conan v2 pipeline (informative, not required for merge) ✔️
All green in build 18 ( |
SpaceIm
approved these changes
Mar 29, 2023
|
@prince-chrismc @uilianries could you review please? This PR is mandatory to get a chance to pass ffmpeg builds in v2 pipeline (which is mandatory for opencv 4.x with its current default options). |
| @property | ||
| def _source_subfolder(self): | ||
| return "source_subfolder" | ||
| # otherwise build fails with: ln: failed to create symbolic link './Makefile' -> '../../../../../../../../../../../../../j/w/prod/buildsinglereference@2/.conan/data/libx264/cci.20220602/_/_/build/622692a7dbc145becf87f01b017e2a0d93cc644e/src/Makefile': File name too long |
There was a problem hiding this comment.
Thanks for adding comments !!!
prince-chrismc
approved these changes
Mar 29, 2023
0xFireWolf
pushed a commit
to 0xFireWolf/conan-center-index
that referenced
this pull request
Apr 2, 2023
…ys (build for msvc) * First attempt to upgrade for conan v2 compatibility * Fixup tests, try and fix compiler_wrapper etc * Fix up linter * Fixup configure --prefix * Use AutotoolsToolchain.update_configure_args() * Attempt to fix File-name-too-long error on Windows * Avoid one bit of the recipe for conan v2 * Fix typo * Fix for apple rpaths * Apply suggestions from code review Thanks to StellaSmith! Co-authored-by: Stella Smith <40411082+StellaSmith@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com> * Remove unused vars: build_canonical_name host_canonical_name * Fixes as suggested by SpaceIM --------- Co-authored-by: Stella Smith <40411082+StellaSmith@users.noreply.github.com> Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
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.
I started upgrading this recipe, but have got stuck on the parts related to MacOS and nasm...
example, the only example I could see to replace apple_deployment_target_flag() was from the Boost recipe upgrade, and it looks different.
Could I please get some assistance? Or perhaps someone else lead this PR?
My goal is to allow this recipe to build nicely for MSVC from within my msys shell.