Hi David,

I would take a look at https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL for your Source URL information.

http://fedoraproject.org/wiki/Packaging:DistTag also provides some information on conditionals to clean up the rh_dist you define in the spec.

I am not sure if the suse/sles related conditional logic is allowable since it isn't pertinent, someone else may be able to offer insight.

The duplicate License: declaration also seems unnecessary

%defattr is unnecessary

%clean is unnecessary unless supporting el5

http://fedoraproject.org/wiki/Packaging:Guidelines#.25clean

Please drop those futile opensuse macros in Fedora packages.

More detailed initial review thought(Note you need a sponsor, I can't help, I will address this at the end):

1. The packaging style looks like a decade ago.

%define qore_ver 0.8.11

You should put 0.8.11 in Version tag and use %{version} instead of custom macro, we have some fundamental macros which you should avoid using custom macro replaced

2. I don't think you've read the guideline, for example, %define -> %global:

https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

3. Remove those non-Fedora conditional bits:

4. # see if we can determine the distribution type
%if 0%{!?dist:1}
%define rh_dist %(if [ -f /etc/redhat-release ];then cat /etc/redhat-release|sed "s/[^0-9.]*//"|cut -f1 -d.;fi)
%if 0%{?rh_dist}
%define dist .rhel%{rh_dist}
%else

---------

Please learn how to use macro %{?el}/%{?fedora}

4. Drop obsoleted RPM macros which are still heavily used by other distros:

BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root

%defattr(-,root,root,-)

%clean

%defattr(-,root,root,-)

5. You are polluting dist tag:

http://fedoraproject.org/wiki/Packaging:DistTag

6. Drop BuildRequires: gcc-c++:

https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2

7. Never make BuildRequires: fdupes in any Fedora specs, we don't need it.

8. You should avoid packaging libraries with version as its package name:

libqore5

You'd better change it to libqore or qore-libs

9. I still don't understand those Provides:  in libqore, can't RPM handle this?

10. %ifarch x86_64 ppc64 x390x
c64=--enable-64bit
%endif
# need to configure with /usr as prefix as this will be used to derive the module directory
./configure RPM_OPT_FLAGS="$RPM_OPT_FLAGS" --prefix=/usr --disable-debug --disable-static $c64 --libdir=%{_libdir}

a) %configure macro should be used

b) Does qore work on ARM? We will have AArch64(ARM v8) in the future.

11. /usr/bin/ -> %{_bindir}

%{_prefix}/include/ -> %{_includedir}

/usr/share/man/ -> %{_mandir}

I think you don't need to care about RHEL5 nowadays.

12. Why not merge 2 doc packages into 1 -doc?

13. mkdir -p $RPM_BUILD_ROOT/usr/bin
mkdir -p $RPM_BUILD_ROOT/%{module_dir}/%{qore_ver}
mkdir -p $RPM_BUILD_ROOT/usr/man/man1

I think install script should do that(create in install script or with install -p), since you are the upstream, these could be enhanced.

13. https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

14. %post -n libqore5
ldconfig %{_libdir}

%postun -n libqore5
ldconfig %{_libdir}

http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Shared_libraries

------------------------------------
Anyway, read and follow this if you want to be sponsored:

https://fedoraproject.org/wiki/Join_the_package_collection_maintainers

(In reply to Jason Taylor from comment #1)
> Hi David,
> 
> I would take a look at
> https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL
> for your Source URL information.

ok done

> 
> http://fedoraproject.org/wiki/Packaging:DistTag also provides some
> information on conditionals to clean up the rh_dist you define in the spec.
> 

ok done

> I am not sure if the suse/sles related conditional logic is allowable since
> it isn't pertinent, someone else may be able to offer insight.

ok removed

> 
> The duplicate License: declaration also seems unnecessary
> 
> %defattr is unnecessary
> 
> %clean is unnecessary unless supporting el5
> 
> http://fedoraproject.org/wiki/Packaging:Guidelines#.25clean

ok - all done

thanks very much for the excellent review and help!

David

(In reply to Christopher Meng from comment #2)
> Please drop those futile opensuse macros in Fedora packages.

ok - done

I wanted to have one spec file for all rpm-based distros, but since it's a problem I've made a spec file just for fedora/rhel

thanks
david

(In reply to Christopher Meng from comment #3)
> More detailed initial review thought(Note you need a sponsor, I can't help,
> I will address this at the end):
> 
> 1. The packaging style looks like a decade ago.
> 
> %define qore_ver 0.8.11
> 
> You should put 0.8.11 in Version tag and use %{version} instead of custom
> macro, we have some fundamental macros which you should avoid using custom
> macro replaced
> 

you are right, this spec file was born a long time ago.

this has been fixed

> 2. I don't think you've read the guideline, for example, %define -> %global:
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#.
> 25global_preferred_over_.25define
> 

I did not read them closely enough, you are right.

this is now fixed.

> 3. Remove those non-Fedora conditional bits:
> 

sll removed

> 4. # see if we can determine the distribution type
> %if 0%{!?dist:1}
> %define rh_dist %(if [ -f /etc/redhat-release ];then cat
> /etc/redhat-release|sed "s/[^0-9.]*//"|cut -f1 -d.;fi)
> %if 0%{?rh_dist}
> %define dist .rhel%{rh_dist}
> %else
> 
> ---------
> 
> Please learn how to use macro %{?el}/%{?fedora}
> 

fixed

> 4. Drop obsoleted RPM macros which are still heavily used by other distros:
> 
> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
> 
> %defattr(-,root,root,-)
> 
> %clean
> 
> %defattr(-,root,root,-)
> 

removed / fixed

> 5. You are polluting dist tag:
> 
> http://fedoraproject.org/wiki/Packaging:DistTag
> 

ok done

> 6. Drop BuildRequires: gcc-c++:
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2
> 

dropped / done

> 7. Never make BuildRequires: fdupes in any Fedora specs, we don't need it.
> 

removed (was there for opensuse)

> 8. You should avoid packaging libraries with version as its package name:
> 
> libqore5
> 
> You'd better change it to libqore or qore-libs
> 

changed to libqore for fedora / rhel

> 9. I still don't understand those Provides:  in libqore, can't RPM handle
> this?
> 

actually, no (at least not AFAIK).  the Provides are there so that qore binary modules, which are loaded at runtime by libqore, can be matched with the module ABI of the qore library.

I plan on making submission requests for qore module packages later (hopefully after I can get sponsorship to main qore for Fedora - I realize that this is not a given and anyway will take time and commitment on my part).  There are quiet a few of these already.

The modules then will declare Requires: for the specific module API that they are compiled against.  These modules will be binary-loadable by future libqore packages that declare the old module ABI.  The RPM system then will not complain when libqore is upgraded and a module compiled against a previous version of qore (and using an older, but still compatible qore module ABI) is still on the system.

Otherwise without this mechanism, I would have to add an explicit dependency to libqore in the modules' spec files, which would be more restrictive than what is actually necessary, since future versions of libqore normally maintain ABI compatibility with earlier versions.

It was my impression that the spec files Provides: lines for such artificial dependencies was to handle this sort of situation.

> 10. %ifarch x86_64 ppc64 x390x
> c64=--enable-64bit
> %endif
> # need to configure with /usr as prefix as this will be used to derive the
> module directory
> ./configure RPM_OPT_FLAGS="$RPM_OPT_FLAGS" --prefix=/usr --disable-debug
> --disable-static $c64 --libdir=%{_libdir}
> 
> a) %configure macro should be used
> 
> b) Does qore work on ARM? We will have AArch64(ARM v8) in the future.
> 

I have moved all the 64-bit detection stuff to configure and added support for 64-bit ARM (aarch64)

> 11. /usr/bin/ -> %{_bindir}
> 
> %{_prefix}/include/ -> %{_includedir}
> 
> /usr/share/man/ -> %{_mandir}
> 
> I think you don't need to care about RHEL5 nowadays.
> 

done - removed

> 12. Why not merge 2 doc packages into 1 -doc?
> 

The reason for this is because the devel-doc package is only needed for programmers programming against the C++ API (ie for qore binary modules).  This doc package is large, but most users won't need it (I expect).

Most users will only need the doc package, which has the Qore documentation telling programmer's how to program in the Qore language.

So From my point of view it makes sense to have two packages for the two very different kinds of documentation provided by qore.

However, if this is a blocking issue for acceptance by Fedora, then let me know, and I will merge them both.

> 13. mkdir -p $RPM_BUILD_ROOT/usr/bin
> mkdir -p $RPM_BUILD_ROOT/%{module_dir}/%{qore_ver}
> mkdir -p $RPM_BUILD_ROOT/usr/man/man1
> 
> I think install script should do that(create in install script or with
> install -p), since you are the upstream, these could be enhanced.
> 

you are right, this was not necesary, configure generates a Makefile that performs these actions automatically.

This has been removed from the spec file.

> 13.
> https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
> 

done

> 14. %post -n libqore5
> ldconfig %{_libdir}
> 
> %postun -n libqore5
> ldconfig %{_libdir}
> 
> http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Shared_libraries
> 

done

> ------------------------------------
> Anyway, read and follow this if you want to be sponsored:
> 
> https://fedoraproject.org/wiki/Join_the_package_collection_maintainers

thanks for that!  I had indeed read that and had already verified my package with a koji scratch build.

I plan on following up on the other activities recommended for achieving sponsorship in the near future.

I realize that this will take some time, and that I will need to demonstrate a commitment to Fedora's processes and quality standards.

Thank you very much for your very detailed review and the time you took to make it.

I very much appreciate it.

The new SRPM and spec file have been uploaded to the URLs above (copied here) with all the changes from the comments here.

- Spec URL: http://qore.org/srpms/qore.spec
- SRPM URL: http://qore.org/srpms/qore-0.8.11-1.fc20.src.rpm

thanks,
David

https://fedoraproject.org/wiki/Packaging:FrequentlyMadeMistakes

| Increase the "Release" tag every time you upload a new package to avoid
| confusion. The reviewer and other interested parties probably still have
| older versions of your SRPM lying around to check what has changed between
| the old and new packages; those get confused when the revision didn't change. 

The %changelog also doesn't document any of the changes you've supplied.
https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs


> mv $RPM_BUILD_DIR/%{name}-%{version}/test $RPM_BUILD_DIR/%{name}-%{version}/examples

"mv test examples" should suffice.

At the beginning of each of the main spec file sections, you are within the primary builddir already as specified via %setup (or its default -n %name-%version).


> %configure --disable-debug --disable-static

This belongs at the beginning of the %build section.
Also see "rpm -E %configure".


> %build
> %{__make}

https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make


> %package doc
> Summary: API documentation, programming language reference, and Qore example programs
> Group: Development/Languages

Rather "Group: Documentation" unless you want to drop the Group tag altogether:
https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag

> Requires: libqore%{?_isa} = %{version}-%{release}

Plain documentation packages (which contain files that can be displayed with arbitrary HTML/PDF viewers) typically do not need to depend on base libraries, or else you could not install the documentation without pulling in dependency bloat.


> %package -n libqore
> Summary: The libraries for the qore runtime and qore clients
> Group: Development/Languages

The Group tag for runtime library base packages has been "System Environment/Libraries" for many years.


> %package devel
> Summary: The header files needed to compile programs using the qore library
> Group: Development/Languages

The Group tag for build-time library -devel packages has been "Development/Libraries" for many years.


> Provides: qore-module-api-0.18
> Provides: qore-module-api-0.17
> Provides: qore-module-api-0.16
> Provides: qore-module-api-0.15
> Provides: qore-module-api-0.14
> Provides: qore-module-api-0.13
> Provides: qore-module-api-0.12
> Provides: qore-module-api-0.11
> Provides: qore-module-api-0.10
> Provides: qore-module-api-0.9
> Provides: qore-module-api-0.8
> Provides: qore-module-api-0.7
> Provides: qore-module-api-0.6
> Provides: qore-module-api-0.5

Odd. And rather limited. You could not do "Requires: qore-module-api >= 0.10", for example. Why not

  Provides: qore-module(api) = 0.5
  Provides: qore-module(api) = 0.6
  Provides: qore-module(api) = 0.7
  ...

and so on?

[...]

Have you pointed the fedora-review tool at this ticket yet?
"fedora-review -b 1111691"

> %package doc

Plus, documentation very often is not arch-specific, so a -doc subpackage could be "BuildArch: noarch". In that case, an arch-specific dependency on a library base package would not be possibly anyway.

(In reply to Michael Schwendt from comment #7)
> https://fedoraproject.org/wiki/Packaging:FrequentlyMadeMistakes
> 
> | Increase the "Release" tag every time you upload a new package to avoid
> | confusion. The reviewer and other interested parties probably still have
> | older versions of your SRPM lying around to check what has changed between
> | the old and new packages; those get confused when the revision didn't
> change. 

done - new URLs below

> 
> The %changelog also doesn't document any of the changes you've supplied.
> https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs
> 

ok done

> 
> > mv $RPM_BUILD_DIR/%{name}-%{version}/test $RPM_BUILD_DIR/%{name}-%{version}/examples
> 
> "mv test examples" should suffice.
> 
> At the beginning of each of the main spec file sections, you are within the
> primary builddir already as specified via %setup (or its default -n
> %name-%version).
> 

ok, thanks, done

> 
> > %configure --disable-debug --disable-static
> 
> This belongs at the beginning of the %build section.
> Also see "rpm -E %configure".
> 

done

> 
> > %build
> > %{__make}
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make
> 
> 

ok done

> > %package doc
> > Summary: API documentation, programming language reference, and Qore example programs
> > Group: Development/Languages
> 
> Rather "Group: Documentation" unless you want to drop the Group tag
> altogether:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag
> 

ok done

> > Requires: libqore%{?_isa} = %{version}-%{release}
> 
> Plain documentation packages (which contain files that can be displayed with
> arbitrary HTML/PDF viewers) typically do not need to depend on base
> libraries, or else you could not install the documentation without pulling
> in dependency bloat.
> 
> 

ok done

> > %package -n libqore
> > Summary: The libraries for the qore runtime and qore clients
> > Group: Development/Languages
> 
> The Group tag for runtime library base packages has been "System
> Environment/Libraries" for many years.
> 
> 

ok fixed

> > %package devel
> > Summary: The header files needed to compile programs using the qore library
> > Group: Development/Languages
> 
> The Group tag for build-time library -devel packages has been
> "Development/Libraries" for many years.
> 
> 

ok done

> > Provides: qore-module-api-0.18
> > Provides: qore-module-api-0.17
> > Provides: qore-module-api-0.16
> > Provides: qore-module-api-0.15
> > Provides: qore-module-api-0.14
> > Provides: qore-module-api-0.13
> > Provides: qore-module-api-0.12
> > Provides: qore-module-api-0.11
> > Provides: qore-module-api-0.10
> > Provides: qore-module-api-0.9
> > Provides: qore-module-api-0.8
> > Provides: qore-module-api-0.7
> > Provides: qore-module-api-0.6
> > Provides: qore-module-api-0.5
> 
> Odd. And rather limited. You could not do "Requires: qore-module-api >=
> 0.10", for example. Why not
> 
>   Provides: qore-module(api) = 0.5
>   Provides: qore-module(api) = 0.6
>   Provides: qore-module(api) = 0.7
>   ...
> 
> and so on?
> 
> [...]

ok, this makes sense.  The reason I did not do it like this before is because I did not come up with this solution when I was first researching this topic.

however there are already a set of dependent module RPMs out in the wild (for Fedora, RHEL, and other distributions) that assume the old non-versioned Provides: are available.

I would be happy to change it, because I agree that it looks better/cleaner, however I'm afraid of breaking the existing RPMs.

So I'm not sure what to do here - any advice you can give would be greatly appreciated.

> 
> Have you pointed the fedora-review tool at this ticket yet?
> "fedora-review -b 1111691"

I'm running it now against the new SRPM and spec file with: "fedora-review -n qore".  It's still running at the moment, so I'll react when I get some output.

Thanks for this tip; I did not know about this until now.

Thanks a lot for your in-depth review and excellent constructive comments.  My packaging knowledge is slowly improving with the state of the qore packaging for Fedora.

URLs with updated spec and new SRPM:
- Spec URL: http://qore.org/srpms/qore.spec
- SRPM URL: http://qore.org/srpms/qore-0.8.11-2.fc20.src.rpm

thanks,
David

(In reply to Michael Schwendt from comment #8)
> > %package doc
> 
> Plus, documentation very often is not arch-specific, so a -doc subpackage
> could be "BuildArch: noarch". In that case, an arch-specific dependency on a
> library base package would not be possibly anyway.

thanks, excellent tip, this was also done in revision 2 as in the links above.

Also I finished running fedora-review on the updated SRPM and spec, and as far as I can see from that output the only thing left to resolve (besides what I hope are minor/acceptable rpmlint warnings) is the Provides: lines for the library ABIs.  As I mentioned before, I'm not sure what to do about those without causing problems with older module RPMs.

thanks,
David

(In reply to Michael Schwendt from comment #7)
> Odd. And rather limited. You could not do "Requires: qore-module-api >=
> 0.10", for example. Why not
> 
>   Provides: qore-module(api) = 0.5
>   Provides: qore-module(api) = 0.6
>   Provides: qore-module(api) = 0.7
>   ...
> 
> and so on?

Also I have to say that this would have just a cosmetic benefit.  Module APIs (in reality they are ABIs) are not necessarily backwards-compatible.  

Each module will always only declare one Requires: line for the particular ABI it needs.  It's libqore that will declare all the ABIs it supports with the Provides: declarations that allow the RPM system to manage the dependencies properly.

Therefore something like:
Requires: qore-module-api >= 0.10

would not be used anyway, since 0.11 or higher may not be compatible with 0.10.  What the RPM system needs is to know that ABI 0.10 is supported by libqore, which is provided by the Provides: line (now Provides: qore-module-api-0.10).

So this is a much better explanation of why I'm using unversioned artificial dependencies.

I had gone through this so many years ago that I had forgotten the reasoning behind it until I thought it through again now.

thanks,
David

Versioned Provides do not imply that you must use '>='. It would be perfectly acceptable to only use '=' in dependencies.

  $ rpm -q --provides audacious-libs|grep api
  audacious(plugin-api) = 40
  audacious(plugin-api) = 41
  audacious(plugin-api) = 42
  audacious(plugin-api) = 43
  audacious(plugin-api)(x86-64) = 40
  audacious(plugin-api)(x86-64) = 41
  audacious(plugin-api)(x86-64) = 42
  audacious(plugin-api)(x86-64) = 43

  $ repoquery --whatrequires 'audacious(plugin-api)(x86-64)'|wc -l
  19

All require the latest API version 43:

  $ repoquery --whatrequires 'audacious(plugin-api)(x86-64) == 43'|wc -l
  19

Versioned capabilities, however, allow for versioned queries with "rpm", repoquery and other package tools (if not considering Requires, Obsoletes, Conflicts and BuildRequires):

Does anything in the repos support a newer plugin API already? For example, the next development release in alternative packages?

  $ repoquery --whatprovides 'audacious(plugin-api)(x86-64) > 43'|wc -l 
  0

Apparently not.

Does anything in the repos support an older plugin API? For example, a compat package?

  $ repoquery --whatprovides 'audacious(plugin-api)(x86-64) < 40'|wc -l
  0

Apparently not.

Assume we need to drop support for API 41 and older, does anything in the repo require any old Plugin API?

  $ repoquery --whatrequires 'audacious(plugin-api)(x86-64) <= 41' |wc -l
  0

Apparent not.

Assume the next release drops support for API older than 50, does anything in the repo still require any older API?

  $ repoquery --whatrequires 'audacious(plugin-api)(x86-64) < 50'|wc -l
  19

All, but 3rd party repos not included in the query.

[...]

I don't claim the "Provides: qore-module-api-0.18" is not sufficient for what it is being used so far -- an exact dependency on that thing. There are 14 such Provides in the package already. There is a version in them, but in the wrong place. That's unusual and inconvenient.

What to do about it also depends on 3rd party packages that already use these strict dependencies and how many shall be included in the package collection. In that case, proper Obsoletes tags would need to be added anyway. There could also be a transition to versioned Provides only for new API versions, while the old ones would be kept as long as they will still be supported.

[...]

Any comment on fedora-review licensecheck.txt and rpmlint.txt?

Plus:

* https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

(In reply to Michael Schwendt from comment #12)
> Versioned Provides do not imply that you must use '>='. It would be
> perfectly acceptable to only use '=' in dependencies.
> 
>   $ rpm -q --provides audacious-libs|grep api
>   audacious(plugin-api) = 40
>   audacious(plugin-api) = 41
>   audacious(plugin-api) = 42
>   audacious(plugin-api) = 43
>   audacious(plugin-api)(x86-64) = 40
>   audacious(plugin-api)(x86-64) = 41
>   audacious(plugin-api)(x86-64) = 42
>   audacious(plugin-api)(x86-64) = 43
> 
>   $ repoquery --whatrequires 'audacious(plugin-api)(x86-64)'|wc -l
>   19
> 
> All require the latest API version 43:
> 
>   $ repoquery --whatrequires 'audacious(plugin-api)(x86-64) == 43'|wc -l
>   19
> 
> Versioned capabilities, however, allow for versioned queries with "rpm",
> repoquery and other package tools (if not considering Requires, Obsoletes,
> Conflicts and BuildRequires):
> 
> Does anything in the repos support a newer plugin API already? For example,
> the next development release in alternative packages?
> 
>   $ repoquery --whatprovides 'audacious(plugin-api)(x86-64) > 43'|wc -l 
>   0
> 
> Apparently not.
> 
> Does anything in the repos support an older plugin API? For example, a
> compat package?
> 
>   $ repoquery --whatprovides 'audacious(plugin-api)(x86-64) < 40'|wc -l
>   0
> 
> Apparently not.
> 
> Assume we need to drop support for API 41 and older, does anything in the
> repo require any old Plugin API?
> 
>   $ repoquery --whatrequires 'audacious(plugin-api)(x86-64) <= 41' |wc -l
>   0
> 
> Apparent not.
> 
> Assume the next release drops support for API older than 50, does anything
> in the repo still require any older API?
> 
>   $ repoquery --whatrequires 'audacious(plugin-api)(x86-64) < 50'|wc -l
>   19
> 
> All, but 3rd party repos not included in the query.
> 
> [...]
> 
> I don't claim the "Provides: qore-module-api-0.18" is not sufficient for
> what it is being used so far -- an exact dependency on that thing. There are
> 14 such Provides in the package already. There is a version in them, but in
> the wrong place. That's unusual and inconvenient.
> 
> What to do about it also depends on 3rd party packages that already use
> these strict dependencies and how many shall be included in the package
> collection. In that case, proper Obsoletes tags would need to be added
> anyway. There could also be a transition to versioned Provides only for new
> API versions, while the old ones would be kept as long as they will still be
> supported.
> 
> [...]
>

OK I have updated the package to use the versioned capabilities and declare the old unversioned capabilities obsolete.

the new declarations are as follows:

Provides: qore-module(abi) = 0.18
Obsoletes: libqore5 < 0.8.12
Obsoletes: qore-module-api-0.18
Obsoletes: qore-module-api-0.17
Obsoletes: qore-module-api-0.16
Obsoletes: qore-module-api-0.15
Obsoletes: qore-module-api-0.14
Obsoletes: qore-module-api-0.13
Obsoletes: qore-module-api-0.12
Obsoletes: qore-module-api-0.11
Obsoletes: qore-module-api-0.10
Obsoletes: qore-module-api-0.9
Obsoletes: qore-module-api-0.8
Obsoletes: qore-module-api-0.7
Obsoletes: qore-module-api-0.6
Obsoletes: qore-module-api-0.5

> Any comment on fedora-review licensecheck.txt and rpmlint.txt?

licensecheck.txt:
I missed this one before - there were two GPL files taken from glibc (getopt_long.cpp and getopt_long.h) that I had forgotten about.   These files are not used in the Linux build, but regardless their presence precludes releasing Qore under an optional MIT license.

I have just replaced them with BSD-licensed variants (taken from FreeBSD).  The BSD-licensed versions will be present in the next release.

The mass of files with LGPL headers are from Qore, and this source has been now released under a less restrictive MIT license.

I will update all these source files to reflect the new licensing status and also the option to use them under the GPL and LGPL (Note: the qore library allows itself to be initialized under the GPL which allows it to load binary modules also tagged as under the GPL; if the qore library is not initialized under the GPL then GPL-tagged modules cannot be loaded).

This will take some time, so I will post another comment when the updated source packages are available.

rpmlint.txt:
The spelling errors can be ignored in my opinion - all except "Qore" (name of the project) are valid according to the New Oxford American Dictionary.

+ qore: manual-page-warning: I removed the undefined macros

+ libqore: obsolete-not-provided: I have to admit I do not understand the rpmlint -I explanation for this warning - AFAIK I should declare the old library name as obsoleted by the new one in case anyone has the old version installed

+ libqore: only-non-binary-in-usr-lib: this is due to the Qore-language module files being stored under lib*, this is because they are stored together with the binary modules.  The binary modules were first, then a few versions ago the qore library implemented support for "user modules" which are modules written in Qore instead of C++.  libqore includes the user modules, and they are stored in the same directory as the binary modules.  I had assumed (read: was hoping) that this would be acceptable, but if not, then I can make the appropriate changes to the language and the packaging to package them under /usr/share/qore-modules instead and have two locations for qore modules depending on the module type.

+ qore-devel: no-documentation: all the READMEs, etc were provided with libqore.  The development documentation is provided in devel-doc.  I'm not sure what to do about this - maybe add a README?

+ qore-devel: no-manual-page-for-binary: I have not written man pages for these programs yet - I had assumed from the Fedora packaging guidelines that this was a "nice to have" but not an absolute must.   I am planning on writing man pages for these but do not have them yet.

+ qore.src: unversioned-explicit-provides: as above

> 
> Plus:
> 
> * https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

I re-added these as follows:
%build
export CXXFLAGS="%{optflags}"
%configure --disable-debug --disable-static
make %{?_smp_mflags}

I had misunderstood a review comment from Christopher Meng and $RPM_OPT_FLAGS before - I hope the solution above is better and future-proof.

It will take me a while to update the source with the updated license info, after which I'll post the URLs to the updated revision 3 sources for the qore packages.

Thanks again for your time and the detailed help with the review process.

thanks,
David

> obsolete-not-provided

First of all, while you can put anything in an Obsoletes tag, only package names (with/without a version-release range) will actually result in the specified package(s) getting replaced with another package (or multiple ones). 

Further, anything declared as obsolete breaks existing dependencies on it. Unless there is a corresponding "Provides" for the obsolete thing.

> Obsoletes: libqore5 < 0.8.12

If there's a "Requires: libqore5 …" anywhere, there would be a broken dependency, regardless of whether "libqore5" is a physical package %name, a virtual package name, or something else as a "Provides" tag.

http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages

> Obsoletes: qore-module-api-0.18

Same here. This capability will be "gone", "hidden" from the depsolver, and anything that still depends on it will cause a broken dependency. The package that contains this Obsoletes tag does not implicitly "Provides" the obsolete thing.

(In reply to Michael Schwendt from comment #14)
> > obsolete-not-provided
> 
> First of all, while you can put anything in an Obsoletes tag, only package
> names (with/without a version-release range) will actually result in the
> specified package(s) getting replaced with another package (or multiple
> ones). 
> 
> Further, anything declared as obsolete breaks existing dependencies on it.
> Unless there is a corresponding "Provides" for the obsolete thing.
> 
> > Obsoletes: libqore5 < 0.8.12
> 
> If there's a "Requires: libqore5 …" anywhere, there would be a broken
> dependency, regardless of whether "libqore5" is a physical package %name, a
> virtual package name, or something else as a "Provides" tag.
> 
> http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.
> 2FReplacing_Existing_Packages
> 
> > Obsoletes: qore-module-api-0.18
> 
> Same here. This capability will be "gone", "hidden" from the depsolver, and
> anything that still depends on it will cause a broken dependency. The
> package that contains this Obsoletes tag does not implicitly "Provides" the
> obsolete thing.

OK, I tried to read about this stuff, but I did not see this covered in any of the RPM or packaging guides.  I think that I understand now.

So then I will do this instead:

Provides: qore-module(abi) = 0.18
Provides: libqore5 = 0.8.11
Obsoletes: libqore5 < 0.8.12
# provided for backwards-compatibility with unversioned capabilities and will be removed when the ABI drops backwards-compatibility
Provides: qore-module-api-0.18
Provides: qore-module-api-0.17
Provides: qore-module-api-0.16
Provides: qore-module-api-0.15
Provides: qore-module-api-0.14
Provides: qore-module-api-0.13
Provides: qore-module-api-0.12
Provides: qore-module-api-0.11
Provides: qore-module-api-0.10
Provides: qore-module-api-0.9
Provides: qore-module-api-0.8
Provides: qore-module-api-0.7
Provides: qore-module-api-0.6
Provides: qore-module-api-0.5

Furthermore I hope to have the license text updated in the source by later today (CET).

Thanks again for your explanations and patience.

thanks,
David

(In reply to David Nichols from comment #15)
> (In reply to Michael Schwendt from comment #14)
> > > obsolete-not-provided
> > 
> > First of all, while you can put anything in an Obsoletes tag, only package
> > names (with/without a version-release range) will actually result in the
> > specified package(s) getting replaced with another package (or multiple
> > ones). 
> > 
> > Further, anything declared as obsolete breaks existing dependencies on it.
> > Unless there is a corresponding "Provides" for the obsolete thing.
> > 
> > > Obsoletes: libqore5 < 0.8.12
> > 
> > If there's a "Requires: libqore5 …" anywhere, there would be a broken
> > dependency, regardless of whether "libqore5" is a physical package %name, a
> > virtual package name, or something else as a "Provides" tag.
> > 
> > http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.
> > 2FReplacing_Existing_Packages
> > 
> > > Obsoletes: qore-module-api-0.18
> > 
> > Same here. This capability will be "gone", "hidden" from the depsolver, and
> > anything that still depends on it will cause a broken dependency. The
> > package that contains this Obsoletes tag does not implicitly "Provides" the
> > obsolete thing.
> 
> OK, I tried to read about this stuff, but I did not see this covered in any
> of the RPM or packaging guides.  I think that I understand now.
> 
> So then I will do this instead:
> 
> Provides: qore-module(abi) = 0.18
> Provides: libqore5 = 0.8.11
> Obsoletes: libqore5 < 0.8.12
> # provided for backwards-compatibility with unversioned capabilities and
> will be removed when the ABI drops backwards-compatibility
> Provides: qore-module-api-0.18
> Provides: qore-module-api-0.17
> Provides: qore-module-api-0.16
> Provides: qore-module-api-0.15
> Provides: qore-module-api-0.14
> Provides: qore-module-api-0.13
> Provides: qore-module-api-0.12
> Provides: qore-module-api-0.11
> Provides: qore-module-api-0.10
> Provides: qore-module-api-0.9
> Provides: qore-module-api-0.8
> Provides: qore-module-api-0.7
> Provides: qore-module-api-0.6
> Provides: qore-module-api-0.5

This did not work because libqore then provides a capability that it's obsoleting.

Therefore I will update the version to 0.8.11.1 and then do the following:

Provides: qore-module(abi) = 0.18
Provides: libqore5 = %{version}
Obsoletes: libqore5 < %{version}
# provided for backwards-compatibility with unversioned capabilities and will be removed when the ABI drops backwards-compatibility
Provides: qore-module-api-0.18
Provides: qore-module-api-0.17
Provides: qore-module-api-0.16
Provides: qore-module-api-0.15
Provides: qore-module-api-0.14
Provides: qore-module-api-0.13
Provides: qore-module-api-0.12
Provides: qore-module-api-0.11
Provides: qore-module-api-0.10
Provides: qore-module-api-0.9
Provides: qore-module-api-0.8
Provides: qore-module-api-0.7
Provides: qore-module-api-0.6

I can also remove the unversioned Provides if it's a problem, I just would like to keep backwards compatibility with existing installed modules if possible, but if it's not acceptable for Fedora, then I will remove the lines.

I should have the new release ready in a few minutes.

> This did not work [...]

So-called "self-Obsoletes" do work, because the same package still Provides what it Obsoletes.

Self-Obsoletes sometimes are used to replace multiple packages (of no specific arch) with a single new package.

> Provides: libqore5 = 0.8.11
> Obsoletes: libqore5 < 0.8.12

Anything that "Requires: libqore5 <= 0.8.11" would still have worked, because it is still provided.

> Provides: libqore5 = %{version}
> Obsoletes: libqore5 < %{version}

This is cleaner, of course. Though, typically one hardcodes a specific maximum version-release in the Obsoletes tag to be really accurate and not obsolete more than necessary (e.g. if %version increments, the Obsoletes tag would adjust and also obsolete newer versions than what had been specified originally).

(In reply to Michael Schwendt from comment #17)
> > This did not work [...]
> 
> So-called "self-Obsoletes" do work, because the same package still Provides
> what it Obsoletes.
> 
> Self-Obsoletes sometimes are used to replace multiple packages (of no
> specific arch) with a single new package.
> 
> > Provides: libqore5 = 0.8.11
> > Obsoletes: libqore5 < 0.8.12
> 
> Anything that "Requires: libqore5 <= 0.8.11" would still have worked,
> because it is still provided.
> 
> > Provides: libqore5 = %{version}
> > Obsoletes: libqore5 < %{version}
> 
> This is cleaner, of course. Though, typically one hardcodes a specific
> maximum version-release in the Obsoletes tag to be really accurate and not
> obsolete more than necessary (e.g. if %version increments, the Obsoletes tag
> would adjust and also obsolete newer versions than what had been specified
> originally).

OK I have it like this now:
Provides: qore-module(abi) = 0.19
Provides: qore-module(abi) = 0.18
Provides: libqore5 = %{version}
Obsoletes: libqore5 < 0.8.11.1

I also updated the module (and library) ABI because I moved the user (text) modules to $(datarootdir)/qore-modules (ie /usr/share/qore-modules/...) to address one of the fedora-review issues, and for this I also added additional symbols to the library.

However now I'm getting a new error in fedora-review as follows:
libqore.x86_64: E: useless-provides qore-module(abi)

Any clue what I'm doing wrong here?

(In reply to David Nichols from comment #18)
> OK I have it like this now:
> Provides: qore-module(abi) = 0.19
> Provides: qore-module(abi) = 0.18
> Provides: libqore5 = %{version}
> Obsoletes: libqore5 < 0.8.11.1
> 
> I also updated the module (and library) ABI because I moved the user (text)
> modules to $(datarootdir)/qore-modules (ie /usr/share/qore-modules/...) to
> address one of the fedora-review issues, and for this I also added
> additional symbols to the library.
> 
> However now I'm getting a new error in fedora-review as follows:
> libqore.x86_64: E: useless-provides qore-module(abi)

I also checked https://apps.fedoraproject.org/packages/audacious/sources/spec and see that audacious-libs only uses one versioned Provides: declaration for audacious(plugin-api) (they also use %{?_isa} in the declaration, which I've added).

So basically the question is if:
Provides: qore-module(abi)%{?_isa} = 0.19
%{?_isa:Provides: qore-module(abi) = 0.19}
Provides: qore-module(abi)%{?_isa} = 0.18
%{?_isa:Provides: qore-module(abi) = 0.18}

is OK despite the error in fedora-review (libqore.x86_64: E: useless-provides qore-module(abi)).  I found another fedora package (rubygem-em-socksify) that passed review with this error, so maybe it's ok - not totally clear to me why it's not a warning if under some circumstances it's OK.

Anyway, assuming it's OK, the new URLs are:
Spec URL: http://qore.org/srpms/qore.spec
SRPM URL: http://qore.org/srpms/qore-0.8.11.1-1.fc20.src.rpm

Changes:
- added explicit versioned capability for library ABI compatibility for module RPMs
- added explicit versioned capability for libqore5 due to name change on fedora/rhel
- obsoletes previous versions of libqore5 in case of foreign RPM installation
- added %%{optflags} to configure
- updated license text in library source to reflect most liberal license option (MIT) with reference to LGPL and GPL options
- replaced GPL getopt_long.* files with BSD variants (not used on Linux builds)
- updated module and library ABI info
- moved user module directory to ${_datarootdir}
- moved module and user module directories to libqore package where they should be
- disabled dependency tracking in configure

The upstream source has been significantly updated with the license text updates as mentioned above.

There are still some GPL files included in licensecheck.txt: ltmain.sh (included by autotools from libtool) and parser.cpp/parser.hpp which are generated by Bison and subject to the Bison "special exception".

there are some rpmlint warnings still present, but I hope they are acceptable (the spelling ones are definitely OK I believe).  There are missing man pages for two programs used in C++ development for libqore, and the devel documentation is in a separate package due to its size, so there are no docs in the devel package.

> libqore.x86_64: E: useless-provides qore-module(abi)

rpmlint is mistaken in this case. A bug report had been filed years ago in bug 460872, but it has been closed due to a misunderstanding.


> Provides: qore-module(abi)%{?_isa} = 0.19

At some time, adding %{?_isa} to create arch-specific dependencies has become popular and can also help the depsolver. For example, when the depsolver encounters broken deps in a multiarch repo such as x86_64, it would try to resolve the deps with i686 packages (even older ones), which sometimes leads to pulling in lots of i686 packages.

If you choose to add %{?_isa}, there is not much reason to also add the non-%isa Provides. 

The audacious packages had started with non-%isa Provides, which should be dropped nowadays since nothing depends on them anymore.

(In reply to Michael Schwendt from comment #20)
> > libqore.x86_64: E: useless-provides qore-module(abi)
> 
> rpmlint is mistaken in this case. A bug report had been filed years ago in
> bug 460872, but it has been closed due to a misunderstanding.

ok thanks for that info.

> > Provides: qore-module(abi)%{?_isa} = 0.19
> 
> At some time, adding %{?_isa} to create arch-specific dependencies has
> become popular and can also help the depsolver. For example, when the
> depsolver encounters broken deps in a multiarch repo such as x86_64, it
> would try to resolve the deps with i686 packages (even older ones), which
> sometimes leads to pulling in lots of i686 packages.
> 
> If you choose to add %{?_isa}, there is not much reason to also add the
> non-%isa Provides. 
> 
> The audacious packages had started with non-%isa Provides, which should be
> dropped nowadays since nothing depends on them anymore.

ok I see that makes sense - I also had the same thought, but thought it was safer to do it just like an already-accepted package.

new URLs:
Spec URL: http://qore.org/srpms/qore.spec
SRPM URL: http://qore.org/srpms/qore-0.8.11.1-2.fc20.src.rpm

only the qore-module(abi) Provides were updated to remove the non-%isa versions.

Michael, thanks a lot for your time, expertise, and help with this package submission review.

Are these obsoletes lines needed? These are not available in the official repo but only in the qore 3rd party yum repo.

(In reply to Christopher Meng from comment #22)
> Are these obsoletes lines needed? These are not available in the official
> repo but only in the qore 3rd party yum repo.

There is only Obsoletes line in the most recent spec file:
Obsoletes: libqore5 < 0.8.11.1

This is needed for people that have the 3rd-party rpm installed, since now the name will have changed since people should use the official yum repo in the future since it will ideally be fed directly from upstream.

updated URLS:

new URLs:
Spec URL: http://qore.org/srpms/qore.spec
SRPM URL: http://qore.org/srpms/qore-0.8.11.1-3.fc20.src.rpm

changes:
- added license files and license READMEs to packages that can potentially be installed independently (doc and devel-doc)
- removed --disable-static from the configure call since it's the default
- created a new qore-stdlib package for noarch user module files in /usr/share, split from libqore
- removed ChangeLog from distribution sources

note that the source has not yet been uploaded to sourceforge - I will make the upstream release once I'm sure no more changes are needed for the package to be accepted in Fedora.

Jason Taylor (jason.taylor) has offered to co-maintain this package when it's approved as well.

koji builds of qore-0.8.11.1-3:

f20:
http://koji.fedoraproject.org/koji/taskinfo?taskID=7082844

rawhide:
http://koji.fedoraproject.org/koji/taskinfo?taskID=7083028

Could someone please let me know if the latest package attempt is OK?

I've taken care of all issues to the best of my knowledge with Release 3...

I did my first informal review here:
https://bugzilla.redhat.com/show_bug.cgi?id=1116548#c2

another informal package review:
https://bugzilla.redhat.com/show_bug.cgi?id=1117022#c1

updated URLS:

new URLs:
Spec URL: http://qore.org/srpms/qore.spec
SRPM URL: http://qore.org/srpms/qore-0.8.11.1-4.fc20.src.rpm

changes:
- added a %%check section using the new "make check" target
- simplified spec due to upstream changes (moved test/ subdir to examples/ in upstream)

koji builds of qore-0.8.11.1-4 for f21:
http://koji.fedoraproject.org/koji/taskinfo?taskID=7118205

I found some bugs in external module building related to changes made for Fedora packaging in this package, therefore I have updated URLS:

new URLs:
Spec URL: http://qore.org/srpms/qore.spec
SRPM URL: http://qore.org/srpms/qore-0.8.11.1-5.fc20.src.rpm

changes:
- synced with upstream fixes for 64 bit ppc compilation and command-line enhancements for module directory handling

koji builds of qore-0.8.11.1-5 for f21:
http://koji.fedoraproject.org/koji/taskinfo?taskID=7119247

Upon further manual review of the RPMS, a packaging bug in the file specs in the qore-doc package was found which is fixed in the new release:

Spec URL: http://qore.org/srpms/qore.spec
SRPM URL: http://qore.org/srpms/qore-0.8.11.1-6.fc20.src.rpm

changes:
- fixed doc and devel-doc file specs to fix packaging bugs for documentation

koji builds of qore-0.8.11.1-6 for f21:
http://koji.fedoraproject.org/koji/taskinfo?taskID=7131746

another informal review:
https://bugzilla.redhat.com/show_bug.cgi?id=1121601#c1

I'm not a sponsor, but the package looks good to me, I see no errors.

(In reply to Adam Williamson (Red Hat) from comment #33)
> I'm not a sponsor, but the package looks good to me, I see no errors.

thanks Adam, I appreciate you taking a look at it.

If I understand correctly, I should email sponsors individually and ask them if they would care to sponsor this package and me as a maintainer, is that correct?

that would be a good way to try and find one, yeah - maybe look at their packages / reviews and see if you can find one who's interested in this general area of work. You could also ping folks on IRC. sorry for the delay in getting this approved!

the scm repo has moved to github; the build has been updated accordingly:

Spec URL: http://qore.org/srpms/qore.spec
SRPM URL: http://qore.org/srpms/qore-0.8.11.1-7.fc22.src.rpm

koji builds of qore-0.8.11.1-6 for f23:
http://koji.fedoraproject.org/koji/taskinfo?taskID=10995416

I'm back to actively looking for a sponsor.

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

I am still interested and will update the specfile and src.rpm

I see this has been hanging around for a while so I'll take it. David, if you still need a sponsor, be sure to block the FE-NEEDSPONSOR bug report.

I'll wait a bit for a new SPEC and SRPM so take your time :).

(In reply to Andy Mender from comment #39)
> I see this has been hanging around for a while so I'll take it. David, if
> you still need a sponsor, be sure to block the FE-NEEDSPONSOR bug report.
> 
> I'll wait a bit for a new SPEC and SRPM so take your time :).

thanks Andy

Here are the updated files:
- https://docs.qore.org/srpms/qore.spec
- https://docs.qore.org/srpms/qore-0.9.4.5-1.fc32.src.rpm

related Koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=48178539

> %global module_dir %{_libdir}/qore-modules
> %global user_module_dir %{_datarootdir}/qore-modules/

Not a requirement, but %{_datadir} resolves to the same directory - "/usr/share".

> Summary: Multithreaded Programming Language
> Name: qore
> Version: 0.9.4.5
> Release: 1%{?dist}
> License: LGPLv2+ or GPLv2+ or MIT
> Group: Development/Languages
> URL: http://qore.org
> Source0: https://github.com/qorelanguage/qore/releases/download/release-%{version}/%{name}-%{version}.tar.bz2
> Requires: /usr/bin/env
> BuildRequires: gcc-c++
> BuildRequires: flex >= 2.5.31
> BuildRequires: bison
> BuildRequires: openssl-devel
> BuildRequires: pcre-devel
> BuildRequires: zlib-devel
> BuildRequires: gmp-devel
> BuildRequires: mpfr-devel
> BuildRequires: doxygen
> BuildRequires: pkgconfig
> BuildRequires: bzip2-devel

Not a strict requirement, but I would split the initial tags into blocks and improve formatting a bit:
%global         module_dir %{_libdir}/qore-modules
%global         user_module_dir %{_datarootdir}/qore-modules/

Name:           qore
Version:        0.9.4.5
Release:        1%{?dist}
Summary:        Multithreaded Programming Language

License:        LGPLv2+ or GPLv2+ or MIT
Group:          Development/Languages
URL:            http://qore.org
Source0:        https://github.com/qorelanguage/qore/releases/download/release-%{version}/%{name}-%{version}.tar.bz2

Requires:       /usr/bin/env
BuildRequires:  gcc-c++
BuildRequires: 	flex >= 2.5.31
BuildRequires: 	bison
BuildRequires: 	openssl-devel
BuildRequires: 	pcre-devel
BuildRequires: 	zlib-devel
BuildRequires: 	gmp-devel
BuildRequires: 	mpfr-devel
BuildRequires: 	doxygen
BuildRequires: 	pkgconfig
BuildRequires: 	bzip2-devel

> License:        LGPLv2+ or GPLv2+ or MIT
> Group:          Development/Languages

- I would add a comment above the License block with this link: https://github.com/qorelanguage/qore/blob/develop/README-LICENSE
  to make it clear that depending on the license choice, different modules are enabled or disabled. `licensecheck` complains a great deal, unfortunately.
- The Group: tag is obsolete and should be removed.
- "/usr/bin/env" is a part of the coreutils package, but since it's a part of the base system, this Requires can probably be removed.
- Check which of the BuildRequires for -devel package can be replaced with pkgconfig(foo). Relevant section of the Packaging Guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/

The main package should probably contain Requires like these to make sure all of the subpackages are correctly linked to the main package:
Requires: %{name}-libqore%{?_isa} = %{version}-%{release}
Requires: %{name}-stdlib%{?_isa} = %{version}-%{release}
etc.

> %package -n libqore
> Summary: The libraries for the qore runtime and qore clients
> Group: System Environment/Libraries
> Provides: qore-module(abi)%{?_isa} = 0.23
> Provides: qore-module(abi)%{?_isa} = 0.22

- The Group tag is obsolete and should be removed.
- rpmlint complains about the abi provides: libqore.x86_64: E: useless-provides qore-module(abi)(x86-64)
  I couldn't find anything explicit in the Packaging Guidelines, but perhaps you can remove these?

> Provides: libqore6 = %{version}
> Obsoletes: libqore6 < %{version}

I'm wondering about these lines. Neither qore, nor any version of libqore is in the repositories so I don't think these are needed.

> %files -n libqore
> %{_libdir}/libqore.so.6.2.0
> %{_libdir}/libqore.so.6
> %doc COPYING.LGPL COPYING.GPL COPYING.MIT README.md README-LICENSE README-MODULES RELEASE-NOTES AUTHORS ABOUT

The COPYING.* files are license files and they should be tagged with the %license macro.

> %post -n libqore -p /sbin/ldconfig

> %postun -n libqore -p /sbin/ldconfig

Running ldconfig is no longer needed and these lines should be removed.

> %package stdlib
> Summary: Standard library modules
> Group: System Environment/Libraries
> Requires: libqore = %{version}-%{release}
> Requires: qore-module(abi)%{?_isa} = 0.23

- Group tags are obsolete and should be removed.
- Use a fully qualified version Requires like so:
  Requires: %{name}-libqore%{?_isa} = %{version}-%{release}
- The abi requirement can probably be removed.

> %files stdlib
> %{user_module_dir}
> %{module_dir}
> %doc COPYING.MIT README-LICENSE

- The %{module_dir} global definition should end with a slash to make the package own the directory like it's done in %{user_module_dir}.
- COPYING.MIT is a license file and should be listed with the %license macro.

> %files doc
> %doc docs/lang docs/modules/* examples/ COPYING.LGPL COPYING.GPL COPYING.MIT README-LICENSE

The COPYING.* files are license files and should be listed with the %license macro.

> %dir %{_libdir}/cmake
> %{_libdir}/cmake/Qore
> %{_includedir}/*

- Should this package own the /usr/lib/cmake dir? That doesn't sound right. I would use the following instead:
  %{_libdir}/cmake/Qore/

- %{_includedir}/* is a really bad idea. Maybe it would be better to have something like this?
  %{_includedir/qore/

> %files devel-doc
> %doc docs/library/html/* COPYING.LGPL COPYING.GPL COPYING.MIT README-LICENSE

The COPYING.* files are license files and should be listed with the %license macro.

> %package misc-tools
> Summary: Miscellaneous user tools writen in Qore Programming Language
> License: LGPL-2.0+ or GPL-2.0+ or MIT
> Group: Development/Tools/Other
> Requires: qore = %{version}-%{release}
> BuildArch: noarch

- The License block contains invalid license definitions. Use the same ones as listed for the main package.
- Group tags are obsolete and should be removed.
- Use a fully qualified version Requires like so:
  Requires: %{name}-qore%{?_isa} = %{version}-%{release}
  Although I'm not sure whether here it makes sense if the main qore package were to explicitly Requires qore-misc-tools like I suggested earlier.

> %files misc-tools
> %defattr(-,root,root,-)

Use of %defattr is obsolete and should be removed.

> %build
> export CXXFLAGS="%{optflags}"

> %files
> %{_bindir}/qore
> %{_bindir}/qdbg
> %{_bindir}/qdbg-server
> %{_bindir}/qdbg-remote
> %{_bindir}/qdbg-vsc-adapter
> %{_mandir}/man1/qore.1.*

Do the individual subpackages have their own manpages?
> %configure --disable-debug --disable-dependency-tracking
> make %{?_smp_mflags}

Use %make_build instead of running make and supplying the smp flags.

> %install
> make install prefix=%{_prefix} DESTDIR=$RPM_BUILD_ROOT

- See whether it's possible to replace this entire "make instalL" call with %make_install.
- Also, add the "-p" flag to %make_install to preserve timestamps.

Here's the full review matrix for further reference (it's not flawless, so some items might not be 100% accurate):
Package Review
==============

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


Issues:
=======
- ldconfig not called in %post and %postun for Fedora 28 and later.
  Note: /sbin/ldconfig called in libqore
  See: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets
- 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.
  Note: License file COPYING.GPL is not marked as %license
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/LicensingGuidelines/#_license_text
- 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/amender/rpmbuild/SPECS/qore/qore/diff.txt
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/


===== 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]: Header files in -devel subpackage, if present.
[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 successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
     Note: Using prebuilt packages
[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", "GNU Lesser General Public License",
     "*No copyright* Expat License", "Expat License BSD 2-clause
     "Simplified" License", "Expat License", "ISC License BSD 2-clause
     "Simplified" License", "ISC License", "GPL (v3 or later)", "BSD
     2-clause "Simplified" License", "FSF All Permissive License", "FSF
     Unlimited License (with Retention) GNU General Public License (v2)",
     "FSF Unlimited License (with Retention)", "NTP License", "Boost
     Software License 1.0". 29034 files have unknown license. Detailed
     output of licensecheck in
     /home/amender/rpmbuild/SPECS/qore/qore/licensecheck.txt
     Review: discussed before.
[!]: License file installed when any subpackage combination is installed.
     Review: Use %license macros where mentioned.
[!]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[!]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/share/qore
[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/qore
[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.
[x]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[?]: 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.
[!]: Package does not generate any conflict.
     Review: see earlier comments about the use of %{_includedir}.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
     Review: Obsoletes and Provides added, but not needed.
[!]: Requires correct, justified where necessary.
     Review: see comments on /usr/bin/env
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[?]: 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 256000 bytes in 11 files.
[!]: Package complies to the Packaging Guidelines
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[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:
[x]: Reviewer should test that the package builds in mock.
[-]: 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).
[!]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in libqore
     , qore-stdlib , qore-misc-tools
     Review: see earlier comments.
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Scriptlets must be sane, if used.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[x]: 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.
[!]: Packages should try to preserve timestamps of original installed
     files.
[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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: The placement of pkgconfig(.pc) files are correct.
[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:
[ ]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
     Note: Arch-ed rpms have a total of 4853760 bytes in /usr/share
     Review: Is this an artifact of fedora-review or is an extra qore-data package required?
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.


Rpmlint
-------
Checking: qore-0.9.4.5-1.fc33.x86_64.rpm
          libqore-0.9.4.5-1.fc33.x86_64.rpm
          qore-stdlib-0.9.4.5-1.fc33.x86_64.rpm
          qore-doc-0.9.4.5-1.fc33.noarch.rpm
          qore-devel-0.9.4.5-1.fc33.x86_64.rpm
          qore-devel-doc-0.9.4.5-1.fc33.noarch.rpm
          qore-misc-tools-0.9.4.5-1.fc33.noarch.rpm
          qore-debuginfo-0.9.4.5-1.fc33.x86_64.rpm
          qore-debugsource-0.9.4.5-1.fc33.x86_64.rpm
          qore-0.9.4.5-1.fc33.src.rpm
qore.x86_64: W: spelling-error Summary(en_US) Multithreaded -> Multicolored
qore.x86_64: W: no-manual-page-for-binary qdbg
qore.x86_64: W: no-manual-page-for-binary qdbg-remote
qore.x86_64: W: no-manual-page-for-binary qdbg-server
qore.x86_64: W: no-manual-page-for-binary qdbg-vsc-adapter
libqore.x86_64: W: spelling-error Summary(en_US) qore -> wore, ore, sore
libqore.x86_64: W: spelling-error %description -l en_US Qore -> Ore, Sore, Tore
libqore.x86_64: W: spelling-error %description -l en_US qore -> wore, ore, sore
libqore.x86_64: E: useless-provides qore-module(abi)(x86-64)
qore-devel.x86_64: W: no-documentation
qore-devel.x86_64: W: no-manual-page-for-binary qdx
qore-devel.x86_64: W: no-manual-page-for-binary qpp
qore-misc-tools.noarch: W: spelling-error Summary(en_US) writen -> write, written, writes
qore-misc-tools.noarch: W: invalid-license LGPL-2.0+
qore-misc-tools.noarch: W: invalid-license GPL-2.0+
qore-misc-tools.noarch: W: no-documentation
qore-misc-tools.noarch: W: no-manual-page-for-binary qdp
qore-misc-tools.noarch: W: no-manual-page-for-binary qget
qore-misc-tools.noarch: W: no-manual-page-for-binary rest
qore-misc-tools.noarch: W: no-manual-page-for-binary saprest
qore-misc-tools.noarch: W: no-manual-page-for-binary schema-reverse
qore-misc-tools.noarch: W: no-manual-page-for-binary sfrest
qore-misc-tools.noarch: W: no-manual-page-for-binary sqlutil
qore.src: W: spelling-error Summary(en_US) Multithreaded -> Multicolored
10 packages and 0 specfiles checked; 1 errors, 23 warnings.




Rpmlint (debuginfo)
-------------------
Checking: qore-debuginfo-0.9.4.5-1.fc33.x86_64.rpm
          qore-devel-debuginfo-0.9.4.5-1.fc33.x86_64.rpm
          qore-stdlib-debuginfo-0.9.4.5-1.fc33.x86_64.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
(none): E: no installed packages by name qore-devel-doc
(none): E: no installed packages by name qore
(none): E: no installed packages by name qore-stdlib
(none): E: no installed packages by name qore-misc-tools
(none): E: no installed packages by name qore-debuginfo
(none): E: no installed packages by name qore-debugsource
(none): E: no installed packages by name qore-stdlib-debuginfo
(none): E: no installed packages by name qore-devel
(none): E: no installed packages by name qore-devel-debuginfo
(none): E: no installed packages by name libqore
(none): E: no installed packages by name qore-doc
0 packages and 0 specfiles checked; 0 errors, 0 warnings.



Source checksums
----------------
https://github.com/qorelanguage/qore/releases/download/release-0.9.4.5/qore-0.9.4.5.tar.bz2 :
  CHECKSUM(SHA256) this package     : 13741985900dfbb494f1b6304d1f6fb958de1168bd8a46325a8cc2b90a1de562
  CHECKSUM(SHA256) upstream package : 17c8993f4c26b4ae92ec240176eda5e952c52876d7a424fb3b51dc46360d26c0
diff -r also reports differences


Requires
--------
qore (rpmlib, GLIBC filtered):
    /usr/bin/env
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libqore.so.6()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    rtld(GNU_HASH)

libqore (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    ld-linux-x86-64.so.2()(64bit)
    libbz2.so.1()(64bit)
    libc.so.6()(64bit)
    libcrypto.so.1.1()(64bit)
    libcrypto.so.1.1(OPENSSL_1_1_0)(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libmpfr.so.6()(64bit)
    libpcre.so.1()(64bit)
    libssl.so.1.1()(64bit)
    libssl.so.1.1(OPENSSL_1_1_0)(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.5)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    libz.so.1()(64bit)
    rtld(GNU_HASH)

qore-stdlib (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libm.so.6()(64bit)
    libqore
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    qore-module(abi)(x86-64)
    rtld(GNU_HASH)

qore-doc (rpmlib, GLIBC filtered):

qore-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libpthread.so.0()(64bit)
    libqore(x86-64)
    libqore.so.6()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.9)(64bit)
    rtld(GNU_HASH)

qore-devel-doc (rpmlib, GLIBC filtered):

qore-misc-tools (rpmlib, GLIBC filtered):
    qore

qore-debuginfo (rpmlib, GLIBC filtered):

qore-debugsource (rpmlib, GLIBC filtered):



Provides
--------
qore:
    qore
    qore(x86-64)

libqore:
    libqore
    libqore(x86-64)
    libqore.so.6()(64bit)
    libqore6
    qore-module(abi)(x86-64)

qore-stdlib:
    qore-stdlib
    qore-stdlib(x86-64)

qore-doc:
    qore-doc

qore-devel:
    pkgconfig(qore)
    qore-devel
    qore-devel(x86-64)

qore-devel-doc:
    qore-devel-doc

qore-misc-tools:
    qore-misc-tools

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

qore-debugsource:
    qore-debugsource
    qore-debugsource(x86-64)

(In reply to Andy Mender from comment #41)
> > %global module_dir %{_libdir}/qore-modules
> > %global user_module_dir %{_datarootdir}/qore-modules/
> 
> Not a requirement, but %{_datadir} resolves to the same directory -
> "/usr/share".

ok done

> > Summary: Multithreaded Programming Language
> > Name: qore
> > Version: 0.9.4.5
> > Release: 1%{?dist}
> > License: LGPLv2+ or GPLv2+ or MIT
> > Group: Development/Languages
> > URL: http://qore.org
> > Source0: https://github.com/qorelanguage/qore/releases/download/release-%{version}/%{name}-%{version}.tar.bz2
> > Requires: /usr/bin/env
> > BuildRequires: gcc-c++
> > BuildRequires: flex >= 2.5.31
> > BuildRequires: bison
> > BuildRequires: openssl-devel
> > BuildRequires: pcre-devel
> > BuildRequires: zlib-devel
> > BuildRequires: gmp-devel
> > BuildRequires: mpfr-devel
> > BuildRequires: doxygen
> > BuildRequires: pkgconfig
> > BuildRequires: bzip2-devel
> 
> Not a strict requirement, but I would split the initial tags into blocks and
> improve formatting a bit:
> %global         module_dir %{_libdir}/qore-modules
> %global         user_module_dir %{_datarootdir}/qore-modules/
> 
> Name:           qore
> Version:        0.9.4.5
> Release:        1%{?dist}
> Summary:        Multithreaded Programming Language
> 
> License:        LGPLv2+ or GPLv2+ or MIT
> Group:          Development/Languages
> URL:            http://qore.org
> Source0:       
> https://github.com/qorelanguage/qore/releases/download/release-%{version}/
> %{name}-%{version}.tar.bz2
> 
> Requires:       /usr/bin/env
> BuildRequires:  gcc-c++
> BuildRequires: 	flex >= 2.5.31
> BuildRequires: 	bison
> BuildRequires: 	openssl-devel
> BuildRequires: 	pcre-devel
> BuildRequires: 	zlib-devel
> BuildRequires: 	gmp-devel
> BuildRequires: 	mpfr-devel
> BuildRequires: 	doxygen
> BuildRequires: 	pkgconfig
> BuildRequires: 	bzip2-devel

done

> > License:        LGPLv2+ or GPLv2+ or MIT
> > Group:          Development/Languages
> 
> - I would add a comment above the License block with this link:
> https://github.com/qorelanguage/qore/blob/develop/README-LICENSE
>   to make it clear that depending on the license choice, different modules
> are enabled or disabled. `licensecheck` complains a great deal,
> unfortunately.
> - The Group: tag is obsolete and should be removed.
> - "/usr/bin/env" is a part of the coreutils package, but since it's a part
> of the base system, this Requires can probably be removed.
> - Check which of the BuildRequires for -devel package can be replaced with
> pkgconfig(foo). Relevant section of the Packaging Guidelines:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/
> PkgConfigBuildRequires/

done - thank you for the info and references - I missed those (among others)

> The main package should probably contain Requires like these to make sure
> all of the subpackages are correctly linked to the main package:
> Requires: %{name}-libqore%{?_isa} = %{version}-%{release}
> Requires: %{name}-stdlib%{?_isa} = %{version}-%{release}
> etc.
> 
> > %package -n libqore
> > Summary: The libraries for the qore runtime and qore clients
> > Group: System Environment/Libraries
> > Provides: qore-module(abi)%{?_isa} = 0.23
> > Provides: qore-module(abi)%{?_isa} = 0.22

done

> - The Group tag is obsolete and should be removed.
> - rpmlint complains about the abi provides: libqore.x86_64: E:
> useless-provides qore-module(abi)(x86-64)
>   I couldn't find anything explicit in the Packaging Guidelines, but perhaps
> you can remove these?
> 
> > Provides: libqore6 = %{version}
> > Obsoletes: libqore6 < %{version}
> 
> I'm wondering about these lines. Neither qore, nor any version of libqore is
> in the repositories so I don't think these are needed.

They were only for existing systems installed with non-official RPMs, but anyway I removed them

> > %files -n libqore
> > %{_libdir}/libqore.so.6.2.0
> > %{_libdir}/libqore.so.6
> > %doc COPYING.LGPL COPYING.GPL COPYING.MIT README.md README-LICENSE README-MODULES RELEASE-NOTES AUTHORS ABOUT
> 
> The COPYING.* files are license files and they should be tagged with the
> %license macro.

done

> > %post -n libqore -p /sbin/ldconfig
> 
> > %postun -n libqore -p /sbin/ldconfig
> 
> Running ldconfig is no longer needed and these lines should be removed.

done

> > %package stdlib
> > Summary: Standard library modules
> > Group: System Environment/Libraries
> > Requires: libqore = %{version}-%{release}
> > Requires: qore-module(abi)%{?_isa} = 0.23
> 
> - Group tags are obsolete and should be removed.
> - Use a fully qualified version Requires like so:
>   Requires: %{name}-libqore%{?_isa} = %{version}-%{release}
> - The abi requirement can probably be removed.

done

> > %files stdlib
> > %{user_module_dir}
> > %{module_dir}
> > %doc COPYING.MIT README-LICENSE
> 
> - The %{module_dir} global definition should end with a slash to make the
> package own the directory like it's done in %{user_module_dir}.
> - COPYING.MIT is a license file and should be listed with the %license macro.

done

> > %files doc
> > %doc docs/lang docs/modules/* examples/ COPYING.LGPL COPYING.GPL COPYING.MIT README-LICENSE
> 
> The COPYING.* files are license files and should be listed with the %license
> macro.

done

> > %dir %{_libdir}/cmake
> > %{_libdir}/cmake/Qore
> > %{_includedir}/*
> 
> - Should this package own the /usr/lib/cmake dir? That doesn't sound right.
> I would use the following instead:
>   %{_libdir}/cmake/Qore/

done

> - %{_includedir}/* is a really bad idea. Maybe it would be better to have
> something like this?
>   %{_includedir/qore/

done

> > %files devel-doc
> > %doc docs/library/html/* COPYING.LGPL COPYING.GPL COPYING.MIT README-LICENSE
> 
> The COPYING.* files are license files and should be listed with the %license
> macro.

done

> > %package misc-tools
> > Summary: Miscellaneous user tools writen in Qore Programming Language
> > License: LGPL-2.0+ or GPL-2.0+ or MIT
> > Group: Development/Tools/Other
> > Requires: qore = %{version}-%{release}
> > BuildArch: noarch
> 
> - The License block contains invalid license definitions. Use the same ones
> as listed for the main package.
> - Group tags are obsolete and should be removed.

done

> - Use a fully qualified version Requires like so:
>   Requires: %{name}-qore%{?_isa} = %{version}-%{release}
>   Although I'm not sure whether here it makes sense if the main qore package
> were to explicitly Requires qore-misc-tools like I suggested earlier.

This leads to the following error; I assume a noarch pkg should not depend on an arch-specific package:
> BuildError: The following noarch package built differently on different architectures: qore-misc-tools-0.9.4.6-1.fc33.noarch.rpm
> rpmdiff output was:
> removed     REQUIRES qore(armv7hl-32) = 0.9.4.6-1.fc33
> added       REQUIRES qore(x86-32) = 0.9.4.6-1.fc33

> > %files misc-tools
> > %defattr(-,root,root,-)
> 
> Use of %defattr is obsolete and should be removed.

done

> > %build
> > export CXXFLAGS="%{optflags}"
> 
> > %files
> > %{_bindir}/qore
> > %{_bindir}/qdbg
> > %{_bindir}/qdbg-server
> > %{_bindir}/qdbg-remote
> > %{_bindir}/qdbg-vsc-adapter
> > %{_mandir}/man1/qore.1.*
> 
> Do the individual subpackages have their own manpages?

no, they do not

> > %configure --disable-debug --disable-dependency-tracking
> > make %{?_smp_mflags}
> 
> Use %make_build instead of running make and supplying the smp flags.

done

> > %install
> > make install prefix=%{_prefix} DESTDIR=$RPM_BUILD_ROOT
> 
> - See whether it's possible to replace this entire "make instalL" call with
> %make_install.
> - Also, add the "-p" flag to %make_install to preserve timestamps.

yes, it works, thanks; done

thank you for the comprehensive review; I've updated the spec file for the above and also for the newest public release - Qore 0.9.4.6

new links:
- https://docs.qore.org/srpms/qore.spec
- https://docs.qore.org/srpms/qore-0.9.4.6-1.fc32.src.rpm

Koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=48551232

Super good job on the updates! :)

> Requires: %{name}-libqore%{?_isa} = %{version}-%{release}
> Requires: %{name}-stdlib%{?_isa} = %{version}-%{release}

To make the main package fully "meta", there should be lines like these for all of the subpackages. Also, I think the libqore package is actually called just "libqore". At least that's how Koji built it and that's what I see in my local mock environment as well.

> This leads to the following error; I assume a noarch pkg should not depend on an arch-specific package:
> > BuildError: The following noarch package built differently on different architectures: qore-misc-tools-0.9.4.6-1.fc33.noarch.rpm
> > rpmdiff output was:
> > removed     REQUIRES qore(armv7hl-32) = 0.9.4.6-1.fc33
> > added       REQUIRES qore(x86-32) = 0.9.4.6-1.fc33

You're completely right. It only works for the opposite - an arch package depending on a noarch package. What you can do is make qore-misc-tools depend on only a subset of the qore subpackages - the ones it actually requires. I guess that would be "libqore" primarily?

> %install
> %make_install -p
> mkdir -p $RPM_BUILD_ROOT/%{module_dir}
> rm $RPM_BUILD_ROOT/%{_libdir}/libqore.la

If you look closely, the last 2 paths will contain duplicate forward slashes. Not a big thing, but the below should be okay:
mkdir -p $RPM_BUILD_ROOT%{module_dir}
rm $RPM_BUILD_ROOT%{_libdir}/libqore.la

> %files -n libqore
> %{_libdir}/libqore.so.6.2.1
> %{_libdir}/libqore.so.6
> %license COPYING.LGPL COPYING.GPL COPYING.MIT
> %doc README.md README-LICENSE README-MODULES RELEASE-NOTES AUTHORS ABOUT

I think the README-LICENSE file is actually a license file with extra commentary so it should be listed together with the other license files with the %license macro. Also, since qore is multi-licensed and highly modular, I would add README-LICENSE to the -devel and -misc-tools subpackages.

> %changelog
> [...]
> - replaced %{_datarootdir} with ${_datadir}
> [...]
> - removed obsolete references to %defattr and ldconfig
> - use %make_build instead of a hardcoded make line
> - use %make_install -p instead of a hardcoded make install line

Minor nitpick, you should avoid using macros in %changelog records or escape them by repeating the macro character (for instance, %%make_build instead of %make_build).

> * Thu Jul 30 2020 David Nichols <david> 0.9.4.5-1
> - added required BuildRequires for gcc-c++

I think the dist tag on this one was supposed to be 2 (full version 0.9.4.5-2), right?

(In reply to Andy Mender from comment #43)
> Super good job on the updates! :)

thank you very much!

> > Requires: %{name}-libqore%{?_isa} = %{version}-%{release}
> > Requires: %{name}-stdlib%{?_isa} = %{version}-%{release}
> 
> To make the main package fully "meta", there should be lines like these for
> all of the subpackages. Also, I think the libqore package is actually called
> just "libqore". At least that's how Koji built it and that's what I see in
> my local mock environment as well.

of course that is correct - done

I would like to understand what you mean with the Requires lines - currently I have:

qore -> requires libqore, qore-stdlib
libqore -> requires nothing
qore-doc () -> requires nothing
qore-devel (C++ development package) -> requires libqore
qore-devel-doc (C++ development docs) -> requires nothing
qore-misc-tools -> requires qore (which in turn requires libqore and qore-stdlib)

qore-devel:
The Qore library can be used without qore and the qore-stdlib packages to allow for developing programs supporting embedded logic in them; the qore and qore-stdlib packages are both generally useful and in the vast majority of cases would also be used, however in a theoretical "lean" use case implementing only embedded logic in an application where the qore standard library is not required or needed, and external scripting support with qore is irrelevant, they would not be installed and requiring them would just take up extra space with no benefit.

qore-doc and qore-devel-doc:
Regarding requiring anything for the doc packages, I followed originally the same approach as with other languages such as Python, where the doc packages can be installed standalone as well - ex: https://src.fedoraproject.org/rpms/python3-docs/blob/master/f/python3-docs.spec - which has no dependencies on external packaging.

> > This leads to the following error; I assume a noarch pkg should not depend on an arch-specific package:
> > > BuildError: The following noarch package built differently on different architectures: qore-misc-tools-0.9.4.6-1.fc33.noarch.rpm
> > > rpmdiff output was:
> > > removed     REQUIRES qore(armv7hl-32) = 0.9.4.6-1.fc33
> > > added       REQUIRES qore(x86-32) = 0.9.4.6-1.fc33
> 
> You're completely right. It only works for the opposite - an arch package
> depending on a noarch package. What you can do is make qore-misc-tools
> depend on only a subset of the qore subpackages - the ones it actually
> requires. I guess that would be "libqore" primarily?

The scripts in this package require the qore executable to run, and the qore pkg in turn depends on libqore & qore-stdlib, so those other two package are indirect dependencies through qore.  libqore is not sufficient for the qore-misc-tools package, but needs to be in place so that the qore executable will run.

> > %install
> > %make_install -p
> > mkdir -p $RPM_BUILD_ROOT/%{module_dir}
> > rm $RPM_BUILD_ROOT/%{_libdir}/libqore.la
> 
> If you look closely, the last 2 paths will contain duplicate forward
> slashes. Not a big thing, but the below should be okay:
> mkdir -p $RPM_BUILD_ROOT%{module_dir}
> rm $RPM_BUILD_ROOT%{_libdir}/libqore.la

done

> > %files -n libqore
> > %{_libdir}/libqore.so.6.2.1
> > %{_libdir}/libqore.so.6
> > %license COPYING.LGPL COPYING.GPL COPYING.MIT
> > %doc README.md README-LICENSE README-MODULES RELEASE-NOTES AUTHORS ABOUT
> 
> I think the README-LICENSE file is actually a license file with extra
> commentary so it should be listed together with the other license files with
> the %license macro. Also, since qore is multi-licensed and highly modular, I
> would add README-LICENSE to the -devel and -misc-tools subpackages.

done

> > %changelog
> > [...]
> > - replaced %{_datarootdir} with ${_datadir}
> > [...]
> > - removed obsolete references to %defattr and ldconfig
> > - use %make_build instead of a hardcoded make line
> > - use %make_install -p instead of a hardcoded make install line
> 
> Minor nitpick, you should avoid using macros in %changelog records or escape
> them by repeating the macro character (for instance, %%make_build instead of
> %make_build).

oops - I knew that! - done

> > * Thu Jul 30 2020 David Nichols <david> 0.9.4.5-1
> > - added required BuildRequires for gcc-c++
> 
> I think the dist tag on this one was supposed to be 2 (full version
> 0.9.4.5-2), right?

Yes, correct; I did not follow https://docs.fedoraproject.org/en-US/packaging-guidelines/#_multiple_changelog_entries_per_release correctly.
I've updated this now so there's only one changelog entry for the 0.9.4.5-1 release.

I have updated the spec file here:
- https://docs.qore.org/srpms/qore.spec

I did not make a new SRPM or Koji build yet, as I would like to understand the dependency issues above first.

Thank you again for your time and feedback on this!

> I would like to understand what you mean with the Requires lines - currently I have:

> qore -> requires libqore, qore-stdlib
> libqore -> requires nothing
> qore-doc () -> requires nothing
> qore-devel (C++ development package) -> requires libqore
> qore-devel-doc (C++ development docs) -> requires nothing
> qore-misc-tools -> requires qore (which in turn requires libqore and qore-stdlib)

Apologies, I misunderstood the original intention behind the main "qore" package. I thought it's more of a meta/toolchain package.

> qore-devel:
> The Qore library can be used without qore and the qore-stdlib packages to allow for developing programs supporting embedded logic in them; the qore and qore-stdlib packages are both generally useful and in the vast majority of cases would also be used, however in a theoretical "lean" use case implementing only embedded logic in an application where the qore standard library is not required or needed, and external scripting support with qore is irrelevant, they would not be installed and requiring them would just take up extra space with no benefit.

Usually -devel packages require the main package. However, since qore-devel requires libqore, I think not requiring the main package is alright.

> qore-doc and qore-devel-doc:
> Regarding requiring anything for the doc packages, I followed originally the same approach as with other languages such as Python, where the doc packages can be installed standalone as well - ex: https://src.fedoraproject.org/rpms/python3-docs/blob/master/f/python3-docs.spec - which has no dependencies on external packaging.

Good point! I see python3-docs uses the soft-dependency tag "Recommends". You can use something similar if you see fit :).

> The scripts in this package require the qore executable to run, and the qore pkg in turn depends on libqore & qore-stdlib, so those other two package are indirect dependencies through qore.  libqore is not sufficient for the qore-misc-tools package, but needs to be in place so that the qore executable will run.

I'm trying to read a bit deeper into this section from the Packaging Guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_requiring_base_package
I think the problem might be lines like these (using the %{?_isa} macro):
Requires: %{name}%{?_isa} = %{version}-%{release}

Can you try dropping the %{?_isa} macro? Perhaps that's causing issues?

(In reply to Andy Mender from comment #45)
> > qore-devel:
> > The Qore library can be used without qore and the qore-stdlib packages to allow for developing programs supporting embedded logic in them; the qore and qore-stdlib packages are both generally useful and in the vast majority of cases would also be used, however in a theoretical "lean" use case implementing only embedded logic in an application where the qore standard library is not required or needed, and external scripting support with qore is irrelevant, they would not be installed and > > qore-doc and qore-devel-doc:
> > Regarding requiring anything for the doc packages, I followed originally the same approach as with other languages such as Python, where the doc packages can be installed standalone as well - ex: https://src.fedoraproject.org/rpms/python3-docs/blob/master/f/python3-docs.spec - which has no dependencies on external packaging.
> 
> Good point! I see python3-docs uses the soft-dependency tag "Recommends".
> You can use something similar if you see fit :).

OK great suggestion - I now have the following (blank lines added for clarity):

$ grep -e ^%package -e ^Requires: -e ^Recommends: -e ^BuildArch: qore.spec
Requires: libqore%{?_isa} = %{version}-%{release}
Requires: %{name}-stdlib%{?_isa} = %{version}-%{release}

%package -n libqore
%package stdlib
Requires: libqore%{?_isa} = %{version}-%{release}
Recommends: %{name} = %{version}

%package doc
BuildArch: noarch
Recommends: %{name} = %{version}

%package devel
Requires: libqore%{?_isa} = %{version}-%{release}
Recommends: %{name} = %{version}

%package devel-doc
BuildArch: noarch
Recommends: %{name}-devel = %{version}

%package misc-tools
Requires: %{name} = %{version}-%{release}
BuildArch: noarch

> > The scripts in this package require the qore executable to run, and the qore pkg in turn depends on libqore & qore-stdlib, so those other two package are indirect dependencies through qore.  libqore is not sufficient for the qore-misc-tools package, but needs to be in place so that the qore executable will run.
> 
> I'm trying to read a bit deeper into this section from the Packaging
> Guidelines:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/
> #_requiring_base_package
> I think the problem might be lines like these (using the %{?_isa} macro):
> Requires: %{name}%{?_isa} = %{version}-%{release}
> 
> Can you try dropping the %{?_isa} macro? Perhaps that's causing issues?

They have been dropped from all noarch pkgs; do you mean that I should also drop the {%?_isa} from the libqore dependencies?

My take on the above guideline is based on a possibly more liberal interpretation of "base package".  For me the base package is libqore.  This is the fundamental package that has no other dependencies from the project and is usable by itself.  The qore package will be required for most use cases but not all.  libqore will always be required and is usable without qore and also without qore-stdlib, even if in the majority of realistic use cases, qore and qore-stdlib will be what users really want, and libqore will get installed in the background to support those.

Possible installations in increasing order of capabilities:
1) libqore
   An absolute minimum installation for embedded code support only - assuming an external custom API - no standard Qore runtime

2) libqore + qore-stdlib 
   A more realistic minimal installation for embedded code support only, including the Qore standard runtime libraries

3) libqore + qore-stdlib + qore
   The most common installation for normal scripting + embedded code.

new links:
- https://docs.qore.org/srpms/qore.spec
- https://docs.qore.org/srpms/qore-0.9.4.6-3.fc32.src.rpm

Koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=49011915

Thanks again for your time on this!

> $ grep -e ^%package -e ^Requires: -e ^Recommends: -e ^BuildArch: qore.spec
> Requires: libqore%{?_isa} = %{version}-%{release}
> Requires: %{name}-stdlib%{?_isa} = %{version}-%{release}

Looks good.

> %package stdlib
> Requires: libqore%{?_isa} = %{version}-%{release}
> Recommends: %{name} = %{version}

Good.

> %package doc
> BuildArch: noarch
> Recommends: %{name} = %{version}

Good.

> %package devel
> Requires: libqore%{?_isa} = %{version}-%{release}
> Recommends: %{name} = %{version}

Hmm in the spec file I see the following:
%package devel
Summary: The header files needed to compile programs using the qore library
Requires: libqore%{?_isa} = %{version}-%{release}
Recommends: %{name}%{?_isa} = %{version}

> %package devel-doc
> BuildArch: noarch
> Recommends: %{name}-devel = %{version}

Good.

> %package misc-tools
> Requires: %{name} = %{version}-%{release}
> BuildArch: noarch

Good.

> They have been dropped from all noarch pkgs; do you mean that I should also drop the {%?_isa} from the libqore dependencies?

I think it's enough for the noarch packages.

> My take on the above guideline is based on a possibly more liberal interpretation of "base package".  For me the base package is libqore.  This is the fundamental package that has no other dependencies from the project and is usable by itself.  The qore package will be required for most use cases but not all.  libqore will always be required and is usable without qore and also without qore-stdlib, even if in the majority of realistic use cases, qore and qore-stdlib will be what users really want, and libqore will get installed in the background to support those.
> 
> Possible installations in increasing order of capabilities:
> 1) libqore
>    An absolute minimum installation for embedded code support only - assuming an external custom API - no standard Qore runtime
> 
> 2) libqore + qore-stdlib 
>    A more realistic minimal installation for embedded code support only, including the Qore standard runtime libraries
> 
> 3) libqore + qore-stdlib + qore
>    The most common installation for normal scripting + embedded code.

Alright, I think this makes sense then. I'm simply not very experienced with multi-package SPEC files.

I'm a little bothered by the mismatch in the source archive sizes as shown by fedora-review:
> https://github.com/qorelanguage/qore/releases/download/release-0.9.4.6/qore-0.9.4.6.tar.bz2 :
>   CHECKSUM(SHA256) this package     : 6afbc3128bd0d67e81b736f167b1b1c710966f8fb48bd7ecb3f9608ea5a61220
>   CHECKSUM(SHA256) upstream package : 852141f7b3e1c6fdbc72fafa71e6b624a3963fa5d29de5d1b638098a8bef579a

I checked myself and the archive inside the SRPM indeed has a different size than the one downloaded from GitHub. Could you maybe try switching to the tar.gz source tarball instead? https://github.com/qorelanguage/qore/archive/release-0.9.4.6.tar.gz
You would need to modify the URL line a bit.

(In reply to Andy Mender from comment #47)
> > %package devel
> > Requires: libqore%{?_isa} = %{version}-%{release}
> > Recommends: %{name} = %{version}
> 
> Hmm in the spec file I see the following:
> %package devel
> Summary: The header files needed to compile programs using the qore library
> Requires: libqore%{?_isa} = %{version}-%{release}
> Recommends: %{name}%{?_isa} = %{version}

oops I think I added that back because it is an arch-specific package, and I assumed that I should only remove it for the noarch packages.  Should I remove it then here too?

> I'm a little bothered by the mismatch in the source archive sizes as shown
> by fedora-review:
> > https://github.com/qorelanguage/qore/releases/download/release-0.9.4.6/qore-0.9.4.6.tar.bz2 :
> >   CHECKSUM(SHA256) this package     : 6afbc3128bd0d67e81b736f167b1b1c710966f8fb48bd7ecb3f9608ea5a61220
> >   CHECKSUM(SHA256) upstream package : 852141f7b3e1c6fdbc72fafa71e6b624a3963fa5d29de5d1b638098a8bef579a
> 
> I checked myself and the archive inside the SRPM indeed has a different size
> than the one downloaded from GitHub. Could you maybe try switching to the
> tar.gz source tarball instead?
> https://github.com/qorelanguage/qore/archive/release-0.9.4.6.tar.gz
> You would need to modify the URL line a bit.

yes, it's true, the spec file was updated after the release due to this process.  In the actual release I will have the final spec file once all issues are resolved.  I believe we are very close now.

Then I can make a new release of Qore with the updated spec file so that the file match exactly I hope.

I hope that I can get them to match with bz2 due to the space difference.

> oops I think I added that back because it is an arch-specific package, and I assumed that I should only remove it for the noarch packages.  Should I remove it then here too?

Good question. The problems with ambiguous arch dependencies we had before are gone, so I think this can stay.

> yes, it's true, the spec file was updated after the release due to this process.  In the actual release I will have the final spec file once all issues are resolved.  I believe we are very close now.
> 
> Then I can make a new release of Qore with the updated spec file so that the file match exactly I hope.

Keep me posted then! I simply noticed that the tar.gz source tarballs are a little easier to handle.

Hello David :). Any updates on this?

(In reply to Andy Mender from comment #50)
> Hello David :). Any updates on this?

Hi Andy, I apologize for the long delay, as I had some other top priorities take up my time.  I am days away from making the 0.9.5 release of Qore, so I was thinking that I would update the spec file for that release, as Qore 0.9.5 supports full bi-directional Python / Qore dynamic inheritance and code mixing as well as full ARM processor support as well as other feature and bugfixes.

> I simply noticed that the tar.gz source tarballs are a little easier to handle.

I was assuming that the SRPM size issues were due to the fact that I updated the spec file while using the same release package; I hope it will work with tar.bz2 files due to the reduced file size with bz2 compression.

Also this request has gone on so long that I have forgotten how the process works regarding sponsors; I may need to look for a sponsor after all, if so I will update the issue accordingly and start that process.

In any case I expect to have the Qore 0.9.5 release ready next week along with the hopefully final spec file / RPMs.

Sounds good! Thanks for the update :)

Regarding sponsorship, please have a look at this doc: https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

updated release links:

new links:
- https://docs.qore.org/srpms/qore.spec
- https://docs.qore.org/srpms/qore-0.9.6-3.fc32.src.rpm

Koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=55301269

I hope there will not be a problem with the tar.bz2 sources, as the same source was uploaded to the release as used for the SRPM.

Also I still need to make some progress on the sponsorship - on my TODO list!

thx
David

> new links:
> - https://docs.qore.org/srpms/qore.spec
> - https://docs.qore.org/srpms/qore-0.9.6-3.fc32.src.rpm

The SRPM link returns 404. I pulled the SRPM from the Koji build so no worries.

fedora-review also complained that the SSL cert for https://qore.org is invalid, but other than that everything looks fine and the SHA sums match now:
Source checksums
----------------
https://github.com/qorelanguage/qore/releases/download/release-0.9.6/qore-0.9.6.tar.bz2 :
  CHECKSUM(SHA256) this package     : bc38176989f5d10f0e70702acc48979800f41d8ecfd22b250efbc80591329b56
  CHECKSUM(SHA256) upstream package : bc38176989f5d10f0e70702acc48979800f41d8ecfd22b250efbc80591329b56

Package approved!