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.

Sign up for GitHub

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

Jump to bottom

msys2: Conan 2.0 compatibility tweaks #14686

Merged
merged 8 commits into from
Jan 23, 2023
Merged

msys2: Conan 2.0 compatibility tweaks #14686

merged 8 commits into from
Jan 23, 2023

Conversation

System-Arch
Copy link
Contributor

@System-Arch System-Arch commented Dec 11, 2022 •
edited
Loading

Specify library name and version: msys2/cci.latest

While this recipe was ported to use Conan 2.0 modules (see #12715), it did not appear to actually function with Conan 2.0-beta6 or develop2. This set of changes addresses the remaining compatibility issues that were encountered as described in #14667

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

Hooks produced the following warnings for commit cb4c332
msys2/cci.latest 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\msys64\usr\lib\perl5\core_perl\auto\Win32\Win32.dll' links to system library 'netapi32' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\msys64\usr\lib\perl5\core_perl\auto\Win32\Win32.dll' links to system library 'version' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\msys64\usr\lib\perl5\core_perl\auto\Win32\Win32.dll' links to system library 'userenv' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\msys64\usr\lib\perl5\core_perl\auto\Win32\Win32.dll' links to system library 'winhttp' but it is not in cpp_info.system_libs.

@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

Hooks produced the following warnings for commit 66de1e8
msys2/cci.latest 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\bin\msys64\usr\lib\perl5\core_perl\auto\Win32\Win32.dll' links to system library 'userenv' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\bin\msys64\usr\lib\perl5\core_perl\auto\Win32\Win32.dll' links to system library 'netapi32' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\bin\msys64\usr\lib\perl5\core_perl\auto\Win32\Win32.dll' links to system library 'winhttp' but it is not in cpp_info.system_libs.
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] Library '.\bin\msys64\usr\lib\perl5\core_perl\auto\Win32\Win32.dll' links to system library 'version' but it is not in cpp_info.system_libs.

@System-Arch
Copy link
Contributor Author

Hi @prince-chrismc.
I think msys2 might be a good candidate to add to #12888. With these changes, it should now build with Conan 2.0. Thanks

@github-actions github-actions bot mentioned this pull request Dec 12, 2022
Open
@jwillikers
Copy link
Contributor

@System-Arch Why the no_kill option? I take it that's unrelated to Conan 2.0 support?

@System-Arch
Copy link
Contributor Author

@System-Arch Why the no_kill option? I take it that's unrelated to Conan 2.0 support?

Hi @jwillikers,

Correct. It turns out that my Conan 2.0 build automation uses msys2 (not from a Conan package) and the kill, which is really unnecessary in the context of building a Conan package, was killing the automation in the middle of the build process.

@SSE4
Copy link
Contributor

SSE4 commented Dec 13, 2022

that's hard to manage automation of MSYS build and keep clean/consistent state after failed builds, as it leaves running processes after the failures.
also, it's hard to manage parallel builds on the same machine, and it requires a global lock to avoid these MSYS processes messing up with each other.
we spent a lot of time to make it build reliably on CI.

@uilianries
Copy link
Member

uilianries commented Dec 13, 2022 •
edited
Loading

I agree with @SSE4 . Also, I don't think managing process life is a task for Conan client, it should be part from the system itself. I don't think is a good idea adding that option.

@System-Arch
Copy link
Contributor Author

I agree with @SSE4 . Also, I don't think managing process life is a task for Conan client, it should be part from the system itself. I don't think is a good idea adding that option.

Hi @SSE4, Hi @uilianries,
I'm sorry but I am confused by your comments. On the one hand you say "I don't think managing process life is a task for Conan client", which I agree with. On the other hand, the existing Conan client is doing just that by killing off arbitrary processes. Are you suggesting that I take out the no_kill option and leave the "kill" code as it was, or are you (hopefully) recommending removing the "kill" code thus eliminating the need for the "no_kill" option? Thanks for clarifying.

@memsharded memsharded mentioned this pull request Dec 22, 2022
Merged
4 tasks
@ghost ghost mentioned this pull request Dec 22, 2022
Closed
@ghost
Copy link

ghost commented Dec 22, 2022

I detected other pull requests that are modifying msys2/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

Hooks produced the following warnings for commit 656d687
msys2/cci.latest 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] objdump not found

@ericLemanissier ericLemanissier mentioned this pull request Jan 6, 2023
Closed
@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

Hooks produced the following warnings for commit 95c3980
msys2/cci.latest 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] objdump not found

@conan-center-bot

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

Hooks produced the following warnings for commit ecd8e49
msys2/cci.latest 
post_package(): WARN: [MISSING SYSTEM LIBS (KB-H043)] objdump not found

@prince-chrismc
Copy link
Contributor

You can remove objectdump, seems the hooks is broken

@System-Arch @SpaceIm
@spacelm says to ignore objdump hooks issue
5640aa4 
Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 8 (5640aa43df9a0d53163b6940bccb6c6aa0d3d703):

  • msys2/cci.latest@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit 47281b1 into conan-io:master Jan 23, 2023
SpaceIm
SpaceIm reviewed Jan 23, 2023
View reviewed changes
recipes/msys2/all/conanfile.py
}
default_options = {
"exclude_files": "*/link.exe",
"packages": "base-devel,binutils,gcc",
"additional_packages": None,
"no_kill": False,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this option should be removed from package id

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! I for this was not resolved 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this option should be removed from package id

Hi @SpaceIm,

Good catch. Thanks

SpaceIm
SpaceIm reviewed Jan 23, 2023
View reviewed changes
recipes/msys2/all/conanfile.py
Comment on lines +66 to +69
def configure(self):
self.settings.rm_safe("compiler.libcxx")
self.settings.rm_safe("compiler.cppstd")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's useless and confusing. There is no compiler in settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @SpaceIm,
I guess I am the one who is confused. Please see conan-io/hooks#471

SpaceIm
SpaceIm reviewed Jan 23, 2023
View reviewed changes
recipes/msys2/all/conanfile.py

required_conan_version = ">=1.47.0"

required_conan_version = ">=1.55.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

min conan version is 1.53.0 with these unecessary rm_safe, and without it's 1.47.0

@SpaceIm SpaceIm mentioned this pull request Jan 23, 2023
Merged
3 tasks
StellaSmith pushed a commit to StellaSmith/conan-center-index that referenced this pull request Feb 2, 2023
@System-Arch @SpaceIm @StellaSmith
(conan-io#14686) msys2: Conan 2.0 compatibility tweaks
fb3f51b 
* msys2: Conan 2.0 compatibility tweaks

* Remove C++ settings in configure() method

* Revamped packaging per #KB-H013: "DEFAULT PACKAGE LAYOUT"

* Drop Python 2 support

* Bumped required_conan_version to clear "failed" label

* Added objdump to system_libs

* @spacelm says to ignore objdump hooks issue

Co-authored-by: SpaceIm <30052553+SpaceIm@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
Reviewers

@SpaceIm SpaceIm SpaceIm left review comments

@prince-chrismc prince-chrismc prince-chrismc approved these changes

@jwillikers jwillikers jwillikers approved these changes

@jcar87 jcar87 Awaiting requested review from jcar87

@uilianries uilianries Awaiting requested review from uilianries

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@System-Arch @conan-center-bot @jwillikers @SSE4 @uilianries @prince-chrismc @SpaceIm