Bug 1223673

F23 rawhide scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=9814103

This will be needed to build ghc-7.10 on armv7 for F23,
like how we current use llvm34 to build ghc-7.8 on ARMv7.

The llvm34 review for F22 was bug 1161014 BTW
and should be very similar to this.

The spec file is a merger of llvm34.spec and f22 llvm.spec.

I would love to do this but I am starting to travel on Friday and going on vacation. 

I will assign this to myself and try to get this done before I leave. If not, I will get to it at the earliest.

If anyone else wants to take a shot, feel free to assign the review to yourself.

Thanks!  Appreciated :)

Sorry. never got around to this before I went on vacation .. Will have the review done(ish) tomorrow.

Thanks! If you can still do it. :)

I would like to start working on ghc-7.10 for F24 Rawhide soon.

OK, looks pretty good except for a few minor points, and the unversioned .so files in the path, which is common with llvm and llvm34. I'm inclined to say that's OK for now, and we should try to fix this in the llvm package first.. It's not like we're introducing a new bug...


Package Review
==============

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


Issues:
=======

- Minor point: I realized each subpackage creates its own directory
  under /usr/share/doc/. Since they contain very few files and most apply
  to all subpackages (e.g. LICENSE.txt), wouldn't it be better to put everthing
  under a common llvm dir?


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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[!]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[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]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "BSD (3 clause)", "ISC", "Unknown or generated". 2580 files
     have unknown license. Detailed output of licensecheck in
     /home/milan/Dev/Fedora/llvm35-review/llvm35/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib64/llvm35
[!]: %build honors applicable compiler flags or justifies otherwise.
     Could you add a comment explaining why %{optflags} is ignored
     and why --with-optimize-option=-O3 is a good idea?
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[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.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 890880 bytes in 3 files.
[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]: 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.
[x]: Package requires other packages for directories it uses.
[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]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: No %config files under /usr.
[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]: Static libraries in -static or -devel subpackage, providing -devel if
     present.
     Note: Package has .a files: llvm35-static.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[-]: Python eggs must not download any dependencies during the build
     process.
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[-]: Package meets the Packaging Guidelines::Python
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     llvm35-doc , llvm35-libs , llvm35-static
[x]: Package functions as described.
[-]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[!]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
     Could use a bit more verbose comments and links to upstream patches.
[x]: Scriptlets must be sane, if used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[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]: Spec use %global instead of %define unless justified.

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

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 1024000 bytes in /usr/share
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).


Rpmlint
-------
Checking: llvm35-3.5.2-1.fc22.x86_64.rpm
          llvm35-devel-3.5.2-1.fc22.x86_64.rpm
          llvm35-doc-3.5.2-1.fc22.noarch.rpm
          llvm35-libs-3.5.2-1.fc22.x86_64.rpm
          llvm35-static-3.5.2-1.fc22.x86_64.rpm
          llvm35-3.5.2-1.fc22.src.rpm
llvm35.x86_64: W: spelling-error %description -l en_US runtime -> run time, run-time, rudiment
Could fix this one.

llvm35.x86_64: W: no-manual-page-for-binary llvm-diff-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-stress-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-ar-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-nm-3.5
llvm35.x86_64: W: no-manual-page-for-binary llc-3.5
llvm35.x86_64: W: no-manual-page-for-binary lli-child-target-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-bcanalyzer-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-tblgen-3.5
llvm35.x86_64: W: no-manual-page-for-binary opt-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-profdata-3.5
llvm35.x86_64: W: no-manual-page-for-binary bugpoint-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-symbolizer-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-rtdyld-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-cov-3.5
llvm35.x86_64: W: no-manual-page-for-binary lli-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-objdump-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-mcmarkup-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-mc-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-as-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-readobj-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-link-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-dwarfdump-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-extract-3.5
llvm35.x86_64: W: no-manual-page-for-binary macho-dump-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-size-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-dis-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-ranlib-3.5
llvm35-devel.x86_64: W: no-manual-page-for-binary llvm-config-64-3.5
Why don't we have manpages for all those commands, even in the standard llvm package?

llvm35-libs.x86_64: W: no-documentation
llvm35-static.x86_64: W: no-documentation
llvm35.src: W: spelling-error %description -l en_US runtime -> run time, run-time, rudiment
llvm35.src:115: E: hardcoded-library-path in /usr/lib
False positive.

