Bug 1813563 - Review Request: libpasraw - Pascal interface to libraw
Summary: Review Request: libpasraw - Pascal interface to libraw
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Antonio T. (sagitter)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1813566
TreeView+ depends on / blocked
 
Reported: 2020-03-14 13:19 UTC by Mattia Verga
Modified: 2020-03-19 06:22 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2020-03-19 06:22:45 UTC
Type: ---
Embargoed:
anto.trande: fedora-review+


Attachments (Terms of Use)
ldflags patch (467 bytes, patch)
2020-03-14 18:32 UTC, Antonio T. (sagitter)
no flags Details | Diff

Description Mattia Verga 2020-03-14 13:19:30 UTC
Spec URL: https://mattia.fedorapeople.org/libpasraw.spec
SRPM URL: https://mattia.fedorapeople.org/libpasraw-1.3.0-1.20200302gitdbbe4cc.fc33.src.rpm
Description: Provides shared library to interface Pascal program with libraw.
Fedora Account System Username: mattia

Note that this package splits libpasraw out of libpasastro, so it declares a Conflict against libpasastro <= 1.2.
When this is ready to be released, I will update libpasastro to 1.3.

Comment 2 Antonio T. (sagitter) 2020-03-14 18:32:48 UTC
Created attachment 1670156 [details]
ldflags patch

Comment 3 Antonio T. (sagitter) 2020-03-14 18:33:16 UTC
Package Review
==============

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


Issues:
=======

- LICENSE file reports a GPLv3

- incoherent-version-in-changelog 1.3-1.20200302gitdbbe4cc ['1.3.0-1.20200302gitdbbe4cc.fc33', '1.3.0-1.20200302gitdbbe4cc']
 
- Sources used to build the package match the upstream source, as provided
  in the spec URL.
  Note: Upstream MD5sum check error, diff is in
  /home/sagitter/1813563-libpasraw/diff.txt
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/

- Please, create a devel subpackage and include an symbolic link `libpasraw.so` pointed to `libpasraw.so.1.1`
  The source code should generate/include header files too, and installed together the unversioned library.
  Ask to upstream.

- Use the patch to not install anything under `share/doc/libpasraw`, use only %doc to mark the documentation files.

- Linker flags are not used; use a patch like that attached and set the LDLAGS.

- This package provides a library earlier included in `libpasastro-1.2.*`; i guess it's better this way:

new `libpasastro = 1.3.0-1` must 

 BuildRequires: libpasraw-devel >= 0:1.3.0-1
 Requires: libpasraw%{?_isa} >= 0:1.3.0-1

meanwhile, `libpasraw = 1.3.0-1` will be always installed because needed by new `libpasastro >= 1.3.0-1`, 
so it won't be ever **in conflict** with `libpasastro < 1.3.0-1` because they're never installed at the same time.

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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[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.
[!]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Unknown or generated". 43 files have unknown license. Detailed
     output of licensecheck in
     /home/sagitter/1813563-libpasraw/licensecheck.txt
[?]: License file installed when any subpackage combination is installed.
[!]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[!]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[!]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[!]: 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.
[?]: 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.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 2 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]: 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]: 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]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[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]: 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

===== 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.
[!]: Final provides and requires are sane (see attachments).
[ ]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[?]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not 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.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[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]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: There are rpmlint messages (see attachment).
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: libpasraw-1.3.0-1.20200302gitdbbe4cc.fc33.x86_64.rpm
          libpasraw-debuginfo-1.3.0-1.20200302gitdbbe4cc.fc33.x86_64.rpm
          libpasraw-debugsource-1.3.0-1.20200302gitdbbe4cc.fc33.x86_64.rpm
          libpasraw-1.3.0-1.20200302gitdbbe4cc.fc33.src.rpm
