Bug 197006 - [patch] sysutils/gdisk - fix build with utf16 option
Summary: [patch] sysutils/gdisk - fix build with utf16 option
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: William Grzybowski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-22 16:36 UTC by John Hein
Modified: 2017-11-24 21:25 UTC (History)
2 users (show)

See Also:
bro.development: maintainer-feedback+


Attachments
get UTF16 option working - was no-op before (3.08 KB, patch)
2015-01-22 16:36 UTC, John Hein
no flags Details | Diff
update patch to remove more stale pieces (3.08 KB, patch)
2015-01-22 22:30 UTC, John Hein
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Hein 2015-01-22 16:36:33 UTC
Created attachment 152019 [details]
get UTF16 option working - was no-op before

The UTF16 post-patch chunk only fixes up some commented lines in the makefile.  It has no effect on the build.  Because of this gdisk is built without UTF16 support even if you turn on the option.

Attached is a patch to fix that.  The fix to Makefile.freebsd could be fed upstream.
Comment 1 Bugzilla Automation freebsd_committer 2015-01-22 16:36:33 UTC
Auto-assigned to maintainer wg@FreeBSD.org
Comment 2 John Hein 2015-01-22 22:30:52 UTC
Created attachment 152035 [details]
update patch to remove more stale pieces

Updated patch to remove some more unecessary cruft from the makefile.  The removal of /usr/include/ is not needed.  There is no /usr/include in guid.h (and hasn't been since at least back to 0.8.0).
Comment 3 commit-hook freebsd_committer 2015-01-28 15:36:54 UTC
A commit references this bug:

Author: wg
Date: Wed Jan 28 15:36:12 UTC 2015
New revision: 378065
URL: https://svnweb.freebsd.org/changeset/ports/378065

Log:
  sysutils/gdisk: fix build with utf16 option

  PR:		197006
  Submitted by:	John Hein (based on)

Changes:
  head/sysutils/gdisk/Makefile
  head/sysutils/gdisk/files/patch-Makefile.freebsd
Comment 4 bro.development 2015-02-21 23:45:14 UTC
Does this fix really work? The UTF16 option gives me:
clang++ crc32.o support.o guid.o gptpart.o mbrpart.o basicmbr.o mbr.o gpt.o bsd.o parttypes.o attributes.o diskio.o diskio-unix.o gdisk.o gpttext.o  -L/usr/local/lib -licuio -fstack-protector  -luuid -o gdisk
/usr/bin/ld: //usr/local/lib/libicuuc.so.53: invalid DSO for symbol `_ZN6icu_5313UnicodeStringC1EPKc' definition
//usr/local/lib/libicuuc.so.53: could not read symbols: Bad value
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
*** [gdisk] Error code 1

Rerunning last command with -v:

clang++ -v crc32.o support.o guid.o gptpart.o mbrpart.o basicmbr.o mbr.o gpt.o bsd.o parttypes.o attributes.o diskio.o diskio-unix.o sgdisk.o gptcl.o  -L/usr/local/lib -licuio -fstack-protector  -luuid -lpopt -o sgdisk
FreeBSD clang version 3.4.1 (tags/RELEASE_34/dot1-final 208032) 20140512
Target: x86_64-unknown-freebsd10.1
Thread model: posix
Selected GCC installation: 
 "/usr/bin/ld" --eh-frame-hdr -dynamic-linker /libexec/ld-elf.so.1 --hash-style=both --enable-new-dtags -o sgdisk /usr/lib/crt1.o /usr/lib/crti.o /usr/lib/crtbegin.o -L/usr/local/lib -L/usr/lib crc32.o support.o guid.o gptpart.o mbrpart.o basicmbr.o mbr.o gpt.o bsd.o parttypes.o attributes.o diskio.o diskio-unix.o sgdisk.o gptcl.o -licuio -luuid -lpopt -lc++ -lm -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/crtend.o /usr/lib/crtn.o
/usr/bin/ld: //usr/local/lib/libicuuc.so.53: invalid DSO for symbol `_ZN6icu_5313UnicodeStringC1EPKc' definition
//usr/local/lib/libicuuc.so.53: could not read symbols: Bad value
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

Should I create a new ticket?
Comment 5 John Hein 2015-05-11 10:01:57 UTC
Re: Comment 4.  Try adding -licuuc to this:

UTF16_LDFLAGS+=        -licuio -licuuc


That error you see doesn't happen when using the base gcc/binutils in 9.x.  It does happen if you define CXX=g++48 (or g++46 or g++49 or clang++35) which may be invoking ld(1) from a newer binutils (2.22+ ?).

The old gcc/binutils happily links in libicuuc even though it's not mentioned on the command line (ld(1) looks at the DT_NEEDED elf tags - see objdump -x /usr/local/lib/libicuio.so... or elfdump -d).

You can get the same behavior from ld(1) in newer binutils by specifying -Wl,--copy-dt-needed-entries before -licuio.  That would work, too:

UTF16_LDFLAGS+=        -Wl,--copy-dt-needed-entries -licuio

I could see cases for using -licuuc explicitly and cases for using --copy-dt-needed-entries.  I'd probably pick the former, although if the code changes (icu or gdisk), that extra lib could be rendered vestigial, whereas I can see recursively following DT_NEEDED tags as the more future-proof method.

So, yes, go ahead and open a new ticket to modify the UTF16_LDFLAGS (presumably needed for all systems that don't use the older binutils).  And point it here to this comment.

p.s. That mangled symbol, _ZN6icu_5313UnicodeStringC1EPKc is:

echo _ZN6icu_5313UnicodeStringC1EPKc | c++filt
icu_53::UnicodeString::UnicodeString(char const*)

p.p.s.  See also http://stackoverflow.com/questions/26267788/linking-fails-with-gcc-4-8-2-ld-2-24-succeeds-with-gcc-4-4-7-ld-2-20
You could probably track down exactly when binutils changed, but I suspect they had their reasons to not follow DT_NEEDED tags by default anymore.

Also https://lists.freebsd.org/pipermail/freebsd-ports/2011-December/071705.html
Comment 6 John Hein 2015-05-11 10:13:00 UTC
Here's where freebsd made the change:

https://lists.freebsd.org/pipermail/svn-src-head/2013-July/050058.html

10.x was still -current then I think.  Never MFC'd to 9/stable.
Comment 7 John Hein 2015-05-11 10:51:59 UTC
Looks like someone added a bug already.