Bug 202798 - [exp-run] Set iconv-related CMake variables in Uses/iconv.mk
Summary: [exp-run] Set iconv-related CMake variables in Uses/iconv.mk
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Raphael Kubo da Costa
URL:
Keywords:
Depends on:
Blocks: 202516
  Show dependency treegraph
 
Reported: 2015-08-31 20:49 UTC by Raphael Kubo da Costa
Modified: 2015-09-05 10:43 UTC (History)
2 users (show)

See Also:


Attachments
Proposed patch (19.83 KB, patch)
2015-09-02 14:22 UTC, Raphael Kubo da Costa
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Raphael Kubo da Costa freebsd_committer freebsd_triage 2015-08-31 20:49:20 UTC
This exp-run request came from bug 202516, the exp-run request for CMake 3.3.1.

Patch: https://people.freebsd.org/~rakuco/exp-runs/iconv.mk-cmake-vars-v1.patch

There were a lot of problems with iconv in that exp-run, and Tijl and I have been discussing how to address it. I would like to solve this issue independently and before continuing with the CMake 3.3 work. I've tested the patch above (modulo the change in converters/libiconv) with several ports that depend on iconv and CMake. Several of them could be simplified as this patch shows.

It would be good to check this on 10.1 (the most problematic FreeBSD release in terms of iconv support) as well as either 9.3 or HEAD.
Comment 1 Raphael Kubo da Costa freebsd_committer freebsd_triage 2015-08-31 20:52:17 UTC
Tijl: please take a look at the patch. I can also either attach it here or upload it to Phabricator for comments. I've tested CMake 3.3.1 with this patch and ports like libstreamanalyzer build fine on 10.1. I can provide a list of ports that I've been testing as well.
Comment 2 Tijl Coosemans freebsd_committer freebsd_triage 2015-09-01 08:13:01 UTC
The patch looks good to me except maybe this:

Index: textproc/ctpp2/Makefile
===================================================================
--- textproc/ctpp2/Makefile	(revision 395669)
+++ textproc/ctpp2/Makefile	(working copy)
@@ -36,15 +37,11 @@
 DISCARD_ILSEQ_CMAKE_OFF=	-DICONV_DISCARD_ILSEQ=OFF
 TRANSLITERATE_CMAKE_ON=	-DICONV_TRANSLITERATE=ON
 TRANSLITERATE_CMAKE_OFF=	-DICONV_TRANSLITERATE=OFF
+TRANSLITERATE_USES=	iconv:wchar_t

iconv:translit here?
Comment 3 Raphael Kubo da Costa freebsd_committer freebsd_triage 2015-09-01 08:40:06 UTC
Thanks for spotting the oversight. I've uploaded https://people.freebsd.org/~rakuco/exp-runs/iconv.mk-cmake-vars-v2.patch with the fix.

Antoine: it's fine if the exp-run has already started, as both versions of the patch have the same effect.
Comment 5 Raphael Kubo da Costa freebsd_committer freebsd_triage 2015-09-01 21:38:17 UTC
Thanks, I was about to bring that up too. They all seem to come from the removal of that patch from converters/libiconv. How about landing that one separately from the rest?
Comment 6 Antoine Brodin freebsd_committer freebsd_triage 2015-09-02 07:09:34 UTC
Exp-run results on 10.2 amd64:

http://package18.nyi.freebsd.org/build.html?mastername=102amd64-default-PR202798&build=2015-09-02_05h41m56s

2 new failures:

+ {"origin"=>"irc/weechat", "pkgname"=>"weechat-1.1.1_1", "phase"=>"package", "errortype"=>"???"}
+ {"origin"=>"irc/weechat-devel", "pkgname"=>"weechat-devel-20140213_5", "phase"=>"package", "errortype"=>"???"}

Failure logs:

http://package18.nyi.freebsd.org/data/102amd64-default-PR202798/2015-09-02_05h41m56s/logs/errors/weechat-1.1.1_1.log
http://package18.nyi.freebsd.org/data/102amd64-default-PR202798/2015-09-02_05h41m56s/logs/errors/weechat-devel-20140213_5.log
Comment 7 Antoine Brodin freebsd_committer freebsd_triage 2015-09-02 10:44:40 UTC
Exp-run results on 10.1 i386:

http://package22.nyi.freebsd.org/build.html?mastername=101i386-default-PR202798&build=2015-09-02_09h38m43s

