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.

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

freeimage/3.18.0: fix build error using msvc 193 with C++20 #22942

Conversation

elvisdukaj
Copy link
Contributor

Trying to build freeimage using MSVC 193 with C++20 results in a buid error. The issue was reported #18921.

I am creating a MR for this issue. I submitted also a MR upstream: issue and MR.


@CLAassistant
Copy link

CLAassistant commented Mar 1, 2024

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@elvisdukaj elvisdukaj force-pushed the fix/freeimage_msvc_cpp_20_build_error branch from 6fadd20 to 8ea0de0 Compare March 1, 2024 10:39
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@@ -18,3 +18,4 @@ patches:
- patch_file: "patches/012_use-functions-to-override-libtiff-warning-error-handlers.patch"
- patch_file: "patches/013_use-typedef-as-already-declared.patch"
- patch_file: "patches/014_no_auto_ptr.patch"
- patch_file: "patches/015_fix_msvc_cpp_20.patch"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- patch_file: "patches/015_fix_msvc_cpp_20.patch"
- patch_file: "patches/015_fix_msvc_cpp_20.patch"
patch_type: "official"
patch_source: "https://github.com/WinMerge/freeimage/pull/20"
patch_description: "Fix compilation with MSVC and C++20"

Please, always add information about the patch, so we track it.

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

Thank you for your quick response to this bug.

@uilianries
Copy link
Member

@elvisdukaj Could you please ask for access in the issue #4. So the CI can list you as authorized user.

@uilianries uilianries self-assigned this Mar 1, 2024
Copy link
Contributor

@jcar87 jcar87 left a comment

Choose a reason for hiding this comment

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

Hi @elvisdukaj - thank you for your contribution and for looking into this.

Please note that your proposed patch WinMerge/freeimage#20 was not submitted to the canonical sources of freeimage, but to an unofficial clone.

Please submit or contact the maintainers at https://freeimage.sourceforge.io/.

Please note that we will not regularly accept patches fixing problems that are in library code - this should be reported and dealt with upstream. If this library is not maintained anymore, we will not be providing an extended life support in Conan Center. Remember that you can always instruct Conan to install a compatible version.

@elvisdukaj
Copy link
Contributor Author

Hi @jcar87 thank you for your feedback!

Well if it’s not appropriate the patch in this context we can close this MR then. I’ll keep it only internally to my company! The project seems not maintained anymore indeed. I wasn’t aware that the GitHub wasn’t the official one 😞

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ❌

Sorry, the build is only launched for Access Request users. You can request access writing in this issue.

Conan v2 pipeline ❌

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

The v2 pipeline failed. Please, review the errors and note this is required for pull requests to be merged. In case this recipe is still not ported to Conan 2.x, please, ping @conan-io/barbarians on the PR and we will help you.

See details:

Sorry, the build is only launched for Access Request users. You can request access writing in this issue.

@jcar87
Copy link
Contributor

jcar87 commented Mar 4, 2024

Well if it’s not appropriate the patch in this context we can close this MR then. I’ll keep it only internally to my company! The project seems not maintained anymore indeed. I wasn’t aware that the GitHub wasn’t the official one 😞

Thanks so much @elvisdukaj and thanks for your efforts! Keeping this internal is the way to go in these cases.
I will close the PR. We have no way of validating the proposed changes: they don't run on our CI, and we are not familiar with the codebase of this library to ascertain that this is compatible and working. For libraries that are no longer maintained by their authors, we will consider them unmaintained and won't be maintaining them ourselves either.

Thanks!

@jcar87 jcar87 closed this Mar 4, 2024
@thbeu
Copy link
Contributor

thbeu commented Mar 4, 2024

As already mentioned in #18921 (comment), I do hope that there will be a v3.19.x at some time. See also danoli3/FreeImage#6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants