Bug 2445610

Cannot find any valid SRPM URL for this ticket. Common causes are:

- You didn't specify `SRPM URL: ...` in the ticket description
  or any of your comments
- The URL schema isn't HTTP or HTTPS
- The SRPM package linked in your URL doesn't match the package name specified
  in the ticket summary


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

??

The SPRM URL is valid, and is HTTPS.
The package name is c-evo-dh.
I don't know what the problem is here!

Repeating SPRM URL
SRPM URL: https://download.copr.fedorainfracloud.org/results/peterbb/c-evo-dh/srpm-builds/10198112/c-evo-dh-3.2-1.src.rpm

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10203285
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2445610-c-evo-dh/fedora-rawhide-x86_64/10203285-c-evo-dh/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

(In reply to peterbb from comment #2)
> ??
> 
> The SPRM URL is valid, and is HTTPS.
> The package name is c-evo-dh.
> I don't know what the problem is here!

Happens sometimes with automated tests, don't worry.

> URL:        https://git.code.sf.net/p/c-evo-eh/code
My impression is that it's expected the URL field leads to a human-readable website.
I'd link to "https://sourceforge.net/projects/c-evo-eh/" instead.

> Group:      Amusements/Games
Not used in Fedora.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections

> BuildRequires:  lazarus
This will drag whole graphical IDE into the build.
You can require "lazarus-tools" and "lazarus-lcl-gtk2" instead.

> Requires:   gtk2
> Requires:   gdk-pixbuf2
> Requires:   glibc
> Requires:   pango
> Requires:   at-spi2-core
> Requires:   cairo
These are probably not needed, RPM should figure out the dependencies
based on the libraries the program is linking to.

> # If needed, its easier to debug Lazarus programs with an unstripped executables
> %global debug_package %{nil}
Please don't. We want debuginfo in Fedora.

> # Automatic strip not working!  ( W: unstripped-binary-or-object )
> strip --strip-unneeded %{buildroot}%{_libexecdir}/%{name}/c-evo*
> strip --strip-unneeded %{buildroot}%{_libdir}/%{name}/lib*.so
Automated strip does not work because the bit above disables it.

> %global _unpackaged_files_terminate_build 0
Discouraged, as it will allow new files to slip in unnoticed when updating versions.

> %setup -c -q %{name}-%{version}
> cd           %{name}-%{version}
Uhh, why, though? You're creating a directory called "%{name}-%{version}", only to unpack
the source archive - which contains a directory with the same name - and then cd into it.
You can probably just use "%autosetup" here (without any extra arguments).

> %files
> %{_datadir}/metainfo/*
You should build-require "libappstream-glib" and run "appstream-util validate-relax --nonet" on AppData files.
https://docs.fedoraproject.org/en-US/packaging-guidelines/AppData/#_app_data_validate_usage

> %files
> %{_datadir}/%{name}
> %{_datadir}/icons/*
You may want to move these to a -data subpackage and make it noarch.

Many thanks for the review.
Fixed all the issues raised.

https://copr.fedorainfracloud.org/coprs/peterbb/c-evo-dh/build/10208100/

A couple more details that I missed earlier:

> BuildRequires:  fpc
FPC does not support all the architectures that Fedora does.
You'll need to add "ExclusiveArch: %{fpc_arches}" to the spec.

> %install
> make DESTDIR=%{buildroot}/ LibDir=/lib64 ExecDir=/libexec install
You should use "/%{_lib}" here instead of hard-coding "/lib64".

> %{_libdir}/*
> %{_libexecdir}/*
Globbing like this is discouraged. Please use "%{_libdir}/%{name}/" and "%{_libexecdir}/%{name}/" here.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_lists

> %{_datadir}/metainfo/
> %{_datadir}/applications/
This will mark your package as the owner of these directories.
Please list the actual files here.


Regarding the -data split:

> Requires:   %{name}-data
This should be a versioned dependency: "%{name}-data = %{version}-%{release}"

> License:    GPL-2.0-or-later AND CC-BY-3.0
> [...]
> %files
> # GPL-2.0-or-later
> [...]
> %files data
> # CC-BY-3.0
You can actually specify a different License tag for the subpackage.

> %files
> %{_datadir}/%{name}/AI/
> [...]
> %files data
> %dir %{_datadir}/%{name}
> %{_datadir}/%{name}/*.txt
> %{_datadir}/%{name}/Graphics/
> %{_datadir}/%{name}/Help/
> %{_datadir}/%{name}/Localization/
> %{_datadir}/%{name}/Maps/
> %{_datadir}/%{name}/Sounds/
> %{_datadir}/%{name}/Tribes/
This is fine, but a simpler approach would be to make -data own "%{_datadir}/%{name}"
and then add "%exclude %{_datadir}/%{name}/AI".

> %{_datadir}/icons/*
Once again, your package will end up owning the directory.
Since you have a requires on hicolor-icon-theme, you can use "%{_datadir}/icons/hicolor/*/apps/%{name}.svg" here.

Thanks for the extra details. All implemented apart from 
the "simpler approach" to the %files for the data package.
I had some problems with it,
but rather than spend more time,
you said it was fine, and it does work as is!

https://copr.fedorainfracloud.org/coprs/peterbb/c-evo-dh/build/10209872/


Regarding the %{gpgverify} step.

This broke builds on Mageia and older versions of Fedora
as the gpgverify package does not seem to be available there.

Is there a simple way of making this check conditional
on the presence of gpgverify, to restore those builds?

Looks like I could use
%if 0%{?fedora} >= 42
around the gpgverify items.

(In reply to peterbb from comment #9)
> Looks like I could use
> %if 0%{?fedora} >= 42
> around the gpgverify items.

Abandoning that idea now.

Updating URLs

Spec URL: https://download.copr.fedorainfracloud.org/results/peterbb/c-evo-dh/srpm-builds/10209872/c-evo-dh.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/peterbb/c-evo-dh/srpm-builds/10209872/c-evo-dh-3.2-1.src.rpm

Created attachment 2133101 [details]
The .spec file difference from Copr build 10203285 to 10218151

Copr build:
https://copr.fedorainfracloud.org/coprs/build/10218151
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2445610-c-evo-dh/fedora-rawhide-x86_64/10218151-c-evo-dh/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.