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

ZBar 0.10 update #2029

Closed
wants to merge 6 commits into from
Closed

ZBar 0.10 update #2029

wants to merge 6 commits into from

Conversation

ericriff
Copy link
Contributor

Specify library name and version: ZBar/0.10

The main change introduced by this PR is a patch that updates config.guess and config.sub since the ones present in the source code are from 2009 and do not support (among others) aarch64.
There is also some cleanup in the test package and I changed the name of the findPkg.cmake to ZBar. Online you can find it as findZBAR.cmake or findZBar.cmake.

  • 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.

ericriff added 2 commits June 24, 2020 11:58
The original config.guess and config.sub are outdated and do not support
crosscompilation to aarch64.
The patch just replace those files with the ones found here
http://git.savannah.gnu.org/cgit/config.git/tree/
@uilianries
Copy link
Member

5k lines of patch?! This not a patch, it's an entire new project 😅 What's the source of that patch?

@conan-center-bot
Copy link
Collaborator

All green in build 1 (a83f03be0c75a41b345808ec8b4cb84d69cff199)! 😊

@ericriff
Copy link
Contributor Author

I believe I left the link to the source in the commit message

@madebr
Copy link
Contributor

madebr commented Jun 24, 2020

Isn't this the result of a more recent autoconf?

@ericriff
Copy link
Contributor Author

ericriff commented Jun 24, 2020

I'm not really sure. I googled the error I was having and I saw that it is pretty common to get this files outdated and they have to be manually updated using the sources I used.

@madebr
Copy link
Contributor

madebr commented Jun 24, 2020

@ericriff
Try the following:

  1. Remove your gigantic patch
  2. Add libtool/2.4.6 to the build requirements
  3. In build, after eventual other patches:
with tools.chdir(self._source_subfolder):
    self.run("autoreconf -fiv", run_environment=tools.os_info.is_windows)

@ericriff
Copy link
Contributor Author

ericriff commented Jun 24, 2020

I did a quick test and I've got a bunch of errors

configure.ac:109: warning: macro 'AM_ICONV' not found in library
autoreconf: running: /home/eric/.conan/data/autoconf/2.69/_/_/package/44fcf6b9a7fb86b2586303e3db40189d3b511830/bin/autoconf --force
configure.ac:1: error: possibly undefined macro: dnl
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
configure.ac:109: error: possibly undefined macro: AM_ICONV
configure.ac:131: error: possibly undefined macro: AC_MSG_FAILURE
autoreconf: /home/eric/.conan/data/autoconf/2.69/_/_/package/44fcf6b9a7fb86b2586303e3db40189d3b511830/bin/autoconf failed with exit status: 1

Using my system tools instead of build requirements I also get errors

autoreconf: running: automake --add-missing --copy --force-missing
configure.ac:10: installing 'config/compile'
configure.ac:6: installing 'config/missing'
automake: warnings are treated as errors
gtk/Makefile.am.inc:13: warning: '%'-style pattern rules are a GNU make extension
Makefile.am:32:   'gtk/Makefile.am.inc' included from here
gtk/Makefile.am.inc:17: warning: '%'-style pattern rules are a GNU make extension
Makefile.am:32:   'gtk/Makefile.am.inc' included from here
pygtk/Makefile.am.inc:20: warning: '%'-style pattern rules are a GNU make extension
Makefile.am:35:   'pygtk/Makefile.am.inc' included from here
qt/Makefile.am.inc:11: warning: '%'-style pattern rules are a GNU make extension
Makefile.am:39:   'qt/Makefile.am.inc' included from here
qt/Makefile.am.inc:14: warning: '%'-style pattern rules are a GNU make extension
Makefile.am:39:   'qt/Makefile.am.inc' included from here
test/Makefile.am.inc:60: warning: '%'-style pattern rules are a GNU make extension
Makefile.am:45:   'test/Makefile.am.inc' included from here
Makefile.am:70: warning: '%'-style pattern rules are a GNU make extension
Makefile.am:73: warning: '%'-style pattern rules are a GNU make extension
/usr/share/automake-1.15/am/ltlibrary.am: warning: 'gtk/libzbargtk.la': linking libtool libraries using a non-POSIX
/usr/share/automake-1.15/am/ltlibrary.am: archiver requires 'AM_PROG_AR' in 'configure.ac'
Makefile.am:32:   'gtk/Makefile.am.inc' included from here
gtk/Makefile.am.inc:1:   while processing Libtool library 'gtk/libzbargtk.la'
/usr/share/automake-1.15/am/ltlibrary.am: warning: 'plugin/libzbarplugin.la': linking libtool libraries using a non-POSIX
/usr/share/automake-1.15/am/ltlibrary.am: archiver requires 'AM_PROG_AR' in 'configure.ac'
Makefile.am:43:   'plugin/Makefile.am.inc' included from here
plugin/Makefile.am.inc:1:   while processing Libtool library 'plugin/libzbarplugin.la'
/usr/share/automake-1.15/am/ltlibrary.am: warning: 'pygtk/zbarpygtk.la': linking libtool libraries using a non-POSIX
/usr/share/automake-1.15/am/ltlibrary.am: archiver requires 'AM_PROG_AR' in 'configure.ac'
Makefile.am:35:   'pygtk/Makefile.am.inc' included from here
pygtk/Makefile.am.inc:1:   while processing Libtool library 'pygtk/zbarpygtk.la'
/usr/share/automake-1.15/am/ltlibrary.am: warning: 'python/zbar.la': linking libtool libraries using a non-POSIX
/usr/share/automake-1.15/am/ltlibrary.am: archiver requires 'AM_PROG_AR' in 'configure.ac'
Makefile.am:29:   'python/Makefile.am.inc' included from here
python/Makefile.am.inc:1:   while processing Libtool library 'python/zbar.la'
/usr/share/automake-1.15/am/ltlibrary.am: warning: 'qt/libzbarqt.la': linking libtool libraries using a non-POSIX
/usr/share/automake-1.15/am/ltlibrary.am: archiver requires 'AM_PROG_AR' in 'configure.ac'
Makefile.am:39:   'qt/Makefile.am.inc' included from here
qt/Makefile.am.inc:1:   while processing Libtool library 'qt/libzbarqt.la'
/usr/share/automake-1.15/am/ltlibrary.am: warning: 'zbar/libzbar.la': linking libtool libraries using a non-POSIX
/usr/share/automake-1.15/am/ltlibrary.am: archiver requires 'AM_PROG_AR' in 'configure.ac'
Makefile.am:8:   while processing Libtool library 'zbar/libzbar.la'
Makefile.am: installing 'config/depcomp'
autoreconf: automake failed with exit status: 1