libpasraw.x86_64: W: spelling-error Summary(en_US) libraw -> lib raw, lib-raw, library
libpasraw.x86_64: W: spelling-error %description -l en_US libraw -> lib raw, lib-raw, library
libpasraw.x86_64: W: incoherent-version-in-changelog 1.3-1.20200302gitdbbe4cc ['1.3.0-1.20200302gitdbbe4cc.fc33', '1.3.0-1.20200302gitdbbe4cc']
libpasraw.x86_64: W: invalid-url URL: http://sourceforge.net/projects/libpasraw/ HTTP Error 404: Not Found
libpasraw-debuginfo.x86_64: W: invalid-url URL: http://sourceforge.net/projects/libpasraw/ HTTP Error 404: Not Found
libpasraw-debugsource.x86_64: W: invalid-url URL: http://sourceforge.net/projects/libpasraw/ HTTP Error 404: Not Found
libpasraw.src: W: spelling-error Summary(en_US) libraw -> lib raw, lib-raw, library
libpasraw.src: W: spelling-error %description -l en_US libraw -> lib raw, lib-raw, library
libpasraw.src: W: invalid-url URL: http://sourceforge.net/projects/libpasraw/ HTTP Error 404: Not Found
4 packages and 0 specfiles checked; 0 errors, 9 warnings.




Rpmlint (debuginfo)
-------------------
Checking: libpasraw-debuginfo-1.3.0-1.20200302gitdbbe4cc.fc33.x86_64.rpm
libpasraw-debuginfo.x86_64: W: invalid-url URL: http://sourceforge.net/projects/libpasraw/ HTTP Error 404: Not Found
1 packages and 0 specfiles checked; 0 errors, 1 warnings.





Rpmlint (installed packages)
----------------------------
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LC_CTYPE = "C.UTF-8",
	LANG = "en_US.UTF-8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LC_CTYPE = "C.UTF-8",
	LANG = "en_US.UTF-8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
libpasraw-debugsource.x86_64: W: invalid-url URL: http://sourceforge.net/projects/libpasraw/ <urlopen error [Errno -2] Name or service not known>
libpasraw-debuginfo.x86_64: W: invalid-url URL: http://sourceforge.net/projects/libpasraw/ <urlopen error [Errno -2] Name or service not known>
libpasraw.x86_64: W: spelling-error Summary(en_US) libraw -> lib raw, lib-raw, library
libpasraw.x86_64: W: spelling-error %description -l en_US libraw -> lib raw, lib-raw, library
libpasraw.x86_64: W: incoherent-version-in-changelog 1.3-1.20200302gitdbbe4cc ['1.3.0-1.20200302gitdbbe4cc.fc33', '1.3.0-1.20200302gitdbbe4cc']
libpasraw.x86_64: W: invalid-url URL: http://sourceforge.net/projects/libpasraw/ <urlopen error [Errno -2] Name or service not known>
libpasraw.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libpasraw.so.1.1 /lib64/libstdc++.so.6
libpasraw.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libpasraw.so.1.1 /lib64/libm.so.6
libpasraw.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libpasraw.so.1.1 /lib64/libgcc_s.so.1
3 packages and 0 specfiles checked; 0 errors, 9 warnings.



Source checksums
----------------
https://github.com/pchev/libpasraw/archive/dbbe4ccc717f9aed4efd0841ceb88a5825c3463d/libpasraw-1.3.0.tar.gz :
  CHECKSUM(SHA256) this package     : 108a324450932fb8c6c84193e8bb3b45c6b3db9e7b757acf986c33d0b4ddf40c
  CHECKSUM(SHA256) upstream package : ea9b5880398177e769c163a055fc42842bf9190150a3afc8d6cc872380821d69
diff -r also reports differences