llvm35.src:177: E: hardcoded-library-path in %{_prefix}/lib/gcc/%{_target_cpu}*/*/include)
Out of curiosity, why doesn't gcc install these files under %{_libdir}?

6 packages and 0 specfiles checked; 2 errors, 32 warnings.




Rpmlint (debuginfo)
-------------------
Checking: llvm35-debuginfo-3.5.2-1.fc22.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
llvm35.x86_64: W: spelling-error %description -l en_US runtime -> run time, run-time, rudiment
llvm35.x86_64: W: no-manual-page-for-binary llvm-diff-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-stress-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-ar-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-nm-3.5
llvm35.x86_64: W: no-manual-page-for-binary llc-3.5
llvm35.x86_64: W: no-manual-page-for-binary lli-child-target-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-bcanalyzer-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-tblgen-3.5
llvm35.x86_64: W: no-manual-page-for-binary opt-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-profdata-3.5
llvm35.x86_64: W: no-manual-page-for-binary bugpoint-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-symbolizer-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-rtdyld-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-cov-3.5
llvm35.x86_64: W: no-manual-page-for-binary lli-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-objdump-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-mcmarkup-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-mc-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-as-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-readobj-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-link-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-dwarfdump-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-extract-3.5
llvm35.x86_64: W: no-manual-page-for-binary macho-dump-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-size-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-dis-3.5
llvm35.x86_64: W: no-manual-page-for-binary llvm-ranlib-3.5
llvm35-devel.x86_64: W: no-manual-page-for-binary llvm-config-64-3.5
llvm35-static.x86_64: W: no-documentation
llvm35-libs.x86_64: W: no-documentation
6 packages and 0 specfiles checked; 0 errors, 31 warnings.



Requires
--------
llvm35-doc (rpmlib, GLIBC filtered):
    llvm35

llvm35 (rpmlib, GLIBC filtered):
    libLLVM-3.5.so()(64bit)
    libc.so.6()(64bit)
    libdl.so.2()(64bit)
    libffi.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libtinfo.so.5()(64bit)
    libz.so.1()(64bit)
    llvm35-libs(x86-64)
    rtld(GNU_HASH)

llvm35-static (rpmlib, GLIBC filtered):
    llvm35-devel(x86-64)

llvm35-devel (rpmlib, GLIBC filtered):
    /bin/sh
    /usr/sbin/alternatives
    libc.so.6()(64bit)
    libdl.so.2()(64bit)
    libffi-devel
    libffi.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libpthread.so.0()(64bit)
    libstdc++-devel
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libtinfo.so.5()(64bit)
    libz.so.1()(64bit)
    llvm35(x86-64)
    ncurses-devel
    rtld(GNU_HASH)

llvm35-libs (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    config(llvm35-libs)
    libLLVM-3.5.so()(64bit)
    libc.so.6()(64bit)
    libdl.so.2()(64bit)
    libffi.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgcc_s.so.1(GCC_3.4)(64bit)
    libm.so.6()(64bit)
    libpthread.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libtinfo.so.5()(64bit)
    libz.so.1()(64bit)
    libz.so.1(ZLIB_1.2.0)(64bit)
    rtld(GNU_HASH)



Provides
--------
llvm35-doc:
    llvm35-doc

llvm35:
    llvm35
    llvm35(x86-64)

llvm35-static:
    llvm35-static
    llvm35-static(x86-64)

llvm35-devel:
    llvm35-devel
    llvm35-devel(x86-64)

llvm35-libs:
    config(llvm35-libs)
    libLLVM-3.5.so()(64bit)
    libLTO.so()(64bit)
    llvm35-libs
    llvm35-libs(x86-64)



Unversioned so-files
--------------------
llvm35-libs: /usr/lib64/llvm35/BugpointPasses.so
llvm35-libs: /usr/lib64/llvm35/LLVMgold.so
llvm35-libs: /usr/lib64/llvm35/libLLVM-3.5.2.so
llvm35-libs: /usr/lib64/llvm35/libLLVM-3.5.so
llvm35-libs: /usr/lib64/llvm35/libLTO.so

Source checksums
----------------
http://llvm.org/releases/3.5.2/llvm-3.5.2.src.tar.xz :
  CHECKSUM(SHA256) this package     : 44196156d5749eb4b4224fe471a29cc3984df92570a4a89fa859f7394fc0c575
  CHECKSUM(SHA256) upstream package : 44196156d5749eb4b4224fe471a29cc3984df92570a4a89fa859f7394fc0c575


Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review -rn llvm35-3.5.2-1.fc21.src.rpm
Buildroot used: fedora-22-x86_64
Active plugins: Python, Generic, Shell-api, C/C++
Disabled plugins: Java, SugarActivity, fonts, Haskell, Ocaml, Perl, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

(In reply to Milan Bouchet-Valat from comment #7)
> the unversioned .so
> files in the path, which is common with llvm and llvm34. I'm inclined to say
> that's OK for now, and we should try to fix this in the llvm package first..
> It's not like we're introducing a new bug...

Yeah, looking at it again further it does not seem that bad actually:

/etc/ld.so.conf.d/llvm-x86_64.conf
/usr/lib64/llvm/BugpointPasses.so
/usr/lib64/llvm/LLVMgold.so
/usr/lib64/llvm/libLLVM-3.5.0.so
/usr/lib64/llvm/libLLVM-3.5.so
/usr/lib64/llvm/libLTO.so
/usr/lib64/llvm/readline.so

LLVM.so is versioned (before .so) which I consider sufficient.
Then there are a number of modules which I think are also okay
(no idea what readline is doing there...).
Which just leaves libLTO.so, which seems to me is not
used by anything in llvm35?  (Not sure what it is for
to be honest.)

So I also feel we could waive this like was done for llvm34.

> - Minor point: I realized each subpackage creates its own directory
>   under /usr/share/doc/. Since they contain very few files and most apply
>   to all subpackages (e.g. LICENSE.txt), wouldn't it be better to put
> everthing
>   under a common llvm dir?

This is true and also true for many other packages I think.
I'd rather just leave it for simplicity - though in principle
I agree with you completely, but I feel this is more a deficiency
of rpm.

> [!]: Development (unversioned) .so files in -devel subpackage, if present.
>      Note: Unversioned so-files in private %_libdir subdirectory (see
>      attachment). Verify they are not in ld path.

See above

> [!]: Package must own all directories that it creates.
>      Note: Directories without known owners: /usr/lib64/llvm35

This is a fedora-review bug.

> [!]: %build honors applicable compiler flags or justifies otherwise.
>      Could you add a comment explaining why %{optflags} is ignored
>      and why --with-optimize-option=-O3 is a good idea?

Okay

> [!]: Patches link to upstream bugs/comments/lists or are otherwise
>      justified.
>      Could use a bit more verbose comments and links to upstream patches.

> Checking: llvm35-3.5.2-1.fc22.x86_64.rpm
>           llvm35-devel-3.5.2-1.fc22.x86_64.rpm
>           llvm35-doc-3.5.2-1.fc22.noarch.rpm
>           llvm35-libs-3.5.2-1.fc22.x86_64.rpm
>           llvm35-static-3.5.2-1.fc22.x86_64.rpm
>           llvm35-3.5.2-1.fc22.src.rpm
> llvm35.x86_64: W: spelling-error %description -l en_US runtime -> run time,
> run-time, rudiment
> Could fix this one.

Hmm, isn't "runtime" pretty standard?

> llvm35.x86_64: W: no-manual-page-for-binary llvm-diff-3.5
:
:
> llvm35-devel.x86_64: W: no-manual-page-for-binary llvm-config-64-3.5
> Why don't we have manpages for all those commands, even in the standard llvm
> package?

Good question - perhaps Fedora could grab a patch from Debian?
Again I think this should be done first for the llvm package.
I opened bug 1258760 for that.

> llvm35.src:177: E: hardcoded-library-path in
> %{_prefix}/lib/gcc/%{_target_cpu}*/*/include)
> Out of curiosity, why doesn't gcc install these files under %{_libdir}?