@madebr
Copy link
Contributor

madebr commented Jun 24, 2020

I'll look into it.

@madebr
Copy link
Contributor

madebr commented Jun 25, 2020

@ericriff
Try applying this patch, using your system libtool.

--- configure.ac
+++ configure.ac
@@ -3,10 +3,11 @@
 AC_INIT([zbar], [0.10], [[email protected]])
 AC_CONFIG_AUX_DIR(config)
 AC_CONFIG_MACRO_DIR(config)
-AM_INIT_AUTOMAKE([1.10 -Wall -Werror foreign subdir-objects std-options dist-bzip2])
+AM_INIT_AUTOMAKE([1.10 -Wall foreign subdir-objects std-options dist-bzip2])
 AC_CONFIG_HEADERS([include/config.h])
 AC_CONFIG_SRCDIR(zbar/scanner.c)
 LT_PREREQ([2.2])
+AM_PROG_AR
 LT_INIT([dlopen win32-dll])
 LT_LANG([Windows Resource])

@madebr
Copy link
Contributor

madebr commented Jun 25, 2020

For using conan's automake, I've opened #2038 to let recipes add include paths for m4 files.
zbar requires the function AM_ICONV, which is provided by iconv.m4.
This is available through coreutils or gettext.

But I'm unsure how to add these to the recipe because they look overkill for reconfiguration only.

Using the patch in my previous comment, the automake package created by #2038, and adding the following lines to build(),
reconfiguration succeeds on my computer ™️

        with tools.chdir(self._source_subfolder):
            with tools.environment_append({"AUTOMAKE_CONAN_INCLUDES": ["/usr/share/aclocal"]}):
                self.run("autoreconf -fiv")

@ericriff
Copy link
Contributor Author

OK, I'll try tomorrow.
I'm not keen to use conan's libtool since it breaks crossbuilding for conan versions older than 1.24.
If you try to run something like

conan create recipes/zbar/all zbar/0.10@user/channel -pr armv7

Conan tries to pull libtool for armv7 instead of x86 and the build fails. If you use the newer approach with a build and a host profile this works as intended.
Using system's autotools to run autoreconf implies a bunch of system requirements that the user must have or the build will fail.
The gigantic patch seemed to be the the path of least resistance.

@Croydon
Copy link
Contributor

Croydon commented Jun 25, 2020

The gigantic patch seemed to be the the path of least resistance.

I disagree. A patch with 5k lines of code is unacceptable complex for package maintainers.

@ericriff
Copy link
Contributor Author

The gigantic patch seemed to be the the path of least resistance.

I disagree. A patch with 5k lines of code is unacceptable complex for package maintainers.

I also considered putting those files in export_sources and then overriding the original ones with those. Maybe that's more maintainer friendly?

@madebr
Copy link
Contributor

madebr commented Jun 25, 2020

