[Bug 1223673] Review Request: llvm35 - 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=1223673



--- Comment #9 from Milan Bouchet-Valat <nalimilan@xxxxxxx> ---
(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.

-- 
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]