Historical I think - it is using target arch dirs instead of multilib.

(In reply to Jens Petersen from comment #8)
> (In reply to Milan Bouchet-Valat from comment #7)
> > the unversioned .so
> > files in the path, which is common with llvm and llvm34. I'm inclined to say
> > that's OK for now, and we should try to fix this in the llvm package first..
> > It's not like we're introducing a new bug...
> 
> Yeah, looking at it again further it does not seem that bad actually:
> 
> /etc/ld.so.conf.d/llvm-x86_64.conf
> /usr/lib64/llvm/BugpointPasses.so
> /usr/lib64/llvm/LLVMgold.so
> /usr/lib64/llvm/libLLVM-3.5.0.so
> /usr/lib64/llvm/libLLVM-3.5.so
> /usr/lib64/llvm/libLTO.so
> /usr/lib64/llvm/readline.so
> 
> LLVM.so is versioned (before .so) which I consider sufficient.
> Then there are a number of modules which I think are also okay
> (no idea what readline is doing there...).
> Which just leaves libLTO.so, which seems to me is not
> used by anything in llvm35?  (Not sure what it is for
> to be honest.)
So what happens when llvm and llvm33/llvm34/llvm35 are installed, could we get conflicts about LLVMgold.so, BugpointPasses.so, libLTO.so and readline.so? I don't think so, or it wouldn't have worked.

> So I also feel we could waive this like was done for llvm34.
Sure, let's investigate this for llvm first.

> > - Minor point: I realized each subpackage creates its own directory
> >   under /usr/share/doc/. Since they contain very few files and most apply
> >   to all subpackages (e.g. LICENSE.txt), wouldn't it be better to put
> > everthing
> >   under a common llvm dir?
> 
> This is true and also true for many other packages I think.
> I'd rather just leave it for simplicity - though in principle
> I agree with you completely, but I feel this is more a deficiency
> of rpm.
Not a big deal, but I think RPM handles this if you make the directory owned by llvm35-libs, which AFAICT all subpackages depend on.

> > [!]: Development (unversioned) .so files in -devel subpackage, if present.
> >      Note: Unversioned so-files in private %_libdir subdirectory (see
> >      attachment). Verify they are not in ld path.
> 
> See above
OK.

> > [!]: Package must own all directories that it creates.
> >      Note: Directories without known owners: /usr/lib64/llvm35
> 
> This is a fedora-review bug.
OK.

> > [!]: %build honors applicable compiler flags or justifies otherwise.
> >      Could you add a comment explaining why %{optflags} is ignored
> >      and why --with-optimize-option=-O3 is a good idea?
> 
> Okay
> 
> > [!]: Patches link to upstream bugs/comments/lists or are otherwise
> >      justified.
> >      Could use a bit more verbose comments and links to upstream patches.
> 
> > Checking: llvm35-3.5.2-1.fc22.x86_64.rpm
> >           llvm35-devel-3.5.2-1.fc22.x86_64.rpm
> >           llvm35-doc-3.5.2-1.fc22.noarch.rpm
> >           llvm35-libs-3.5.2-1.fc22.x86_64.rpm
> >           llvm35-static-3.5.2-1.fc22.x86_64.rpm
> >           llvm35-3.5.2-1.fc22.src.rpm
> > llvm35.x86_64: W: spelling-error %description -l en_US runtime -> run time,
> > run-time, rudiment
> > Could fix this one.
> 
> Hmm, isn't "runtime" pretty standard?
Yeah, but I would do anything to shut down an annoying warning. :-)

> > llvm35.x86_64: W: no-manual-page-for-binary llvm-diff-3.5
> :
> :
> > llvm35-devel.x86_64: W: no-manual-page-for-binary llvm-config-64-3.5
> > Why don't we have manpages for all those commands, even in the standard llvm
> > package?
> 
> Good question - perhaps Fedora could grab a patch from Debian?
> Again I think this should be done first for the llvm package.
> I opened bug 1258760 for that.
Or, better, the manpages could be submitted upstream if Debian has some. But definitely not an issue for llvm35.

> > llvm35.src:177: E: hardcoded-library-path in
> > %{_prefix}/lib/gcc/%{_target_cpu}*/*/include)
> > Out of curiosity, why doesn't gcc install these files under %{_libdir}?
> Historical I think - it is using target arch dirs instead of multilib.
Funny...


I think it'll be good to go if you post an updated version with the small fixes I mentioned.

> > [!]: %build honors applicable compiler flags or justifies otherwise.
> >      Could you add a comment explaining why %{optflags} is ignored
> >      and why --with-optimize-option=-O3 is a good idea?
> 
> Okay

I think %configure should setup to %{optflags}.  Are you saying
it is not working or sufficient here?

-O3 just comes from llvm.spec - I can try to track down why it is there.

> > [!]: Patches link to upstream bugs/comments/lists or are otherwise
> >      justified.
> >      Could use a bit more verbose comments and links to upstream patches.

Ideally yes :)
Good to file a bug against llvm perhaps.

(In reply to Jens Petersen from comment #10)
> > > [!]: %build honors applicable compiler flags or justifies otherwise.
> > >      Could you add a comment explaining why %{optflags} is ignored
> > >      and why --with-optimize-option=-O3 is a good idea?
> > 
> > Okay
> 
> I think %configure should setup to %{optflags}.  Are you saying
> it is not working or sufficient here?
Ah, right.

> -O3 just comes from llvm.spec - I can try to track down why it is there.
Yes, a small comment wouldn't hurt. I think the guidelines say that -03 should generally be avoided unless you can be sure it really improves performance.

> > > [!]: Patches link to upstream bugs/comments/lists or are otherwise
> > >      justified.
> > >      Could use a bit more verbose comments and links to upstream patches.
> 
> Ideally yes :)
> Good to file a bug against llvm perhaps.
Yes, yet another thing to fix first in llvm.

Do you want to upload a new version so that I can approve the package?

I reported bug 1272001 about the unversioned .so files.
I will try to continue the reviews tomorrow, thanks.

(In reply to Milan Bouchet-Valat from comment #9)
> So what happens when llvm and llvm33/llvm34/llvm35 are installed, could we
> get conflicts about LLVMgold.so, BugpointPasses.so, libLTO.so and
> readline.so? I don't think so, or it wouldn't have worked.

I don't know. I am not aware of any problems, right.
I don't even know which of the programs if any are using them.
I couldn't see anything linking to libLTO for example
(of course something could be dlopen'ing them).

> > So I also feel we could waive this like was done for llvm34.
> Sure, let's investigate this for llvm first.

Yes - bug filed (comment 12).

> > > - Minor point: I realized each subpackage creates its own directory
> > >   under /usr/share/doc/. Since they contain very few files and most apply
> > >   to all subpackages (e.g. LICENSE.txt), wouldn't it be better to put
> > > everthing
> > >   under a common llvm dir?
> > 
> > This is true and also true for many other packages I think.
> > I'd rather just leave it for simplicity - though in principle
> > I agree with you completely, but I feel this is more a deficiency
> > of rpm.
> Not a big deal, but I think RPM handles this if you make the directory owned
> by llvm35-libs, which AFAICT all subpackages depend on.

I happy to take patch if you want to do that.
Sorry I don't really have time/energy/motivation to do it. :)
And again it is true for the main llvm package too I believe, right?

> > Hmm, isn't "runtime" pretty standard?
> Yeah, but I would do anything to shut down an annoying warning. :-)

These warnings are very frequent across many packages.
The spelling dictionaries are really too limited
or don't cover such technical usage.
I feel it is good to stay close to the original llvm.spec
too as far as possible.

> > Good question - perhaps Fedora could grab a patch from Debian?
> > Again I think this should be done first for the llvm package.
> > I opened bug 1258760 for that.
> Or, better, the manpages could be submitted upstream if Debian has some. But
> definitely not an issue for llvm35.

Yep

(In reply to Milan Bouchet-Valat from comment #11)
> Yes, a small comment wouldn't hurt. I think the guidelines say that -03
> should generally be avoided unless you can be sure it really improves
> performance.

Apparently it was added without comment in this commit:

commit be655c46e5d3707531fb8bef5430a9c064653197
Author: Jan Vcelak <jvcelak>
Date:   Tue Nov 12 21:48:50 2013 +0100

    update to 3.3, add compiler-rt and lldb
:    

Not sure how much thought was put into it.

I sent a mail to Jan asking about it.
I opened bug 1272394 to track this too!
We can wait for his reply or go ahead I guess
since I consider this a backport from the llvm package.


Can you live with the current spec file or what do you think
must be changed still for approval? I hope I didn't miss anything. :)

OK, good to go. This is the occasion to clean the original llvm package apparently! ;-)

I just noticed we both did a small mistake when calling alternative: the command lacks the version suffix. Should be:

%posttrans devel
alternatives \
  --install \
  %{_bindir}/llvm-config \
  llvm-config \
  %{_bindir}/llvm-config-%{__isa_bits}-%{major_version} \
  %{__isa_bits}

%postun devel
if [ $1 -eq 0 ]; then
  alternatives --remove llvm-config \
    %{_bindir}/llvm-config-%{__isa_bits}-%{major_version}
fi
exit 0

Another thing: to build Julia using the versioned LLVM 3.3, I had to fix the includedir reported by llvm-config, which was still pointing at /usr/include. You may want to do the same. See http://pkgs.fedoraproject.org/cgit/llvm33.git/commit/?id=833ba633af3f1544baea55960f12c827f11f8819

Thanks Milan for the heads-up - I put the changes in llvm34/master too,
and will include them in llvm35.

llvm35-3.5.2-2.fc21 has been submitted as an update to Fedora 21. https://bodhi.fedoraproject.org/updates/FEDORA-2015-8024f9d443

(I requested an epel7 branch but now not sure if it is really good idea/necessary (I mean since it will be long-lived) but if there is a need for it in EPEL I don't see a big problem to add it - Copr might well be a better place though otherwise.)

llvm35-3.5.2-2.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update llvm35'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-e783f41037

llvm35-3.5.2-2.fc21 has been pushed to the Fedora 21 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update llvm35'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-8024f9d443

llvm35-3.5.2-2.fc21 has been pushed to the Fedora 21 stable repository. If problems still persist, please make note of it in this bug report.

llvm35-3.5.2-2.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.