Ideal would be to have a release. Is the project (temporarilly) dead?
Ideally 2nd would be to autoreconf.
Ideally 3rd would be to extract the armv8 specific portions, and apply those in a patch.

@madebr
Copy link
Contributor

madebr commented Jun 25, 2020

@ericriff
Copy link
Contributor Author

I believe the project is dead. There is also a mirror in github. On that one the latest release is also 0.10 but they continued developing for android and iOS. But it is weird because if you compare the sources of github and the official ones (sourceforge), on the same release (0.10) they're not the same. Specifically, configure is missing on the github clone, I'm not sure about the rest of the files. When had this conversation when we merged the original recipe for zbar and we decided to stick with the official sources.

@ericriff
Copy link
Contributor Author

@ericriff
Try applying this patch, using your system libtool.

--- configure.ac
+++ configure.ac
@@ -3,10 +3,11 @@
 AC_INIT([zbar], [0.10], [[email protected]])
 AC_CONFIG_AUX_DIR(config)
 AC_CONFIG_MACRO_DIR(config)
-AM_INIT_AUTOMAKE([1.10 -Wall -Werror foreign subdir-objects std-options dist-bzip2])
+AM_INIT_AUTOMAKE([1.10 -Wall foreign subdir-objects std-options dist-bzip2])
 AC_CONFIG_HEADERS([include/config.h])
 AC_CONFIG_SRCDIR(zbar/scanner.c)
 LT_PREREQ([2.2])
+AM_PROG_AR
 LT_INIT([dlopen win32-dll])
 LT_LANG([Windows Resource])

This patch works using libtool from system. I'm able to crosscompile for aarch64 without the gigapatch.

@madebr
Copy link
Contributor

madebr commented Jun 27, 2020

I believe the project is dead. There is also a mirror in github. On that one the latest release is also 0.10 but they continued developing for android and iOS.

Check this list: https://git.linuxtv.org/zbar.git/refs/
It contains both the original, the iphone versions and new more recent tags.

But it is weird because if you compare the sources of github and the official ones (sourceforge), on the same release (0.10) they're not the same. Specifically, configure is missing on the github clone,

It looks like the tagged commits are not the releases. The releases are probably the commits + running autoreconf on the repo.

I'm not sure about the rest of the files. When had this conversation when we merged the original recipe for zbar and we decided to stick with the official sources.

Nevertheless, we will need to run autoreconf on the sources, irrespective it is a release or a tagged release.

@ericriff
Copy link
Contributor Author

OK so do you want to switch to this unofficial fork?

@madebr
Copy link
Contributor

madebr commented Jun 29, 2020

This is not up to me.
Since you're using zbar apparently, does the fork fix any issues?

@ericriff
Copy link
Contributor Author

The company where I work has a internal mirror of conan-index and we're sticking with the latest official sources there, at least for now.
I don't have a preference for which sources to use here.

@madebr
Copy link
Contributor

madebr commented Jul 6, 2020

@ericriff
Copy link
Contributor Author

ericriff commented Jul 6, 2020

@madebr
Copy link
Contributor

madebr commented Jul 6, 2020

Oops!
See https://src.fedoraproject.org/rpms/zbar/blob/master/f/zbar.spec#_2
They are building 0.23 from linuxtv

@ericriff
Copy link
Contributor Author

ericriff commented Jul 6, 2020

I'll move to that sources then. That was enough to tilt the balance haha
Thanks for the research and help!

@madebr
Copy link
Contributor

madebr commented Jul 6, 2020

Btw, it looks like fedora is adding gettext as a build requirement.
Probably for the same reason as #2029 (comment)
Probably something to look into.

* Bump version to 0.23.1
* Ditch gigantic patch and use autoreconf instead
* Update test_package. zbar_version() takes 3 arguments now.
* Use gettext and libtool as build_requirements
@ericriff
Copy link
Contributor Author

ericriff commented Jul 6, 2020

Btw, it looks like fedora is adding gettext as a build requirement.
Probably for the same reason as #2029 (comment)
Probably something to look into.

The build fails without it now so we definitely need it. Regardless the build keeps failing on my PC and complains about a missing file in gettext/package/share which is being manually deleted in gettext's recipe. I've launched a build in the CI just in case.
The build succeeds locally with my system tools (libtool 2.4.6 + gettext 0.19.7)
And we no longer need the patch for autoconf.ac, at least with system libtool.

EDIT: In the CI fails with the same error:

----Running------
> autoreconf -fiv
-----------------
autoreconf: Entering directory `.'
autoreconf: running: autopoint --force
autopoint: using AM_GNU_GETTEXT_REQUIRE_VERSION instead of AM_GNU_GETTEXT_VERSION
/home/conan/workspace/onan-center-pull-request_PR-2029/2/pr_2029_2_0_0/.conan/data/gettext/0.20.1/_/_/package/60bd5a6fe61e9f3df0f6a890cd66bc37c573a6de/bin/autopoint: 494: /home/conan/workspace/onan-center-pull-request_PR-2029/2/pr_2029_2_0_0/.conan/data/gettext/0.20.1/_/_/package/60bd5a6fe61e9f3df0f6a890cd66bc37c573a6de/bin/autopoint: cannot open /home/conan/workspace/onan-center-pull-request_PR-2029/2/pr_2029_2_0_0/.conan/data/gettext/0.20.1/_/_/package/60bd5a6fe61e9f3df0f6a890cd66bc37c573a6de/share/gettext/archive.dir.tar.xz: No such file
tar: This does not look like a tar archive
tar: gettext-0.20: Not found in archive
tar: Exiting with failure status due to previous errors
autopoint: *** infrastructure files for version 0.20 not found; this is autopoint from GNU gettext-tools 0.20.1
autopoint: *** Stop.

@conan-center-bot
Copy link
Collaborator

Some configurations of 'zbar/0.23.1' failed in build 2 (746a18cd4fc6ac0dd784be7580f2b66c3e52cc36):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'zbar/0.23.1' failed in build 3 (cd9f4484a1b6eba88ceb4b3214f1468bfc1ac5d2):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'zbar/0.23.1' failed in build 4 (9105f85cc799a6004e265b5ec6fa5f7db5519b23):

@ericriff
Copy link
Contributor Author

ericriff commented Jul 7, 2020

@madebr I've been doing some tests and:

  1. Using gettext as a build_requirement makes the build fail because is autopoint tries to extract the infrastructure files for version 0.20 from package/hash/share/gettext/archive.dir.tar.xz but the share folder has been manually deleted from gettext's recipe. See https://www.gnu.org/software/gettext/manual/html_node/autopoint-Invocation.html

  2. Adding the next statements makes the build pass using conan's libtool (as you said) but it depends on system files.

with tools.chdir(self._source_subfolder):
   with tools.environment_append({"AUTOMAKE_CONAN_INCLUDES": ["/usr/share/aclocal"]}):
      self.run("autoreconf -fiv")

This files are also present in the share folder of gettext.
Can we leave that folder (or at least a part of if) in the gettext package?

  1. Removing AM_GNU_GETTEXT_VERSION(0.20) and AM_GNU_GETTEXT_REQUIRE_VERSION(0.18) from configure.ac make the build pass with a message saying not using gettext, but I don't know the implications of that.

  2. I built gettext locally without removing the share folder and with that package the build succeeds. I'm not sure how to point AUTOMAKE_CONAN_INCLUDES to that package.

EDIT: This still results in undefined macros

aclocal = os.path.join(self.deps_cpp_info["gettext"].rootpath, "share", "aclocal")
with tools.environment_append({"AUTOMAKE_CONAN_INCLUDES": aclocal}):

configure.ac:1: error: possibly undefined macro: dnl
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
configure.ac:152: error: possibly undefined macro: AS_IF
configure.ac:175: error: possibly undefined macro: AC_MSG_FAILURE
configure.ac:392: error: possibly undefined macro: AC_MSG_NOTICE

@ericriff
Copy link
Contributor Author

ericriff commented Sep 21, 2020

I've completely forgotten about this PR since we stuck with version 0.10.
I managed to get it to build locally by adding gettext as a build requirement, but it doesn't work out of the box since it needs the following file that is being deleted on the gettext recipe (the whole share folder is being deleted):
gettextPackage/recipeHash/share/gettext/archive.dir.tar.xz
I modified the gettext recipe so it keeps that file and it works, but the hooks complain about it:
ERROR: [DEFAULT PACKAGE LAYOUT (KB-H013)] Unknown folder 'share' in the package (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H013)
What should I do in this situation?

@conan-center-bot
Copy link
Collaborator

Some configurations of 'zbar/0.23.1' failed in build 5 (a93101c0b24dd4423b2fb24e30966359d8864dcd):

@madebr
Copy link
Contributor

madebr commented Sep 25, 2020

What should I do in this situation?

This is what I would do:
First, I would fix the gettext recipe and change the installation directory of the share folder (=datarootdir) to bin/share.
Remove the other superfluous files from the bin/share folder.
Eventually add some self.user_info variables so consumers know the location of certain files.
e.g. self.user_info.GETTEXT_TAR = os.path.join(self.package_folder, "bin", "share", "gettext", "archive.dir.tar.xz")
I'm not sure about the naming of the variables.
Then I think this information can be used in zbar.

@stale
Copy link

stale bot commented Dec 1, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 1, 2020
@stale
Copy link

stale bot commented Dec 31, 2020

This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions.

@stale stale bot closed this Dec 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants