Bug 1539116

Summary: Review Request: grass - GRASS GIS - Geographic Resources Analysis Support System
Product: [Fedora] Fedora Reporter: markusN <neteler>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: devrim, fedora, lemenkov, package-review
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-08-15 10:41:09 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Updated SPEC file for GRASS GIS 7.4.0 none

Description markusN 2018-01-26 17:28:15 UTC
Spec URL: https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_7_4/rpm/grass.spec
SRPM URL: http://data.neteler.org/tmp/grass-7.4.0-1.fc27.src.rpm
Description: New upstream version GRASS GIS 7.4.0. The SPEC file received a cleanup and also addresses latest FTBS issues.
Fedora Account System Username: neteler

Comment 1 markusN 2018-01-28 15:39:21 UTC
I have made a minor update (included as a patch), now it also compiles on EPEL7:

http://data.neteler.org/tmp/grass-7.4.0-2.fc27.src.rpm

Comment 2 Antonio T. (sagitter) 2018-01-28 16:01:02 UTC
Are you sponsored?
fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

Comment 3 markusN 2018-01-28 16:58:43 UTC
No, I am not sponsored so far. My goal is to get this update of GRASS GIS into Fedora which addresses most (all?) of the open bugs:

https://bugzilla.redhat.com/buglist.cgi?component=grass&list_id=8348120&product=Fedora

Comment 4 Antonio T. (sagitter) 2018-01-28 17:37:16 UTC
- Fix dependencies between packages:

%{name}%{?isa}-libs = %{version}-%{release} becomes
%{name}-libs%{?_isa} = %{version}-%{release}

or

%{name}%{?isa} = %{version}-%{release} becomes
%{name}%{?_isa} = %{version}-%{release}

- '-gui' sub-package already gets 'grass-libs' through 'grass'; line #110 can be removed.

- appdata.xml files go into '/usr/share/metainfo'.

Comment 5 Artur Frenszek-Iwicki 2018-01-28 19:03:34 UTC
>Group: Applications/Engineering
The "Group:" tag is not used in Fedora.

># EPEL7 fails on url tag, so we ignore failure:
>appstream-util validate-relax --nonet %{buildroot}/%{_datadir}/appdata/*.appdata.xml || echo "Ignoring appstream-util failure"
What exactly is the error here? Could we patch the appdata file so the validation doesn't fail?

>%post
>/bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :
>
>%postun
>if [ $1 -eq 0 ] ; then
>	/bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>	/usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
>fi
>
>%posttrans
>/usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
The icon cache scriplets are no longer used in Fedora. These should be made EPEL-only.

Another minor issue I can see is the mixed use of $RPM_BUILD_ROOT and %buildroot. It is preferred to pick one and stick with it.

Comment 6 markusN 2018-01-28 19:36:52 UTC
Thanks to both for the quick review.

(In reply to Iwicki Artur from comment #5)
> >Group: Applications/Engineering
> The "Group:" tag is not used in Fedora.
> 
> ># EPEL7 fails on url tag, so we ignore failure:
> >appstream-util validate-relax --nonet %{buildroot}/%{_datadir}/appdata/*.appdata.xml || echo "Ignoring appstream-util failure"
>
> What exactly is the error here?

This one:

appstream-util validate-relax --nonet /builddir/build/BUILDROOT/grass-7.4.0-2.el7.centos.x86_64//usr/share/appdata/grass.appdata.xml
/builddir/build/BUILDROOT/grass-7.4.0-2.el7.centos.x86_64//usr/share/appdata/grass.appdata.xml: GLib-GIO-Message: Using the 'memory' GSettings backend.  Your settings will not be saved or shared with other applications.
FAILED:
? markup-invalid        : <id> does not have correct extension for kind
Validation of files failed

> Could we patch the appdata file so the validation doesn't fail?

The "offending" line is this one:

  <id>org.osgeo.grass</id>

Does it need to be
  <id>org.osgeo.grass.desktop</id>
?

> >%post
> >/bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :
> >
> >%postun
> >if [ $1 -eq 0 ] ; then
> >	/bin/touch --no-create %{_datadir}/icons/hicolor &>/dev/null
> >	/usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
> >fi
> >
> >%posttrans
> >/usr/bin/gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
> The icon cache scriplets are no longer used in Fedora. These should be made
> EPEL-only.
> 
> Another minor issue I can see is the mixed use of $RPM_BUILD_ROOT and
> %buildroot. It is preferred to pick one and stick with it.

OK, thanks. I am now testing with an updated SPEC file which includes fixes as per comment 4.

Comment 7 Artur Frenszek-Iwicki 2018-01-28 19:47:53 UTC
I compared this with a couple of appdata files I have installed on my system and yes, it seems that the value of the "<id>" tag should end with ".desktop".

Comment 8 markusN 2018-01-28 20:46:11 UTC
(In reply to Iwicki Artur from comment #7)
> I compared this with a couple of appdata files I have installed on my system
> and yes, it seems that the value of the "<id>" tag should end with
> ".desktop".

OK, fixed upstream (will then ship with the future GRASS GIS 7.4.1).

The two $RPM_BUILD_ROOT entries needed and not the same as %buildroot (compilation fails when changing them to %buildroot), so I left them as-is.

All other suggestions are included (new SPEC attached) and compile successfully on EPEL7 and Fedora:
https://copr.fedorainfracloud.org/coprs/neteler/grass74/build/706613/

Thanks again for the review!

Comment 9 markusN 2018-01-28 20:46:47 UTC
Created attachment 1387344 [details]
Updated SPEC file for GRASS GIS 7.4.0

Comment 10 Antonio T. (sagitter) 2018-01-29 16:00:18 UTC
Please, post direct links to newest SPEC and SRPM (like https://copr-be.cloud.fedoraproject.org/results/neteler/grass74/fedora-27-x86_64/00706613-grass/grass.spec, we need them for using 'fedora-review' tool)

Explicate all changes in Changelog (http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs).

This is not correct:

* Sun Jan 28 2018 Markus Neteler <neteler> - 7.4.0-2
- SPEC cleanup as per review in https://bugzilla.redhat.com/show_bug.cgi?id=1539116

Comment 11 Peter Lemenkov 2018-01-30 13:03:23 UTC
Hello Marcus,
Nice to see you there! Certainly Fedora RPM package for GRASS will benefit from your participation.

I'm going to remove FE-NEEDSPONSOR - I've just sponsored Marcus Neteler.

Comment 12 Peter Lemenkov 2018-01-30 13:13:51 UTC
Devrim, could you please add Markus as a comaintainer?

Comment 13 markusN 2018-01-30 21:02:29 UTC
(In reply to Antonio Trande from comment #10)
> Please, post direct links to newest SPEC and SRPM (like
> https://copr-be.cloud.fedoraproject.org/results/neteler/grass74/fedora-27-
> x86_64/00706613-grass/grass.spec, we need them for using 'fedora-review'
> tool)
> 
> Explicate all changes in Changelog
> (http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs).
> 
> This is not correct:
> 
> * Sun Jan 28 2018 Markus Neteler <neteler> - 7.4.0-2
> - SPEC cleanup as per review in
> https://bugzilla.redhat.com/show_bug.cgi?id=1539116


Thanks, Antonio, will update accordingly.

Comment 14 markusN 2018-01-30 21:39:19 UTC
(In reply to Peter Lemenkov from comment #11)
> Hello Marcus,
> Nice to see you there! Certainly Fedora RPM package for GRASS will benefit
> from your participation.
> 
> I'm going to remove FE-NEEDSPONSOR - I've just sponsored Marcus Neteler.

Thanks a lot for your support, Peter.

As requested in Comment 10, I expanded the Changelog section in the SPEC file which is here:

https://copr-be.cloud.fedoraproject.org/results/neteler/grass74/fedora-27-x86_64/00707314-grass/grass.spec

Comment 15 Peter Lemenkov 2018-08-15 10:41:09 UTC
(In reply to markusN from comment #14)
> (In reply to Peter Lemenkov from comment #11)
> > Hello Marcus,
> > Nice to see you there! Certainly Fedora RPM package for GRASS will benefit
> > from your participation.
> > 
> > I'm going to remove FE-NEEDSPONSOR - I've just sponsored Marcus Neteler.
> 
> Thanks a lot for your support, Peter.
> 
> As requested in Comment 10, I expanded the Changelog section in the SPEC
> file which is here:
> 
> https://copr-be.cloud.fedoraproject.org/results/neteler/grass74/fedora-27-
> x86_64/00707314-grass/grass.spec

Ok, everything was imported and built, so I'm going to close this ticket.