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

zxing-cpp: add version 1.3.0 #11210

Merged
merged 5 commits into from
Jun 17, 2022
Merged

zxing-cpp: add version 1.3.0 #11210

merged 5 commits into from
Jun 17, 2022

Conversation

toge
Copy link
Contributor

@toge toge commented Jun 15, 2022

Specify library name and version: zxing-cpp/*

  • support gcc >= 11 (adding additional patches)
  • another test_package.cpp for zing-cpp/1.3.0
  • add find_package(stb) in test_package
  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the conan-center hook activated.

@ghost ghost mentioned this pull request Jun 15, 2022
Closed
@ghost
Copy link

ghost commented Jun 15, 2022

I detected other pull requests that are modifying zxing-cpp/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
@github-actions github-actions bot mentioned this pull request Jun 15, 2022
Open
This was referenced Jun 15, 2022
Open
Closed
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@toge
Copy link
Contributor Author

toge commented Jun 16, 2022 

The system cannot execute the specified program.
ERROR: zxing-cpp/1.3.0 (test package): Error in test() method, line 24
	self.run(bin_path, run_environment=True)
	ConanException: Error 1 while executing bin\test_package

Oh.I can't find the way to solve this.
It will take some time to solve it as I currently do not have an environment to use Visual Studio C++.

@conan-center-bot
Copy link
Collaborator

All green in build 4 (36f26e08f731ba05dea8d4665aa63cdd9ec4b38d):

  • zxing-cpp/1.0.8@:
    All packages built successfully! (All logs)

  • zxing-cpp/1.3.0@:
    All packages built successfully! (All logs)

@toge
Copy link
Contributor Author

toge commented Jun 16, 2022

I would like to remove this condition from support because it is a puzzling phenomenon that occurs only under very limited conditions.

  • zxing-cpp/1.3.0
  • Visual Studio C++
  • MT
  • Debug

If anyone knows the cause of this problem, I would appreciate it if you could correct it.

@uilianries
Copy link
Member

@toge Maybe @madebr and @SpaceIm know about, they contributed to this recipe before.

@conan-center-bot conan-center-bot requested a review from SSE4 June 17, 2022 12:00
madebr
madebr reviewed Jun 17, 2022
View reviewed changes
recipes/zxing-cpp/all/conanfile.py
Comment on lines +70 to +72
# FIXME: This is a workaround for "The system cannot execute the specified program."
if tools.Version(self.version) == "1.3.0" and is_msvc_static_runtime(self) and self.settings.build_type == "Debug":
raise ConanInvalidConfiguration("{}/{} doesn't support MT + Debug.".format(self.name, self.version))
Copy link
Contributor

Choose a reason for hiding this comment

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

It is this combination wou want to exclude from the build matrix?
Can you reproduce the problem locally? Is it VS2017-only, or does it happen for all VC's?

The error you're trying to work around ("The system cannot execute the specified program.`) looks weird.
This log? #11210 (comment)

@prince-chrismc
Copy link
Contributor

Seems like an older issue #8373 (comment)

Lots of projects struggle with MT since so few use it. I think this is a great improvement!

@madebr
Copy link
Contributor

madebr commented Jun 17, 2022

So this actually means we're either using zxing-cpp/stb_image wrong in the test_package,
or there is a bug in one of these libraries?

@conan-center-bot conan-center-bot merged commit 537cb6b into conan-io:master Jun 17, 2022
hoxnox pushed a commit to hoxnox/conan-center-index that referenced this pull request Jun 30, 2022
@toge @hoxnox
(conan-io#11210) zxing-cpp: add version 1.3.0
0683a43 
* zxing-cpp: add version 1.3.0

* fix indent

* drop support gcc 5 in zxing-cpp/>=1.2

* add LINK_CPP_STATICALLY

* (trial) unsupport MTd in 1.3.0
@axxel
Copy link
Contributor

axxel commented Jul 11, 2022

I'm the maintainer of the zxing-cpp library and just noticed this packaging effort. I'd like to point out a few things I noticed:

  • to build the lib we require c++17 since v1.2.0 (minimum VS 2019 16.8 / gcc 7 / clang 5)
  • I just released v1.4.0 a couple of days ago
  • up until 1.4 a c++11 compiler was sufficient to use the library (on the client side)

Question: when would you consider removing support to package 1.0.8 (which is totally outdated by now)?

@SSE4 SSE4 mentioned this pull request Jul 11, 2022
Merged
4 tasks
@toge
Copy link
Contributor Author

toge commented Jul 11, 2022

@axxel
Thank you for your valuable comments.
I will reflect your comments in a separate PR with the addition of version 1.4.0. (#11614)

As for 1.0.8, in accordance with conan's version policyI intended to remove it after some time.
If you have any problems with it, please let me know why and I will remove it with a comment.

@axxel
Copy link
Contributor

axxel commented Jul 12, 2022

Very nice. I did not see the pending PR already adding 1.4. Looks good. Minor issue: the description seems to still claim it to be a "c++14" port.

@toge
Copy link
Contributor Author

toge commented Jul 14, 2022

@axxel
I fixed the description.
Thanks a lot for review it and developing a great library!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@madebr madebr madebr left review comments

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

@SSE4 SSE4 SSE4 approved these changes

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
@toge @conan-center-bot @uilianries @prince-chrismc @madebr @axxel @SSE4