Request 448172 (revoked)

This request supersedes: request 446366 (Show diff)

Overview

Request 448172 revoked

Redone the package and reduced the number of patches. The soversion patch is only there to get a proper versioned library. This one is not aligned with upstream, but they are reworking everything so a newer version most likely won't need this one anymore.

Created by tittiatcoke over 7 years ago
In state revoked
Supersedes 446366
Open review for openSUSE:Factory:Staging:adi:275

Submit avogadrolibs

Comments for request 448172 5
Comments for request 446366 1

Jan Engelhardt (jengelh) - over 7 years ago

reviewer

The namespace libAvogadro.so.* is reserved by upstream. Has upstream already communicated they will be using "libAvogadro.so.2" (and that it be incompatible to what was in 0.9.6)?

Otherwise, as per https://en.opensuse.org/openSUSE:Shared_library_packaging_policy#When_there_is_no_versioning , your openSUSE package needs to use e.g. libAvogadro-0.9.6.so, or libAvogadro-suse.so.0.

Jan Engelhardt (jengelh) - over 7 years ago

reviewer

SR 454963

mrdocs - over 7 years ago

See the commit message. I'm good with this as a temporary fix.

Ciaran Farrell (babelworx) - over 7 years ago

reviewer

Please check the licenses of these files (GPL-3.0+?) and how they interact with the rest of the package. avogadrolibs-0.9.0/avogadro/qtplugins/qtaim/qtaimcubature.cpp avogadrolibs-0.9.0/avogadro/qtplugins/qtaim/qtaimcubature.cpp avogadrolibs-0.9.0/avogadro/qtplugins/qtaim/qtaimcubature.h

Fabian Vogt (Vogtinator) - about 7 years ago

source maintainer

Those get linked against the shared avogadrolibs runtime and linked into a shared object, loaded at runtime.

Dominique Leuenberger (dimstar) - over 7 years ago

reviewer

Package needs quite some loving touch - discussed on IRC with the folks

21:07 < DimStar> a: it's a static lib : does not belong there.. b) it's libjsoncpp - that's already a package on its own
21:07 < DimStar> then there is no LICENSE/COPYING file packaged as it seems
21:08 < einar77> DimStar: for jsoncpp, given it's static, I guess rm'ing would work?
21:08 < DimStar> c) Shared Library Packaging Policy is really something you should start using
21:08 < DimStar> einar77: I'd check why it even builds it instead of using the system provided, dynamic lib
21:08 < einar77> for c) I usually use it for new packages (syntax-highlighting was created with baselibs.conf, etc.)
21:08 < fvogt> I'll have a quick look
21:08 < DimStar> having this potentially statically linked makes you nicely hide potential sec issues that could be fixed in libjsoncpp
21:09 < einar77> avogadro is used for research, so I wouldn't be surprised if so (not that we don't have to fix it)
21:09 < DimStar> +# FIXME: you should use %%cmake macros => also seems true :)
21:10 < DimStar> +%fdupes %{buildroot} => you know the dangers of potentially hardlinking accross possible partition boundaries
21:10 < DimStar> +%{_libdir}/cmake/avogadrolibs/ => belongs to devel
21:10 < DimStar> (as would the static lib anyway, if it were to be packaged; in a -static-devel)
21:10 < einar77> DimStar: sorry for pushing it this late, we honestly forgot about this in the huge mass of packages
21:11 < DimStar> luckily it only took me two minutes of review time to come up with all that :)
21:12 < DimStar> 0002-create-soversion-libs.patch is a nice patch: but that is hopefully aligned with upstream; if all dists use a different soname, 
                 we're screwed
21:12 < fvogt> Use and installation of jsoncpp is actually hardcoded in avogadrolibs...