[Bug 1161014] Review Request: llvm34 - The Low Level Virtual Machine

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1161014

Jens Petersen <petersen@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jv+fedora@xxxxxxxxx



--- Comment #6 from Jens Petersen <petersen@xxxxxxxxxx> ---
Thank you very much for looking at the package.

(In reply to Mukundan Ragavan from comment #5)
> - Package contains BR: python2-devel or python3-devel
> 
> ---> python-devel -> python2-devel
> 
> 191 BuildRequires:  python-devel

Ok, good catch but actually lldb is not built in the llvm34 package currently.
I think I will just remove clang, and the rest of the conditional parts
completely for clarity and simplicity.

(Diffing with f21 llvm.spec might give you a better idea of the changes I have
made.)

> [?]: 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.

I think this is okay.  The same libraries are in the current llvm package.

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

I am confused: here /usr/lib64/llvm34 is owned by llvm34-libs.

> ---> The specfile has this - 
> 
> 546 %dir %{_datadir}/llvm
> 
> Shouldn't this be llvm34?
:
> [!]: Package does not own files or directories owned by other packages.
>      Note: Dirs in package are owned also by: /usr/share/llvm/cmake(llvm-
>      devel), /usr/share/llvm(llvm)

Yes, fixing.

> [?]: %build honors applicable compiler flags or justifies otherwise.
> 
> ---> Spec file has this line - 
> 
> 354   --with-optimize-option=-O3
> Is this necessary?

This came from llvm.spec - dunno if it is related to mesa or something.

Apparently it was added in this commit
http://pkgs.fedoraproject.org/cgit/llvm.git/commit/?id=be655c46e5d3707531fb8bef5430a9c064653197
without comment.

We could drop it perhaps dunno or add a comment that it comes from llvm.spec.

> [x]: Each %files section contains %defattr if rpm < 4.4
>      Note: %defattr present but not needed
> 
> ---> Spec files could be cleaned up a bit. Not a big issue though.

Again inherited from llvm.spec.

I agree probably better to clean up the file more for readibility: dropping.

> [!]: Fully versioned dependency in subpackages if applicable.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in llvm34-doc
>      , llvm34-libs , llvm34-static
> 
> ---> Shouldn't -libs subpackage have versions dep?

Probably not.  llvm34 has a versioned requires on llvm34-libs.

> llvm34.src:291: W: configure-without-libdir-spec
> llvm34.src:291: E: hardcoded-library-path in /usr/lib

I think these are falsely triggered by the sed on "./configure".
I will try removing the "./" for the former.

> llvm34.src:353: E: hardcoded-library-path in
> %{_prefix}/lib/gcc/%{_target_cpu}*/*/include)

Well that is for gcc, again from llvm.spec.
Dunno if there is a nicer way to get that gcc path.

> llvm34.src:593: E: hardcoded-library-path in %{_prefix}/lib/clang
> 
> ---> /lib --> %{_libdir}?

clang is not packaged.  Dropping.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]