Bug 1270355 - Review Request: nacl-binutils - A GNU collection of binary utilities
Review Request: nacl-binutils - A GNU collection of binary utilities
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jonathan Dieter
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 1270322 1270357 1270358 1270405
  Show dependency treegraph
 
Reported: 2015-10-09 14:30 EDT by Tom "spot" Callaway
Modified: 2016-01-28 13:29 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-01-28 13:23:33 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jdieter: fedora‑review+


Attachments (Terms of Use)
License details on a file by file basis (914.71 KB, text/plain)
2015-12-05 15:38 EST, Jonathan Dieter
no flags Details
Manually set ld scriptdir (1.07 KB, patch)
2015-12-29 03:52 EST, Jonathan Dieter
no flags Details | Diff

  None (edit)
Description Tom "spot" Callaway 2015-10-09 14:30:22 EDT
Spec URL: https://spot.fedorapeople.org/nacl-binutils.spec
SRPM URL: https://spot.fedorapeople.org/nacl-binutils-2.24-3.git1d8592c.fc23.src.rpm
Description: 
Binutils is a collection of binary utilities, including ar (for
creating, modifying and extracting from archives), as (a family of GNU
assemblers), gprof (for displaying call graph profile data), ld (the
GNU linker), nm (for listing symbols from object files), objcopy (for
copying and translating object files), objdump (for displaying
information from object files), ranlib (for generating an index for
the contents of an archive), readelf (for displaying detailed
information about binary files), size (for listing the section sizes
of an object or archive file), strings (for listing printable strings
from files), strip (for discarding symbols), and addr2line (for
converting addresses to file and line).

Fedora Account System Username: spot

This is the "nacl" x86-64 build of binutils, used as part of the "nacl" toolchain for Chromium.
Comment 1 Jonathan Dieter 2015-11-11 07:58:42 EST
Hey Tom, if you'd like I'll start working through reviewing the packages needed for Chromium.  On the first glance through this one, I noticed that it puts the binaries in /usr/x86_64-nacl.  Can we move it?  Would there be a better place to put it?
Comment 2 Tom "spot" Callaway 2015-11-11 11:11:58 EST
The thing to remember about the nacl and nacl-arm packages is that they are cross-toolchains. The nacl-* packages are "x86_64-nacl" vs the Fedora default toolchain which is "x86_64-linux" (and nacl-arm-* is "arm-nacl")

The pathing it uses is in sync with how Fedora handles cross-toolchains, see:

http://koji.fedoraproject.org/koji/rpminfo?rpmID=6697860
Comment 3 Ralf Corsepius 2015-11-11 11:15:44 EST
(In reply to Tom "spot" Callaway from comment #2)
> The thing to remember about the nacl and nacl-arm packages is that they are
> cross-toolchains.
Exactly. This location is the default location the GNU-toolchain installs certain files into and expects to find them. Changing this location is non-trivial.
Comment 4 Jonathan Dieter 2015-11-11 15:15:55 EST
Thanks, that's what I wanted to check on.  I'm just heading to bed, but I'll do  a more thorough review in the morning.
Comment 5 Jonathan Dieter 2015-11-13 03:32:12 EST
Ok, this looks like it's overall in great shape.  I do have a few questions, most of them a bit nit-picky. 

1-
Nothing owns /usr/x86_64-nacl, and I'm pretty sure we should.

2-
All of the binaries are hardlinked from /usr/x86_64-nacl/* to /usr/bin/x86_64-nacl-*.  According to the guidelines at https://fedoraproject.org/wiki/Packaging_Cross_Compiling_Toolchains, these should be symlinks.  In extremely rare (possibly non-existent) situations (like having /usr/bin on a separate partition than /usr), hardlinks will cause a problem.

Having said that, other cross-compiling toolchains in Fedora are using hardlinks instead of symlinks.  I asked on the packaging mailing list, and it seems symlinks are preferable, but not enough as to make this a blocker.

If you're up for converting the hardlinks to symlinks, that would be great, but if you feel it's just make-work that's not worth the effort, we can let this go.

3-
This has ExclusiveArch: x86_64.  I'm sure there are good reasons for this, but, according to https://fedoraproject.org/wiki/Architectures#ExcludeArch_.26_ExclusiveArch (which may look familiar to you, Tom, as I believe you wrote it 😉), ExclusiveArch is highly discouraged.

There's already a comment in the spec requesting that it not be built on arm, but, at least according to https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Build_Failures, it looks like this ExclusiveArch should really be converted to multiple ExcludeArch's with explanations as to why it can't build for each arch.

If this toolchain is designed to only work under x86_64, I'd really love that to be made very explicitly clear in the spec, but if it can theoretically built and used in other arches, I think we have to go through the official process listed in the guidelines cited in the previous paragraph.
Comment 6 Tom "spot" Callaway 2015-12-04 15:58:34 EST
I've added /usr/x86_64-nacl ownership.

I think chromium broke on the softlinks. Would rather just keep it as is, where I know it works.

This toolchain (nacl-binutils, nacl-gcc, nacl-newlib) is only ever used to make x86_64 and i686 builds. I'm not entirely sure if it will work on i686 systems, but it definitely isn't useful anywhere else. We disable nacl/pnacl on i686 right now. For these packages, ExclusiveArch is correct, because we never care/need for it to build anywhere else, and pnacl/chromium wouldn't use it even if we did.

The other nacl toolchain (nacl-arm-$foo) is only ever used to do arm builds, but because of how pnacl works, it needs this to be built for x86_64 (and i686 and arm in theory). Since we're disabling i686 nacl/pnacl right now and I haven't got enough good liquor to work on chromium ARM yet, ExclusiveArch is correct there too.

I think I can figure out how to build the nacl/pnacl stack for arm without nacl, but thats a big TODO (Tomas Popela added some fixes to pull in the ffmpeg arm bits, so thats a big part out of the way).

Chromium upstream doesn't support ppc/s390 as far as I can see, so there's no sense in going through the normal process for that.

I've updated the comment in nacl-binutils to provide a more accurate summary of the situation.

New SRPM: https://spot.fedorapeople.org/nacl-binutils-2.24-4.git1d8592c.fc23.src.rpm
New SPEC: https://spot.fedorapeople.org/nacl-binutils.spec
Comment 7 Jonathan Dieter 2015-12-05 15:34:58 EST
Ok, I've done a full review, and it will be posted in a few minutes, but I just wanted to comment that I find the spec file to be very complex, especially given that it's x86_64 only.  There are numerous references to other arches, including s390 and ppc, and there are numerous references to both static and dynamic libraries that don't make it into the final RPM.

I'm assuming that this is because the spec was copied from another binutils spec, but I really think it should be sanitized before going into Fedora.  If my assumption is wrong, and there are reasons for all this, do please correct me.
Comment 8 Jonathan Dieter 2015-12-05 15:37:00 EST
Sorry, while I'm thinking about it, thanks for fixing issue 1 in comment #5, and giving explanations for issues 2 and 3.
Comment 9 Jonathan Dieter 2015-12-05 15:38 EST
Created attachment 1102597 [details]
License details on a file by file basis
Comment 10 Jonathan Dieter 2015-12-05 15:39:36 EST
Rpmlint
-------
Checking: nacl-binutils-2.24-4.git1d8592c.fc23.x86_64.rpm
          nacl-binutils-debuginfo-2.24-4.git1d8592c.fc23.x86_64.rpm
          nacl-binutils-2.24-4.git1d8592c.fc23.src.rpm
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-ld.bfd /usr/x86_64-nacl/bin/ld
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/as /usr/bin/x86_64-nacl-as
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-strip /usr/x86_64-nacl/bin/strip
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/ar /usr/bin/x86_64-nacl-ar
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-ld /usr/x86_64-nacl/bin/ld
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/ld.bfd /usr/bin/x86_64-nacl-ld.bfd
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/ld.bfd /usr/bin/x86_64-nacl-ld
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/objcopy /usr/bin/x86_64-nacl-objcopy
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/ranlib /usr/bin/x86_64-nacl-ranlib
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/objdump /usr/bin/x86_64-nacl-objdump
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-nm /usr/x86_64-nacl/bin/nm
nacl-binutils.x86_64: W: no-manual-page-for-binary x86_64-nacl-ld.bfd
nacl-binutils.x86_64: W: non-standard-dir-in-usr x86_64-nacl
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-nacl.h
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-vxworks.c
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-vxworks.h
nacl-binutils-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/nacl-binutils-2.24-git1d8592c/include/elf/epiphany.h
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-nacl.c
nacl-binutils.src: W: spelling-error %description -l en_US ar -> AR, Ar, at
nacl-binutils.src: W: spelling-error %description -l en_US gprof -> prof, g prof, Prof
nacl-binutils.src: W: spelling-error %description -l en_US ld -> ls, l, d
nacl-binutils.src: W: spelling-error %description -l en_US nm -> NM, mm, n
nacl-binutils.src: W: spelling-error %description -l en_US objcopy -> obj copy, obj-copy, copybook
nacl-binutils.src: W: spelling-error %description -l en_US objdump -> obj dump, obj-dump, dumpy
nacl-binutils.src: W: spelling-error %description -l en_US ranlib -> ran lib, ran-lib, librarian
nacl-binutils.src: W: spelling-error %description -l en_US readelf -> read elf, read-elf, reader
nacl-binutils.src: W: invalid-url Source0: nacl-binutils-2.24-git1d8592c.tar.bz2
3 packages and 0 specfiles checked; 4 errors, 23 warnings.




Rpmlint (debuginfo)
-------------------
Checking: nacl-binutils-debuginfo-2.24-4.git1d8592c.fc23.x86_64.rpm
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-nacl.h
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-vxworks.c
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-nacl.c
nacl-binutils-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/nacl-binutils-2.24-git1d8592c/include/elf/epiphany.h
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-vxworks.h
1 packages and 0 specfiles checked; 4 errors, 1 warnings.





Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/ar /usr/bin/x86_64-nacl-ar
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/as /usr/bin/x86_64-nacl-as
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-nm /usr/x86_64-nacl/bin/nm
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-ld /usr/x86_64-nacl/bin/ld
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-objdump /usr/x86_64-nacl/bin/objdump
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-strip /usr/x86_64-nacl/bin/strip
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-objcopy /usr/x86_64-nacl/bin/objcopy
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/bin/x86_64-nacl-ld.bfd /usr/x86_64-nacl/bin/ld
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/ld.bfd /usr/bin/x86_64-nacl-ld
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/ld.bfd /usr/bin/x86_64-nacl-ld.bfd
nacl-binutils.x86_64: W: cross-directory-hard-link /usr/x86_64-nacl/bin/ranlib /usr/bin/x86_64-nacl-ranlib
nacl-binutils.x86_64: W: no-manual-page-for-binary x86_64-nacl-ld.bfd
nacl-binutils.x86_64: W: non-standard-dir-in-usr x86_64-nacl
nacl-binutils-debuginfo.x86_64: W: spurious-executable-perm /usr/src/debug/nacl-binutils-2.24-git1d8592c/include/elf/epiphany.h
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-nacl.h
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-nacl.c
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-vxworks.h
nacl-binutils-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/nacl-binutils-2.24-git1d8592c/bfd/elf-vxworks.c
2 packages and 0 specfiles checked; 4 errors, 14 warnings.
Comment 11 Jonathan Dieter 2015-12-05 15:44:33 EST
A vast majority of the rpmlint errors can be ignored.  I'd greatly appreciate it if you'd notify upstream the they have the wrong FSF address in a few of their files.  Also, please chmod -x include/elf/epiphany.h
Comment 12 Jonathan Dieter 2015-12-05 15:45:28 EST
Requires
--------
nacl-binutils (rpmlib, GLIBC filtered):
    /bin/sh
    /sbin/install-info
    libc.so.6()(64bit)
    libdl.so.2()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_3.3.1)(64bit)
    libm.so.6()(64bit)
    libz.so.1()(64bit)
    libz.so.1(ZLIB_1.2.0)(64bit)
    rtld(GNU_HASH)

nacl-binutils-debuginfo (rpmlib, GLIBC filtered):



Provides
--------
nacl-binutils:
    nacl-binutils
    nacl-binutils(x86-64)

nacl-binutils-debuginfo:
    nacl-binutils-debuginfo
    nacl-binutils-debuginfo(x86-64)
Comment 13 Jonathan Dieter 2015-12-05 15:58:07 EST
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:

[!]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.

     Note: This package contains multiple COPYING files, none of which are
           included in %license

[!]: License field in the package spec file matches the actual license.
     
     Note: The files in this package are under numerous licenses, but the main
           concern here is that the aggregate work seems to also be under
           numerous licenses, based on the number of COPYING files in the
           source. I could be wrong, but GPLv3+ seems to be too simplistic.

[!]: License file installed when any subpackage combination is installed.

     Note: This is really tied into the first problem listed above.

[!]: Spec file is legible and written in American English.

     Note: The spec file is incredibly difficult to read, with a lot of
           conditionals that seem to be irrelevant for this package. I suspect
           that the spec was originally copied from another binutils package
           and never really sanitized.

[!]: Permissions on files are set properly.

     Note: include/elf/epiphany.h is set as executable, but should not be
     
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: Package uses nothing in %doc for runtime.
[x]: The spec file handles locales properly.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: Requires correct, justified where necessary.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[-]: Package contains systemd file(s) if in need.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
     
===== SHOULD items =====

Generic:
[!]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)

     Note: Package has unneeded %clean section

[!]: Spec use %global instead of %define unless justified.

     Note: Package uses %define in multiple places

[!]: Scriptlets must be sane, if used.

     Note: It seems that ldconfig isn't needed as no libraries are installed

[!]: URL is correct.

     Note: The URL is http://sources.redhat.com/binutils, but git points to
           chromium.org.  Is this the correct URL?

[x]: Final provides and requires are sane (see attachments).
[x]: Package does not include license text files separate from upstream.
[x]: SourceX tarball generation or download is documented.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX is a working URL.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: Packages should try to preserve timestamps of original installed
     files.
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[-]: Fully versioned dependency in subpackages if applicable.
[-]: Latest version is packaged.
     Note: This version is tied to the version of Chromium being built
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: %check is present and all tests pass.
[?]: Package functions as described.
     Note: I've built and installed the package and verified that the commands
           run, but I have no idea if it actually does what it's supposed to
           do.  This is not a blocker.


===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Rpmlint is run on debuginfo package(s).
[x]: Rpmlint is run on all installed packages.
[x]: Spec file according to URL is the same as in SRPM.
Comment 14 Tom "spot" Callaway 2015-12-07 10:32:24 EST
FSF address issue filed with upstream binutils:
https://sourceware.org/bugzilla/show_bug.cgi?id=19339

The spec was copied from... Fedora's binutils spec. I'd like to keep it as close to that as is reasonably plausible. While it is true that chromium doesn't care about ppc/s390, it might someday, and I'd hate to miss out on these quirks.

ldconfig is not actually invoked because %{isnative} is set to 0.

URL is correct. This is a point in time snapshot from upstream binutils, they run a git mirror on their end.

License is technically correct (the best sort of correct). All the source code goes in a pot, the result is:

Public Domain and GPL+ and GPLv2+ and GPLv3+ and LGPLv2+ and and MIT and BSD

MIT/BSD are permissive, honoring the other licenses means you've honored them as well. Public Domain is the absence of copyright terms, nothing to do there.
GPL+ and GPLv2+ and GPLv3+ is simplified down to GPLv3+ (they all upconvert to GPLv3+ since we have to honor those terms). LGPLv2+ goes to LGPLv3+ to be compatible with GPLv3+, and LGPLv3+ permits you to convey it under the terms of the GPL. Thus, GPLv3+ is correct.

That said, GPLv3+ and LGPLv3+ is a bit more appropriate, so I've switched it to that.

* Mon Dec  7 2015 Tom Callaway <spot@fedoraproject.org> - 2.24-5.git1d8592c
- fix defines to be globals
- fix license tag
- add COPYING files as license files
- drop legacy clean section

New SRPM: https://spot.fedorapeople.org/nacl-binutils-2.24-5.git1d8592c.fc23.src.rpm
New SPEC: https://spot.fedorapeople.org/nacl-binutils.spec
Comment 15 Jonathan Dieter 2015-12-07 12:28:06 EST
Ok, agreed on everything listed in comment #14.  The last remaining item is include/elf/epiphany.h being set as executable.  As this is a permission problem with the source, it only shows up in the -debuginfo package, so I'd be inclined to file a bug and then let it go.  What do you think?

One thing I did notice as I tested this is that x86_64-nacl-ld doesn't look for the ldscripts directory in the right place.  I'm assuming this can be overridden, but would it make sense to patch it so it always points to the right location?  Please do correct me if I'm wrong.
Comment 16 Tom "spot" Callaway 2015-12-07 14:52:21 EST
Where did you see x86_64-nacl-ld looking in the wrong place? Chromium's never failed on it.
Comment 17 Jonathan Dieter 2015-12-08 05:36:37 EST
$ x86_64-nacl-ld -o test.out test.o
x86_64-nacl-ld: cannot open linker script file ldscripts/elf32_x86_64_nacl.xc: No such file or directory

$ strace -etrace=open,stat x86_64-nacl-ld -o test.out test.o
open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
open("/lib64/libz.so.1", O_RDONLY|O_CLOEXEC) = 3
open("/lib64/libdl.so.2", O_RDONLY|O_CLOEXEC) = 3
open("/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
open("/usr/lib/locale/locale-archive", O_RDONLY|O_CLOEXEC) = 3
stat("/usr/bin/x86_64-nacl-ld", {st_mode=S_IFREG|0755, st_size=1870536, ...}) = 0
stat("/usr/bin/../x86_64-nacl/sys-root", 0x7ffefdfc39e0) = -1 ENOENT (No such file or directory)
stat("/usr/bin/x86_64-nacl-ld", {st_mode=S_IFREG|0755, st_size=1870536, ...}) = 0
stat("/usr/bin/../x86_64-nacl/sys-root", 0x7ffefdfc39e0) = -1 ENOENT (No such file or directory)
open("/usr/lib64/gconv/gconv-modules.cache", O_RDONLY) = 3
open("ldscripts/elf32_x86_64_nacl.xc", O_RDONLY) = -1 ENOENT (No such file or directory)
stat("/usr/bin/x86_64-nacl-ld", {st_mode=S_IFREG|0755, st_size=1870536, ...}) = 0
stat("/usr/bin/../lib/ldscripts", 0x7ffefdfc3990) = -1 ENOENT (No such file or directory)
stat("/usr/bin/x86_64-nacl-ld", {st_mode=S_IFREG|0755, st_size=1870536, ...}) = 0
stat("/usr/bin/../lib/ldscripts", 0x7ffefdfc3990) = -1 ENOENT (No such file or directory)
stat("/usr/bin/x86_64-nacl-ld", {st_mode=S_IFREG|0755, st_size=1870536, ...}) = 0
stat("/usr/bin//ldscripts", 0x7ffefdfc3990) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/locale.alias", O_RDONLY|O_CLOEXEC) = 3
open("/usr/share/locale/en_US.UTF-8/LC_MESSAGES/x86_64-nacl-ld.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en_US.utf8/LC_MESSAGES/x86_64-nacl-ld.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en_US/LC_MESSAGES/x86_64-nacl-ld.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en.UTF-8/LC_MESSAGES/x86_64-nacl-ld.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en.utf8/LC_MESSAGES/x86_64-nacl-ld.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en/LC_MESSAGES/x86_64-nacl-ld.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
x86_64-nacl-ld: cannot open linker script file ldscripts/elf32_x86_64_nacl.xc: open("/usr/share/locale/en_US.UTF-8/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en_US.utf8/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en_US/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en.UTF-8/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en.utf8/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
No such file or directory
+++ exited with 1 +++
Comment 18 Jonathan Dieter 2015-12-08 05:40:10 EST
Please note that I don't expect the command to work as is, but I would expect it to at least find all of the link scripts it's looking for.  And it may be that this is not important for Chromium, in which case we can probably ignore this.  It just grates on me a bit that the command doesn't work out of the box.
Comment 19 Tom "spot" Callaway 2015-12-08 13:35:04 EST
I'm not sure if the default binary is expected to find all its linkerscripts by default, though, I admit that would be sane. Specifying it (which I believe is how Chromium always uses it) definitely works:

/usr/bin/x86_64-nacl-ld --verbose -L/usr/x86_64-nacl/lib

I'm not sure the upstream cares about this, because they only ever use the toolchain out of a directory in the chromium source tree.

I also can't find any obvious way to fix it in the code. Ralf, any ideas?

* Mon Dec  7 2015 Tom Callaway <spot@fedoraproject.org> - 2.24-6.git1d8592c
- chmod -x include/elf/epiphany.h

New SRPM: https://spot.fedorapeople.org/nacl-binutils-2.24-6.git1d8592c.fc23.src.rpm
New SPEC: https://spot.fedorapeople.org/nacl-binutils.spec
Comment 20 Jonathan Dieter 2015-12-29 03:52 EST
Created attachment 1110130 [details]
Manually set ld scriptdir

Ok, I've done some digging and this should fix it.  The problem is that $scriptdir depends on $tooldir which we manually set when we run %make.  This patch manually sets $scriptdir to the correct location.
Comment 21 Jonathan Dieter 2015-12-29 03:56:36 EST
As this patch fixes the final problem I listed, this review is APPROVED contingent on either the application of this patch or another fix for comment #17.
Comment 22 Rex Dieter 2016-01-15 12:44:44 EST
Could this get imported soonish?  That would help with doing reviews of all the stuff that depends on it
Comment 23 Gwyn Ciesla 2016-01-15 14:42:10 EST
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/nacl-binutils
Comment 24 Fedora Update System 2016-01-15 15:28:08 EST
nacl-binutils-2.24-7.git1d8592c.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2016-ce25decc26
Comment 25 Fedora Update System 2016-01-16 14:25:00 EST
nacl-binutils-2.24-7.git1d8592c.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-ce25decc26
Comment 26 Fedora Update System 2016-01-17 09:22:50 EST
nacl-binutils-2.24-7.git1d8592c.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-50fd8d04eb
Comment 27 Fedora Update System 2016-01-28 13:23:30 EST
nacl-binutils-2.24-7.git1d8592c.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.
Comment 28 Fedora Update System 2016-01-28 13:29:48 EST
nacl-binutils-2.24-7.git1d8592c.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.