Requires
--------
libpasraw (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libm.so.6()(64bit)
    libraw.so.19()(64bit)
    libstdc++.so.6()(64bit)
    rtld(GNU_HASH)

libpasraw-debuginfo (rpmlib, GLIBC filtered):

libpasraw-debugsource (rpmlib, GLIBC filtered):



Provides
--------
libpasraw:
    libpasraw
    libpasraw(x86-64)
    libpasraw.so.1()(64bit)

libpasraw-debuginfo:
    debuginfo(build-id)
    libpasraw-debuginfo
    libpasraw-debuginfo(x86-64)

libpasraw-debugsource:
    libpasraw-debugsource
    libpasraw-debugsource(x86-64)



Generated by fedora-review 0.7.5 (5fa5b7e) last change: 2020-02-16
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1813563
Buildroot used: fedora-rawhide-x86_64
Active plugins: C/C++, Shell-api, Generic
Disabled plugins: Ocaml, Perl, PHP, R, fonts, SugarActivity, Python, Haskell, Java
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 4 Antonio T. (sagitter) 2020-03-14 18:38:42 UTC
(In reply to Antonio T. (sagitter) from comment #3)
> 
> - This package provides a library earlier included in `libpasastro-1.2.*`; i
> guess it's better this way:
> 
> new `libpasastro = 1.3.0-1` must 
> 
>  BuildRequires: libpasraw-devel >= 0:1.3.0-1
>  Requires: libpasraw%{?_isa} >= 0:1.3.0-1
> 

Actually, `Requires: libpasraw%{?_isa} >= 0:1.3.0-1` is redundant.

Comment 5 Mattia Verga 2020-03-15 08:46:09 UTC
Thanks for the review, but I don't agree with all points:

> - LICENSE file reports a GPLv3
I've reported upstream that LICENSE file (GPLv3+) is incoherent wit copyright file (GPLv2+)

> - incoherent-version-in-changelog 1.3-1.20200302gitdbbe4cc ['1.3.0-1.20200302gitdbbe4cc.fc33', '1.3.0-1.20200302gitdbbe4cc']
I'll fix it
 
> - Sources used to build the package match the upstream source, as provided
>   in the spec URL.
>   Note: Upstream MD5sum check error, diff is in
>   /home/sagitter/1813563-libpasraw/diff.txt
>   See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
I'll fix it

> - Please, create a devel subpackage and include an symbolic link `libpasraw.so` pointed to `libpasraw.so.1.1`
>   The source code should generate/include header files too, and installed together the unversioned library.
>   Ask to upstream.
Why? The package doesn't create an unversioned library and there's no need for it or for a -devel subpackage.

> - Use the patch to not install anything under `share/doc/libpasraw`, use only %doc to mark the documentation files.


> - Linker flags are not used; use a patch like that attached and set the LDLAGS.
Thanks for the patch

> - This package provides a library earlier included in `libpasastro-1.2.*`; i guess it's better this way:
> 
> new `libpasastro = 1.3.0-1` must 
> 
>  BuildRequires: libpasraw-devel >= 0:1.3.0-1
>  Requires: libpasraw%{?_isa} >= 0:1.3.0-1
> 
> meanwhile, `libpasraw = 1.3.0-1` will be always installed because needed by new `libpasastro >= 1.3.0-1`, 
> so it won't be ever **in conflict** with `libpasastro < 1.3.0-1` because they're never installed at the same time.
I don't see the rationale for this. We have a packaging guideline that covers this case:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Conflicts/#_splitting_packages
If the new package should be installable independently of whether the original package is installed, a versioned conflict is allowed

Comment 6 Mattia Verga 2020-03-15 09:57:15 UTC
Spec URL: https://mattia.fedorapeople.org/libpasraw.spec
SRPM URL: https://mattia.fedorapeople.org/libpasraw-1.3.0-2.fc33.src.rpm
Description: Provides shared library to interface Pascal program with libraw.
Fedora Account System Username: mattia

Comment 7 Antonio T. (sagitter) 2020-03-15 13:05:08 UTC
(In reply to Mattia Verga from comment #5)
> 
> > - Please, create a devel subpackage and include an symbolic link `libpasraw.so` pointed to `libpasraw.so.1.1`
> >   The source code should generate/include header files too, and installed together the unversioned library.
> >   Ask to upstream.
> Why? The package doesn't create an unversioned library and there's no need
> for it or for a -devel subpackage.

This library is born to be a `private library`; it has been separated by original software and it became a independent library now, must provide -devel files for other project which use it in buildtime/runtime.
Althought, i don't understand yet what software really needs `libpasraw` among that ones released by upstream.

> 
> > - Use the patch to not install anything under `share/doc/libpasraw`, use only %doc to mark the documentation files.
> 
> 
> > - Linker flags are not used; use a patch like that attached and set the LDLAGS.
> Thanks for the patch
> 
> > - This package provides a library earlier included in `libpasastro-1.2.*`; i guess it's better this way:
> > 
> > new `libpasastro = 1.3.0-1` must 
> > 
> >  BuildRequires: libpasraw-devel >= 0:1.3.0-1
> >  Requires: libpasraw%{?_isa} >= 0:1.3.0-1
> > 
> > meanwhile, `libpasraw = 1.3.0-1` will be always installed because needed by new `libpasastro >= 1.3.0-1`, 
> > so it won't be ever **in conflict** with `libpasastro < 1.3.0-1` because they're never installed at the same time.
> I don't see the rationale for this. We have a packaging guideline that
> covers this case:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Conflicts/
> #_splitting_packages
> If the new package should be installable independently of whether the
> original package is installed, a versioned conflict is allowed

It's true; but the original package (libpasastro), when updated, will not be in conflict anymore with `libpasraw`. Since libpasastro-1.3.0 and libpasraw-1.3.0 RPMs will be released together, i presume, then the conflict `libpasatro-1.2* vs libpasraw-1.3.0` never exists.

Comment 8 Mattia Verga 2020-03-15 14:07:27 UTC
(In reply to Antonio T. (sagitter) from comment #7)
> (In reply to Mattia Verga from comment #5)
> > 
> > > - Please, create a devel subpackage and include an symbolic link `libpasraw.so` pointed to `libpasraw.so.1.1`
> > >   The source code should generate/include header files too, and installed together the unversioned library.
> > >   Ask to upstream.
> > Why? The package doesn't create an unversioned library and there's no need
> > for it or for a -devel subpackage.
> 
> This library is born to be a `private library`; it has been separated by
> original software and it became a independent library now, must provide
> -devel files for other project which use it in buildtime/runtime.
> Althought, i don't understand yet what software really needs `libpasraw`
> among that ones released by upstream.

I've asked on the packaging mailing list if this is really mandatory... there are plenty of private libraries which don't provide their unversioned copy in a -devel subpackage and I can't find anything that says so in the guidelines.
The other projects from the same author also don't require this lib at build time.

> 
> > 
> > > - Use the patch to not install anything under `share/doc/libpasraw`, use only %doc to mark the documentation files.
> > 
> > 
> > > - Linker flags are not used; use a patch like that attached and set the LDLAGS.
> > Thanks for the patch
> > 
> > > - This package provides a library earlier included in `libpasastro-1.2.*`; i guess it's better this way:
> > > 
> > > new `libpasastro = 1.3.0-1` must 
> > > 
> > >  BuildRequires: libpasraw-devel >= 0:1.3.0-1
> > >  Requires: libpasraw%{?_isa} >= 0:1.3.0-1
> > > 
> > > meanwhile, `libpasraw = 1.3.0-1` will be always installed because needed by new `libpasastro >= 1.3.0-1`, 
> > > so it won't be ever **in conflict** with `libpasastro < 1.3.0-1` because they're never installed at the same time.
> > I don't see the rationale for this. We have a packaging guideline that
> > covers this case:
> > https://docs.fedoraproject.org/en-US/packaging-guidelines/Conflicts/
> > #_splitting_packages
> > If the new package should be installable independently of whether the
> > original package is installed, a versioned conflict is allowed
> 
> It's true; but the original package (libpasastro), when updated, will not be
> in conflict anymore with `libpasraw`. Since libpasastro-1.3.0 and
> libpasraw-1.3.0 RPMs will be released together, i presume, then the conflict
> `libpasatro-1.2* vs libpasraw-1.3.0` never exists.


One could have libpasastro-1.2 installed and run `dnf install libpasraw` and that should end in a conflict (I'm not sure if with libpasastro-1.3 released dnf will update the conflicting dependency).
But if one doesn't have libpasastro installed, they should not be forced to install it because it's listed as a requirement from libpasraw.

Comment 9 Fabio Valentini 2020-03-15 14:15:21 UTC
(In reply to Mattia Verga from comment #8)
> I've asked on the packaging mailing list if this is really mandatory...
> there are plenty of private libraries which don't provide their unversioned
> copy in a -devel subpackage and I can't find anything that says so in the
> guidelines.
> The other projects from the same author also don't require this lib at build
> time.

/me puts on his FPC hat

I don't think the argument that "it's just a private library used by some associated projects" counts here, since it's installed into %{_libdir} directly / publicly, and not into a "private" subdirectory of %{_libdir}.

How are programs using this library? I assume they are dlopen()ing it, otherwise not having an unversioned .so or header files doesn't make any sense to me.

Comment 10 Mattia Verga 2020-03-15 15:42:58 UTC
(In reply to Fabio Valentini from comment #9)
> (In reply to Mattia Verga from comment #8)
> > I've asked on the packaging mailing list if this is really mandatory...
> > there are plenty of private libraries which don't provide their unversioned
> > copy in a -devel subpackage and I can't find anything that says so in the
> > guidelines.
> > The other projects from the same author also don't require this lib at build
> > time.
> 
> /me puts on his FPC hat
> 
> I don't think the argument that "it's just a private library used by some
> associated projects" counts here, since it's installed into %{_libdir}
> directly / publicly, and not into a "private" subdirectory of %{_libdir}.
> 
> How are programs using this library? I assume they are dlopen()ing it,
> otherwise not having an unversioned .so or header files doesn't make any
> sense to me.

This library is loaded in Pascal source code with `librawname='libpasraw.so.1';` and `libraw := LoadLibrary(librawname);`, it's not dinamically linked at build time.

Here's an example of the code which needs it: https://github.com/pchev/ccdciel/blob/9ecfac2067ca8546b09ed9641a86309b300e1158/src/u_libraw.pas

From https://github.com/pchev/libpasraw/blob/master/raw/Readme.txt :
This library exist because we cannot link a C++ object directly from Pascal.
It export C function we can link from Pascal to load a raw buffer and return 
a pointer to the raw data.

Comment 11 Mattia Verga 2020-03-16 10:18:33 UTC
Spec URL: https://mattia.fedorapeople.org/libpasraw.spec
SRPM URL: https://mattia.fedorapeople.org/libpasraw-1.3.0-3.fc33.src.rpm
Description: Provides shared library to interface Pascal program with libraw.
Fedora Account System Username: mattia

Updated as you desire.

Comment 12 Antonio T. (sagitter) 2020-03-16 17:01:04 UTC
- libpasraw-1.3.0-3.fc33.src.rpm does not exist.

- %{forgesource} ??

- "make %{_smp_mflags} arch_flags="%{optflags}" LDFLAGS="%{build_ldflags}""

Better:

   %build
   %make_build arch_flags="%{optflags}" LDFLAGS="%{build_ldflags}"

Comment 13 Fabio Valentini 2020-03-16 17:18:33 UTC
> This library is loaded in Pascal source code with `librawname='libpasraw.so.1';` and `libraw := LoadLibrary(librawname);`, it's not dinamically linked at build time.

This is what I meant - it's loaded using something like "dlopen()" (or the Pascal equivalent of it). So it really is not a shared library in the usual sense, at least in my opinion.

Since there are also no headers and other development stuff (e.g. pkgconfig files) for this library, I'd suggest to move it into a "private" location (e.g. into %{_libdir}/libpasraw/), and patch dependent packages to load it from there instead.

(You also got a similar answer on your devel@ post.)

Comment 14 Mattia Verga 2020-03-16 18:19:11 UTC
Spec URL: https://mattia.fedorapeople.org/libpasraw.spec
SRPM URL: https://mattia.fedorapeople.org/libpasraw-1.3.0-4.fc33.src.rpm
Description: Provides shared library to interface Pascal program with libraw.
Fedora Account System Username: mattia

(In reply to Antonio T. (sagitter) from comment #12)
> - libpasraw-1.3.0-3.fc33.src.rpm does not exist.
My fault, I had uploaded the wrong file, now it should be ok

> 
> - %{forgesource} ??
https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_using_forges_hosted_revision_control

> 
> - "make %{_smp_mflags} arch_flags="%{optflags}" LDFLAGS="%{build_ldflags}""
> 
> Better:
> 
>    %build
>    %make_build arch_flags="%{optflags}" LDFLAGS="%{build_ldflags}"
Done

Comment 15 Antonio T. (sagitter) 2020-03-16 19:43:50 UTC
Package approved.

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

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


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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: ldconfig not called in %post and %postun for Fedora 28 and later.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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: "Unknown or generated". 43 files have unknown license. Detailed
     output of licensecheck in
     /home/sagitter/1813563-libpasraw/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[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.
[-]: 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.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 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]: 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]: 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]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[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

===== 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).
[ ]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not 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.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[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]: Fully versioned dependency in subpackages if applicable.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[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).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: libpasraw-1.3.0-4.fc33.x86_64.rpm
          libpasraw-devel-1.3.0-4.fc33.x86_64.rpm
          libpasraw-debuginfo-1.3.0-4.fc33.x86_64.rpm
          libpasraw-debugsource-1.3.0-4.fc33.x86_64.rpm
          libpasraw-1.3.0-4.fc33.src.rpm
libpasraw.x86_64: W: spelling-error Summary(en_US) libraw -> lib raw, lib-raw, library
libpasraw.x86_64: W: spelling-error %description -l en_US libraw -> lib raw, lib-raw, library
libpasraw-devel.x86_64: W: no-documentation
libpasraw.src: W: spelling-error Summary(en_US) libraw -> lib raw, lib-raw, library
libpasraw.src: W: spelling-error %description -l en_US libraw -> lib raw, lib-raw, library
5 packages and 0 specfiles checked; 0 errors, 5 warnings.




Rpmlint (debuginfo)
-------------------
Checking: libpasraw-debuginfo-1.3.0-4.fc33.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LC_CTYPE = "C.UTF-8",
	LANG = "en_US.UTF-8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
perl: warning: Setting locale failed.
perl: warning: Please check that your locale settings:
	LANGUAGE = (unset),
	LC_ALL = (unset),
	LC_CTYPE = "C.UTF-8",
	LANG = "en_US.UTF-8"
    are supported and installed on your system.
perl: warning: Falling back to the standard locale ("C").
libpasraw.x86_64: W: spelling-error Summary(en_US) libraw -> lib raw, lib-raw, library
libpasraw.x86_64: W: spelling-error %description -l en_US libraw -> lib raw, lib-raw, library
libpasraw.x86_64: W: invalid-url URL: https://github.com/pchev/libpasraw <urlopen error [Errno -2] Name or service not known>
libpasraw-devel.x86_64: W: invalid-url URL: https://github.com/pchev/libpasraw <urlopen error [Errno -2] Name or service not known>
libpasraw-devel.x86_64: W: no-documentation
libpasraw-debuginfo.x86_64: W: invalid-url URL: https://github.com/pchev/libpasraw <urlopen error [Errno -2] Name or service not known>
libpasraw-debugsource.x86_64: W: invalid-url URL: https://github.com/pchev/libpasraw <urlopen error [Errno -2] Name or service not known>
4 packages and 0 specfiles checked; 0 errors, 7 warnings.



Source checksums
----------------
https://github.com/pchev/libpasraw/archive/v1.3.0/libpasraw-1.3.0.tar.gz :
  CHECKSUM(SHA256) this package     : 108a324450932fb8c6c84193e8bb3b45c6b3db9e7b757acf986c33d0b4ddf40c
  CHECKSUM(SHA256) upstream package : 108a324450932fb8c6c84193e8bb3b45c6b3db9e7b757acf986c33d0b4ddf40c


Requires
--------
libpasraw (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libraw.so.19()(64bit)
    rtld(GNU_HASH)

libpasraw-devel (rpmlib, GLIBC filtered):
    libpasraw(x86-64)
    libpasraw.so.1()(64bit)

libpasraw-debuginfo (rpmlib, GLIBC filtered):

libpasraw-debugsource (rpmlib, GLIBC filtered):



Provides
--------
libpasraw:
    libpasraw
    libpasraw(x86-64)
    libpasraw.so.1()(64bit)

libpasraw-devel:
    libpasraw-devel
    libpasraw-devel(x86-64)

libpasraw-debuginfo:
    debuginfo(build-id)
    libpasraw-debuginfo
    libpasraw-debuginfo(x86-64)

libpasraw-debugsource:
    libpasraw-debugsource
    libpasraw-debugsource(x86-64)



Generated by fedora-review 0.7.5 (5fa5b7e) last change: 2020-02-16
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1813563
Buildroot used: fedora-rawhide-x86_64
Active plugins: Shell-api, Generic, C/C++
Disabled plugins: Perl, SugarActivity, Java, R, Python, Haskell, Ocaml, fonts, PHP
Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH

Comment 16 Antonio T. (sagitter) 2020-03-16 19:45:07 UTC
(In reply to Mattia Verga from comment #14)
> > 
> > - %{forgesource} ??
> https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
> #_using_forges_hosted_revision_control
> 

Ah! Never used.

Comment 17 Gwyn Ciesla 2020-03-18 17:00:55 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/libpasraw

Comment 18 Mattia Verga 2020-03-19 06:22:45 UTC
Thanks.

Closed by https://bodhi.fedoraproject.org/updates/FEDORA-2020-4cd3d3fcb3


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