Bug 1539116 - Review Request: grass - GRASS GIS - Geographic Resources Analysis Support System
Summary: Review Request: grass - GRASS GIS - Geographic Resources Analysis Support System
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-01-26 17:28 UTC by markusN
Modified: 2018-08-15 10:41 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-08-15 10:41:09 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Updated SPEC file for GRASS GIS 7.4.0 (18.60 KB, text/x-matlab)
2018-01-28 20:46 UTC, markusN
no flags Details

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.


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