ICU4C should leave the keg-only status. #15377

Closed
wants to merge 2 commits into from

4 participants

@mbcoguno

UPDATE: Recent yaz not longer conflict with icu4c, so icu4c can leave the keg-only status. But only works on C11 compiler.


Most keg-only formula conflict with OSX or developer tools, icu4c is only one that is conflict with other formulae. (maybe conflict with system's libxml.) This will be uncomfortable for non-formulae usage, such as writing simple code with very long compiler flags.
superenv removes $HOMEBREW_PREFIX from $PATH unless a formula depends on another formula, so get error when compiling cause a formula will be the rare case. I think icu4c should be leave the keg_only status.

The first commit give a new way to prevent compiling error in standard env. superenv, of course, can prevent error.
Second commit is remove keg_only description and bottle. I think this should be rebottle, the dynamic library's path should be in $HOMEBREW_PREFIX, not in icu4c's prefix.
Last commit is remove unnecessary code, all formulae that need icu4c already have depends_on 'icu4c' code, they don't need anymore if icu4c is in $HOMEBREW_PREFIX/opt. They both works with std- or super- env.

If there have any thing that I am not considering, please contact me.

@jacknagel

Disabling icu support in yaz is just punting rather than addressing the problem though, isn't it? What if people want that functionality? I'm not opposed to removing the keg_only bits from icu4c but the original issue needs to be resolved in a more acceptable way.

This will be uncomfortable for non-formulae usage, such as writing simple code with very long compiler flags.

These days brew --prefix icu4c will give a path like /usr/local/opt/icu4c rather than the versioned path into the Cellar.

@mbcoguno

Disabling icu support in yaz is just punting rather than addressing the problem though, isn't it? ...

Maybe. But 1) No one pull this issue. 2) If someone needs icu support, they can use homebrew's libxml (not system's). It can compiles and work well. I think libxml+icu4c in yaz should be a new issue. If you say giving a patch to solve this problem (system's libxml + icu), I can't solve it.

These days brew --prefix icu4c will give a path like /usr/local/opt/icu4c rather than the versioned path into the Cellar.

But linking them in /usr/local is more convenience. Compilers can find them automatically.

@mbcoguno

@jacknagel Any suggestion?

@mbcoguno

OK, recent yaz can build with icu4c in both std and super env. I will update commit later.

@adamv

I don't have an opinion on this; other maintainers?

@jacknagel

It was originally made keg-only to deal with yaz, so if that doesn't happen anymore we might as well.

@jacknagel

It should probably be tested under stdenv first, though.

@jacknagel

yaz fails on 10.6 under stdenv with icu4c linked: https://gist.github.com/3924788

Builds fine when icu4c is unlinked, or if I add a dep on our libxml2.

@mbcoguno

Sorry, I only tested on Lion with clang 4.1 build 421 on both env. But since I don't have multiple OS and its environment, maybe I will close this issue unless I (or someone) can test all environments and reopen it.
PS. I guess this is compiler issue. Hope it is or I have no better idea with homebrew's style.

@mbcoguno

Interesting, I build yaz from tarball with 4 compiler: apple-gcc4.2, llvm-gcc, clang (4.1), gcc-4.7 (from homebrew-dupes).
GCC 4.7 and clang 4.1 that released on this year can build well, apple-gcc4.2 and llvm-gcc are fails with same error.

Then I can reproduce the same error in clang 4.1 with -Werror=typedef-redefinition. The message is:

/usr/include/libxml2/libxml/encoding.h:41:18: error: redefinition of typedef 'UChar' is a C11 feature
      [-Werror,-Wtypedef-redefinition]
typedef uint16_t UChar;
                 ^
/usr/local/Cellar/icu4c/49.1.2/include/unicode/umachine.h:276:29: note: previous definition is here
    typedef __CHAR16_TYPE__ UChar;
                            ^
1 error generated.

Maybe I can add unless ENV.compiler == :clang and MacOS.clang_build_version >= 421 after keg_only.

Please review this.

@jacknagel

I think we can just work around it with -Wno-typedef-redefinition. I can build yaz on 10.6 this way and it links with icu4c correctly.

@mbcoguno

But -Wno-typedef-redefinition can't add in gcc-4.2 and llvm-gcc and there is no idea about this. Maybe adding fails_with :gcc and fails_with :llvm; or adding opoo "Recommended clang".

@mbcoguno mbcoguno referenced this pull request Nov 9, 2012
Closed

icu4c: Update to 50.1 #15934

@mbcoguno

ICU4C is updated, anyone can review this request?

@Sharpie

One argument for keeping all system duplicates keg-only is that the Homebrew buildsystem will take steps to ensure software is linked against the newer versions. Otherwise, as shown by 4a68f0b, linking is a crapshoot and some extreme measures have to be taken.

@jacknagel

I notice that icu4c lost its keg-only status in 03ed757, which was part of #15934.

@mbcoguno
Will we observed #15934 changed several days until no new issue about this? And, edit yaz formula is just enough?

I'm sorry there has lots of issue with my update. What can I do for these?

@adamv

Post a new pull request that does what it needs to do.

@adamv adamv closed this Nov 26, 2012
@mbcoguno mbcoguno deleted the mbcoguno:icu4c branch Jan 14, 2013
@ssp ssp referenced this pull request Aug 23, 2014
@asparagui asparagui yaz 5.4.1 bd7f134
@ssp ssp referenced this pull request Aug 23, 2014
Closed

yaz vs ICU #31830

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 16, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.