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

davfs2: fix inconsistency with HAVE_ICONV and HAVE_ICONV_H #2149

Closed
wants to merge 1 commit into from

Conversation

commodo
Copy link
Contributor

@commodo commodo commented Dec 19, 2015

Fixes: #1327

Seems that HAVE_ICONV was 1 and HAVE_ICONV_H undefined.
Resulting in build errors like this:
webdav.c: In function 'dav_conv_from_utf_8':
webdav.c:587:9: error: 'from_utf_8' undeclared (first use in this function)
if (from_utf_8)

Also, we're adding $(ICONV_DEPENDS) to the DEPENDS of this package.
Just adding the DEPENDS does not fix.
Seems the patch is still necessary.

Signed-off-by: Alexandru Ardelean ardeleanalex@gmail.com

Seems that HAVE_ICONV was 1 and HAVE_ICONV_H undefined.
Resulting in build errors like this:
  webdav.c: In function 'dav_conv_from_utf_8':
  webdav.c:587:9: error: 'from_utf_8' undeclared (first use in this function)
       if (from_utf_8)

Also, we're adding $(ICONV_DEPENDS) to the DEPENDS of this package.
Just adding the DEPENDS does not fix.
Seems the patch is still necessary.

Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
@hnyman
Copy link
Contributor

hnyman commented Dec 20, 2015

@fededim

@fededim
Copy link
Contributor

fededim commented Dec 22, 2015

test without iconv
Yesterday I have successfully built davfs2 on ramips arch with the latest trunk without and with libiconv-full (unbelievably at the first attempt). Can you please tell me with which arch/packages you are experiencing the problem ? I am not a linux expert but I do not think that changing the #ifdef clause from HAVE_ICONV_H to HAVE_ICONV in sources is a good idea since as far as I know the header files are checked by autoconf and defines are set according (https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/Generic-Headers.html). It's better to understand why HAVE_ICONV_H is not set and maybe patch only Configure.ac and not all source files, but I do not think that this will be the case either.

@commodo
Copy link
Contributor Author

commodo commented Dec 22, 2015

So, the build seems to fail on x86 & x86.64.
http://buildbot.openwrt.org:8010/broken_packages/x86.64/davfs2/compile.txt
The x86 broken packages page seems is not generated yet.
It may become later online.

I agree that my fix may just overlook a more deeper issue.

In any case, if my PR brings about a verification of what's really wrong with those 2 platforms then it's fine.
For me this was a simple fix that worked.

Sent from Samsung Mobile

-------- Original message --------
From: Federico Di Marco notifications@github.com
Date:22/12/2015 14:37 (GMT+02:00)
To: openwrt/packages packages@noreply.github.com
Cc: Alexandru Ardelean ardeleanalex@gmail.com
Subject: Re: [packages] davfs2: fix inconsistency with HAVE_ICONV and HAVE_ICONV_H (#2149)
Yesterday I have successfully built davfs2 on ramips arch with the latest trunk without and with libiconv-full (unbelievably at the first attempt). Can you please tell me with which arch/packages you are experiencing the problem ? I am not a linux expert but I do not think that changing the #ifdef clause from HAVE_ICONV_H to HAVE_ICONV in sources is a good idea since as far as I know the header files are checked by autoconf and defines are set according (https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/Generic-Headers.html). It's better to understand why HAVE_ICONV_H is not set and maybe patch only Configure.ac and not all source files, but I do not think that this will be the case either.


Reply to this email directly or view it on GitHub.

@fededim
Copy link
Contributor

fededim commented Dec 22, 2015

It seems that during my compilation configure skip some checks (for example iconv library), any idea why ?!? Moreover in your build log it writes a suspicious entry "checking for working iconv... guessing yes"...

p.s. obviously I re-downloaded the whole openwrt sources from github after nuking the directory and libiconv-full package is selected in menuconfig.

My log
checking for CFPreferencesCopyAppValue... no
checking for CFLocaleCopyCurrent... no
checking whether to use NLS... no
checking for neon library in /home/compile/openwrt/staging_dir/target-mipsel_1004kc+dsp_musl-1.1.11/usr... found
checking linking against neon... yes
configure: using neon library 0.30.0

Server
checking for CFPreferencesCopyAppValue... no
checking for CFLocaleCopyCurrent... no
checking for GNU gettext in libc... no
checking for iconv... yes
checking for working iconv... guessing yes
checking for GNU gettext in libintl... no
checking whether to use NLS... no
checking for neon library in /home/buildbot/slave/x86.64/build/staging_dir/target-x86_64_musl-1.1.11/usr... found
checking linking against neon... yes
configure: using neon library 0.30.0

@fededim
Copy link
Contributor

fededim commented Dec 22, 2015

I did not know about this, maybe it would be better to add this information to https://wiki.openwrt.org/doc/devel/packages, should someone might want to port any new package based on libiconv or libintl...in any case I would like why my configure behaves differently from the one in the build server

http://lists.en.qi-hardware.com/pipermail/discussion/2011-February/007092.html

@commodo
Copy link
Contributor Author

commodo commented Dec 22, 2015

From my side it's fair to add either a reference to that page or copy the contents to that page into the OpenWRT wiki.
I have no strong opinion nor much say in the matter (I guess).

I care more about this PR and whether it should go in as is, or a better solution is found :)

@fededim
Copy link
Contributor

fededim commented Jun 7, 2017

Sorry I forgot your old PR which was left unmerged (maintainers were waiting for my ok), I created a new one with your changes #4458.

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.

davfs2 - can use full language support - error build
3 participants