0 new failure
Comment 8 Antoine Brodin freebsd_committer freebsd_triage 2015-09-02 11:33:18 UTC
Exp-run results on 9.3 i386:

http://package23.nyi.freebsd.org/build.html?mastername=93i386-default-PR202798&build=2015-09-02_10h43m42s

17 new failures:

+ {"origin"=>"audio/deadbeef", "pkgname"=>"deadbeef-0.6.2_5", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"comms/hidapi", "pkgname"=>"hidapi-0.8.0.r1_3", "phase"=>"configure", "errortype"=>"configure_error"}
+ {"origin"=>"deskutils/fbreader", "pkgname"=>"fbreader-0.99.6_1", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"deskutils/ljclive", "pkgname"=>"ljclive-0.4.10_1", "phase"=>"build", "errortype"=>"missing_header"}
+ {"origin"=>"deskutils/owncloudclient", "pkgname"=>"owncloudclient-1.8.4", "phase"=>"build", "errortype"=>"compiler_error"}
+ {"origin"=>"emulators/vmw", "pkgname"=>"vmw-060510", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"irc/scrollz", "pkgname"=>"scrollz-2.2.3_1", "phase"=>"build", "errortype"=>"clang"}
+ {"origin"=>"japanese/chasen-base", "pkgname"=>"ja-chasen-base-2.4.5_1", "phase"=>"configure", "errortype"=>"configure_error"}
+ {"origin"=>"japanese/uim-mozc", "pkgname"=>"ja-uim-mozc-2.17.2106.102", "phase"=>"build", "errortype"=>"missing_header"}
+ {"origin"=>"java/jboss71", "pkgname"=>"jboss71-7.1.3", "phase"=>"build", "errortype"=>"???"}
+ {"origin"=>"net/c3270", "pkgname"=>"c3270-3.3.14ga11_1", "phase"=>"configure", "errortype"=>"configure_error"}
+ {"origin"=>"www/htmlcxx", "pkgname"=>"htmlcxx-0.85_2", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"www/httrack", "pkgname"=>"httrack-3.48.21_1", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"www/p5-Gtk2-WebKit", "pkgname"=>"p5-Gtk2-WebKit-0.09_2", "phase"=>"build", "errortype"=>"???"}
+ {"origin"=>"www/xapian-omega", "pkgname"=>"xapian-omega-1.2.21", "phase"=>"configure", "errortype"=>"configure_error"}
+ {"origin"=>"x11/mrxvt-devel", "pkgname"=>"mrxvt-devel-0.5.4_10", "phase"=>"build", "errortype"=>"linker_error"}
+ {"origin"=>"x11/x3270", "pkgname"=>"x3270-3.3.15_2", "phase"=>"configure", "errortype"=>"configure_error"}

Failure logs:

http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/deadbeef-0.6.2_5.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/hidapi-0.8.0.r1_3.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/fbreader-0.99.6_1.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/ljclive-0.4.10_1.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/owncloudclient-1.8.4.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/vmw-060510.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/scrollz-2.2.3_1.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/ja-chasen-base-2.4.5_1.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/ja-uim-mozc-2.17.2106.102.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/jboss71-7.1.3.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/c3270-3.3.14ga11_1.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/htmlcxx-0.85_2.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/httrack-3.48.21_1.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/p5-Gtk2-WebKit-0.09_2.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/xapian-omega-1.2.21.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/mrxvt-devel-0.5.4_10.log 
http://package23.nyi.freebsd.org/data/93i386-default-PR202798/2015-09-02_10h43m42s/logs/errors/x3270-3.3.15_2.log
Comment 9 Tijl Coosemans freebsd_committer freebsd_triage 2015-09-02 12:35:11 UTC
(In reply to Raphael Kubo da Costa from comment #5)
I'll fix the FreeBSD 9 failures and open a separate bug for that.
Comment 10 Raphael Kubo da Costa freebsd_committer freebsd_triage 2015-09-02 12:49:52 UTC
OK, so shall I remove that part of the change from this patch. If that's the case, there should be only irc/weechat and irc/weechat-devel to fix (I have a fix ready).
Comment 11 Tijl Coosemans freebsd_committer freebsd_triage 2015-09-02 13:07:11 UTC
(In reply to Raphael Kubo da Costa from comment #10)
Yes, go ahead.  All changes to converters/libiconv and the BUILD_DEPENDS change in Mk/Uses/iconv.mk can be removed.
Comment 12 Raphael Kubo da Costa freebsd_committer freebsd_triage 2015-09-02 14:22:27 UTC
Created attachment 160634 [details]
Proposed patch

Hopefully final version attached to the bug and also available here: https://people.freebsd.org/~rakuco/exp-runs/iconv.mk-cmake-vars-v3.patch

It drops the changes to converters/libiconv and fixes irc/weechat and irc/weechat-devel. The rest is identical.

Antoine: can I get a portmgr signoff?
Comment 13 Antoine Brodin freebsd_committer freebsd_triage 2015-09-02 14:25:47 UTC
I will test this and let you know tomorrow.
Comment 14 Antoine Brodin freebsd_committer freebsd_triage 2015-09-03 13:26:47 UTC
Looks good, approved.
Comment 15 commit-hook freebsd_committer freebsd_triage 2015-09-03 15:45:12 UTC
A commit references this bug:

Author: rakuco
Date: Thu Sep  3 15:44:17 UTC 2015
New revision: 395972
URL: https://svnweb.freebsd.org/changeset/ports/395972

Log:
  Uses/iconv.mk: Set iconv-related CMake variables.

  The way we deal with iconv in base and ports across different FreeBSD
  releases is complicated: 9.x does not have iconv.h in base, 10.1 has it with
  a different prototype for iconv(3) and later versions have the right
  iconv(3) prototype. And, in some cases (USES=iconv:{translit,wchar_t}), we
  must always use the libiconv port.

  This is why there are so many checks in Uses/iconv.mk: we need to know the
  situation we currently have in order to decide whether to pull iconv from
  converters/libiconv, whether to just use its header (and pull the library
  from base) or whether to use everything from base.

  r384038 adjusted several CMake-based ports, but did so in a way that was not
  very scalable and required a few intrusive patches to some ports. Most ports
  that have both USES=cmake and USES=iconv use variations of FindIconv.cmake
  that behave similarly. This change passes the header and library values we
  really want to use to CMake using the most common variable names, bypassing
  the calls to find_path() and find_library() that would sometimes end up
  finding the wrong file. The few ports that use different variable names have
  had their Makefiles adjusted (we manually pass the values we want via
  CMAKE_ARGS).

  Other changes:
  - chinese/fcitx: Explicitly set LIBICONV_LIBC_HAS_ICONV_OPEN=OFF as we
    always want the version from ports because of USES=iconv:wchar_t.
  - editors/calligra: Explicitly use iconv:translit because Kexi needs it.
  - irc/weechat and irc/weechat-devel: The FindIconv.cmake patches could not
    be entirely removed because the check_library_exists() calls are wrong.
    Sent upstream: https://github.com/weechat/weechat/pull/513
  - textproc/ctpp2: Use iconv:translit when the TRANSLITERATE option is used.

  PORTREVISION has been bumped in editors/calligra and textproc/ctpp2 because
  their dependency list has changed in 10.2 and later as the ports version is
  always used now.

  PR:		202798
  Reviewed by:	antoine, tijl
  Approved by:	portmgr (antoine)

Changes:
  head/Mk/Uses/iconv.mk
  head/audio/tagutil/files/
  head/chinese/fcitx/Makefile
  head/chinese/fcitx/files/patch-cmake__FindLibiconv.cmake
  head/deskutils/libstreamanalyzer/Makefile
  head/deskutils/libstreams/Makefile
  head/editors/calligra/Makefile
  head/irc/weechat/Makefile
  head/irc/weechat/files/patch-cmake-FindIconv.cmake
  head/irc/weechat-devel/Makefile
  head/irc/weechat-devel/files/patch-cmake-FindIconv.cmake
  head/mail/libvmime/files/patch-cmake__FindIconv.cmake
  head/net-im/licq/files/patch-cmake-Modules-FindIconv.cmake
  head/textproc/ctpp2/Makefile
  head/textproc/ctpp2/files/patch-CMakeLists.txt
  head/textproc/simplexml/Makefile
  head/textproc/simplexml/files/patch-cmake__FindIconv.cmake
  head/textproc/wbxml2/files/patch-cmake-modules-FindIconv.cmake
Comment 16 Raphael Kubo da Costa freebsd_committer freebsd_triage 2015-09-03 15:47:33 UTC
Landed. There's a slight difference between v3 and the version I committed: I've bumped editor/calligra's and textproc/ctpp2's PORTREVISIONs following the changes from USES=iconv to USES=iconv:translit which always requires the ports version.

Thanks everyone!