Bug 908088 (ascend)

ipopt dependency changed to coin-or-Ipopt.
.c/.h files rearranged to be all included in devel package.  

Spec URL: http://sagitter.fedorapeople.org/ascend/ascend.spec
SRPM URL: http://sagitter.fedorapeople.org/ascend/ascend-0.9.8-3.fc18.src.rpm

doc sub-package changed to noarch.

Spec URL: http://sagitter.fedorapeople.org/ascend/ascend.spec
SRPM URL: http://sagitter.fedorapeople.org/ascend/ascend-0.9.8-4.fc18.src.rpm

'xgraph' Requires removed.

Spec URL: http://sagitter.fedorapeople.org/ascend/ascend.spec
SRPM URL: http://sagitter.fedorapeople.org/ascend/ascend-0.9.8-5.fc18.src.rpm

The development goes slowly and there are some things to fix.

Last changes:

- Update to svn4638
- 'data' sub-package is now arched

It seems that it's not possible define noarch the 'data' package.
I obtained 
BuildError: mismatch when analyzing ascend-data-0.9.8-1.20140211svn4638.fc21.noarch.rpm 
http://koji.fedoraproject.org/koji/taskinfo?taskID=6522713

- Built a 'data' sub-package
- Removed Tcl/Tk subpackage building because of missing dependencies
  (tktable) 
- Defined two macros for the packaging
- Defined the scriplets in %%post, %%postun, %%posttrans
- Patching compiler flags
- Added CUnit-devel BR
- IDA solver excluded

Spec URL: http://sagitter.fedorapeople.org/ascend/ascend.spec
SRPM URL: http://sagitter.fedorapeople.org/ascend/ascend-0.9.8-2.20140211svn4638.fc20.src.rpm
Koji build in rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=6522934

I'll take a look (thanks for offering to swap reviews!).

A few initial comments:

1.  SHOULD drop deprecated rpm constructs, including Group: tags  (unless you plan on supporting building on/for epel-5)


2.  SHOULD give script or detailed instructions on how to (re)generate Source0 archive


3. MUST use
BuildRequires: python2-devel
and associated macros %{python2_sitelib} %{python2_sitearch} macros per
https://fedoraproject.org/wiki/Packaging:Python


4.  MUST drop unwanted deps
Requires(post): desktop-file-utils shared-mime-info
Requires(postun): desktop-file-utils shared-mime-info
see also:
https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database
https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#mimeinfo


5.  MUST fix icon scriptlets.  
stuff referencing %{_datadir}/icons/hicolor is ok, but not %{_datadir}/icons.  in short, drop the lines:
touch --no-create %{_datadir}/icons &>/dev/null
gtk-update-icon-cache %{_datadir}/icons &>/dev/null || :
gtk-update-icon-cache %{_datadir}/icons &>/dev/null || :


6.  MUST review/fix folder ownership.  Offhand, it would appear nothing owns
%{models}/  (expands to %{_libdir}/ascend/models)
%{models}/sensitivity/
%{johnpye}/  (expands to %{_libdir}/ascend/models/johnpye)
%{johnpye}/grena/
%{johnpye}/extpy/
%{johnpye}/extfn/
%{johnpye}/datareader/
...
you should get the idea.


7. SHOULD consider a -libs subpkg for 
%{_libdir}/libascend.so.*
This is mostly to be multilib friendly, as anything matching %{_libdir}/lib*.so.* will get multilib'd automatically (e.g. pkg.i686 will get pulled into x86_64 repo).

(In reply to Rex Dieter from comment #7)
> A few initial comments:
> 
> 1.  SHOULD drop deprecated rpm constructs, including Group: tags  (unless
> you plan on supporting building on/for epel-5)
> 
> 
> 2.  SHOULD give script or detailed instructions on how to (re)generate
> Source0 archive
> 
> 
> 3. MUST use
> BuildRequires: python2-devel
> and associated macros %{python2_sitelib} %{python2_sitearch} macros per
> https://fedoraproject.org/wiki/Packaging:Python
> 
> 
> 4.  MUST drop unwanted deps
> Requires(post): desktop-file-utils shared-mime-info
> Requires(postun): desktop-file-utils shared-mime-info
> see also:
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#mimeinfo

Well, this package was also created for RHEL5, but now needs packages which are not in EPEL.
Fixed.

> 
> 
> 5.  MUST fix icon scriptlets.  
> stuff referencing %{_datadir}/icons/hicolor is ok, but not
> %{_datadir}/icons.  in short, drop the lines:
> touch --no-create %{_datadir}/icons &>/dev/null
> gtk-update-icon-cache %{_datadir}/icons &>/dev/null || :
> gtk-update-icon-cache %{_datadir}/icons &>/dev/null || :

Fixed.

> 
> 
> 6.  MUST review/fix folder ownership.  Offhand, it would appear nothing owns
> %{models}/  (expands to %{_libdir}/ascend/models)
> %{models}/sensitivity/
> %{johnpye}/  (expands to %{_libdir}/ascend/models/johnpye)
> %{johnpye}/grena/
> %{johnpye}/extpy/
> %{johnpye}/extfn/
> %{johnpye}/datareader/
> ...
> you should get the idea.

Ops! Important inattention.
Fixed.

> 
> 
> 7. SHOULD consider a -libs subpkg for 
> %{_libdir}/libascend.so.*
> This is mostly to be multilib friendly, as anything matching
> %{_libdir}/lib*.so.* will get multilib'd automatically (e.g. pkg.i686 will
> get pulled into x86_64 repo).

All libraries packaged in a -libs sub-package.

Spec URL: http://sagitter.fedorapeople.org/ascend/ascend.spec
SRPM URL: http://sagitter.fedorapeople.org/ascend/ascend-0.9.8-3.20140211svn4638.fc20.src.rpm

Ascend may provide a Tcl/Tk user interface by the tktable. The latter package was packaged in Fedora until two years ago.

Recently (May of 2013), it seems to have been resumed: http://sourceforge.net/projects/tktable/.
Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6559860

It must be reviewed again. What do you think ?

Spec URL: http://sagitter.fedorapeople.org/ascend/ascend.spec
SRPM URL: http://sagitter.fedorapeople.org/ascend/ascend-0.9.8-4.20140211svn4638.fc20.src.rpm

- Excluded CMSLV solver
- Included IDA solver option with patch

My apologies for failing to notice your updating this... I'll try to finish this up by tomorrow.

License: OK

Item 3 above not completely fixed, there's still one reference to 
%{python_sitearch}

8.  -libs subpkg MUST include ldconfig scriptlets (and now main pkg no longer needs them)

9.  nothing depends on new -libs subpkg.  You probably want to add a dependency to at least the main and -devel subpkgs:
Requires: %{name}-libs%{?_isa} = %{version}-%{release}

(and have -devel drop dep on main pkg).

While we're at it, make -libs, -data dep on main pkg *not* use %_isa (else you lose the whole point of making a -libs subpkg to be multilib -friendly)

10.  nothing depends on -data subpkg, should main pkg depend on it?  Or is it optional addon?

11. While on the topic of -libs, I'd recommend it *only* include the shared library here, plus any essential resources the library may require.  Is there a reason you added all that other stuff too?

12.  MUST omit extraneous python dep
Requires: python >= 2.4
https://fedoraproject.org/wiki/Packaging:Python#Multiple_Python_Runtimes
(it'll get an automatic dependency on python(abi) instead)

13. MUST omit gcc-c++ dependency,
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Exceptions_2

14. URL: http://ascend.cheme.cmu.edu/
seems inaccessible (at the moment?).  Googling found me http://ascend4.org/ too, but it's not working either.  Please verify which URL is correct.

(In reply to Rex Dieter from comment #12)
> License: OK
> 
> Item 3 above not completely fixed, there's still one reference to 
> %{python_sitearch}

Done.
> 
> 8.  -libs subpkg MUST include ldconfig scriptlets (and now main pkg no
> longer needs them)

Done.
> 
> 9.  nothing depends on new -libs subpkg.  You probably want to add a
> dependency to at least the main and -devel subpkgs:
> Requires: %{name}-libs%{?_isa} = %{version}-%{release}
> 
> (and have -devel drop dep on main pkg).
> 
> While we're at it, make -libs, -data dep on main pkg *not* use %_isa (else
> you lose the whole point of making a -libs subpkg to be multilib -friendly)

Okay.

> 
> 10.  nothing depends on -data subpkg, should main pkg depend on it?  Or is
> it optional addon?

Honestly, I had wanted to split all files in a better way depending on their usage. Effectively -data files can be useful for testing and directly called by main software.


> 
> 11. While on the topic of -libs, I'd recommend it *only* include the shared
> library here, plus any essential resources the library may require.  Is
> there a reason you added all that other stuff too?

No particular reason, just to keep all libraries in same package.

> 
> 12.  MUST omit extraneous python dep
> Requires: python >= 2.4
> https://fedoraproject.org/wiki/Packaging:Python#Multiple_Python_Runtimes
> (it'll get an automatic dependency on python(abi) instead)
> 
> 13. MUST omit gcc-c++ dependency,
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#Exceptions_2
> 
> 14. URL: http://ascend.cheme.cmu.edu/
> seems inaccessible (at the moment?).  Googling found me http://ascend4.org/
> too, but it's not working either.  Please verify which URL is correct.

All surplus coming from old packages.
Fixed.

Spec URL: http://sagitter.fedorapeople.org/ascend/ascend.spec
SRPM URL: http://sagitter.fedorapeople.org/ascend/ascend-0.9.8-5.20140211svn4638.fc20.src.rpm

item 8 was fixed incorrectly.  -libs scriptlet should be ldconfig *only* the rest of the stuff left in the main pkg scriptlet.  Do this:

%post libs -p /sbin/ldconfig
%postun -libs -p /sbin/ldconfig

%post
update-desktop-database &> /dev/null || :
update-mime-database /usr/share/mime &> /dev/null || :
touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :

%postun
update-desktop-database &> /dev/null || :
update-mime-database /usr/share/mime &> /dev/null || :
if [ $1 -eq 0 ] ; then
    touch --no-create %{_datadir}/icons/hicolor &>/dev/null
    gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
fi

%posttrans
gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :


item 9 is not completely fixed, both -data and -libs still use %{_isa} in dep on main package, which we want to avoid.  In short, switch:
Requires: %{name}%{?_isa} = %{version}-%{release}
to
Requires: %{name} = %{version}-%{release}

just noticed a typo in my comment about item 8, theres an extra - in there.  use: 
%post libs -p /sbin/ldconfig
%postun libs -p /sbin/ldconfig
...

(In reply to Rex Dieter from comment #14)
> item 8 was fixed incorrectly.  -libs scriptlet should be ldconfig *only* the
> rest of the stuff left in the main pkg scriptlet.  Do this:
> 
> %post libs -p /sbin/ldconfig
> %postun -libs -p /sbin/ldconfig
> 
> %post
> update-desktop-database &> /dev/null || :
> update-mime-database /usr/share/mime &> /dev/null || :
> touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :
> 
> %postun
> update-desktop-database &> /dev/null || :
> update-mime-database /usr/share/mime &> /dev/null || :
> if [ $1 -eq 0 ] ; then
>     touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>     gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
> fi
> 
> %posttrans
> gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
> 
> 
> item 9 is not completely fixed, both -data and -libs still use %{_isa} in
> dep on main package, which we want to avoid.  In short, switch:
> Requires: %{name}%{?_isa} = %{version}-%{release}
> to
> Requires: %{name} = %{version}-%{release}

Done.
I left same release number.

Spec URL: http://sagitter.fedorapeople.org/ascend/ascend.spec
SRPM URL: http://sagitter.fedorapeople.org/ascend/ascend-0.9.8-5.20140211svn4638.fc20.src.rpm

-data still has:
Requires: %{name}%{?_isa} = %{version}-%{release}
should be:
Requires: %{name} = %{version}-%{release}

-devel has:
Requires: %{name}-libs = %{version}-%{release}
should be:
Requires: %{name}-libs%{?_isa} = %{version}-%{release}


I'm curious now, if there's a circular dependency between the main pkg (Requires: %{name}-data) and -data (Requires: %{name}), why even make a -data subpkg?

(In reply to Rex Dieter from comment #17)
> -data still has:
> Requires: %{name}%{?_isa} = %{version}-%{release}
> should be:
> Requires: %{name} = %{version}-%{release}
> 
> -devel has:
> Requires: %{name}-libs = %{version}-%{release}
> should be:
> Requires: %{name}-libs%{?_isa} = %{version}-%{release}
> 
> 
> I'm curious now, if there's a circular dependency between the main pkg
> (Requires: %{name}-data) and -data (Requires: %{name}), why even make a
> -data subpkg?

Initially, I prefered to split all files both to put them in order and to organise them in the better manner; even if they may be packaged in a single rpm.

Briefly:

main rpm
 Requires: %{name}-libs = %{version}-%{release}
 Requires: %{name}-data = %{version}-%{release}

-libs rpm
 Requires: %{name}%{?_isa} = %{version}-%{release}

-devel rpm
 Requires: %{name}-libs%{?_isa} = %{version}-%{release}

-data rpm 
 Requires: %{name} = %{version}-%{release}

Is it okay?

No.  in short, you want all references to the main pkg to *not* use %{_isa} and any reference to -libs or -data to use %{_isa}

These 2 need to be:

main rpm
 Requires: %{name}-libs%{?_isa} = %{version}-%{release}
 Requires: %{name}-data%{?_isa} = %{version}-%{release}

-libs rpm
  Requires: %{name} = %{version}-%{release}

(In reply to Rex Dieter from comment #20)
> No.  in short, you want all references to the main pkg to *not* use %{_isa}
> and any reference to -libs or -data to use %{_isa}
> 
> 
> -libs rpm
>   Requires: %{name} = %{version}-%{release}

-libs should not consider also the architecture?

Correct, dependencies should not consider architecture (ie, and use %{_isa}) for packages that are not (or that you don't want to be ) multilib'd.  In this case, we do not want the main package to be multilib'd.

Otherwise, there is no point in making a -libs subpkg at all.

Sorry for the confusion.

Spec URL: http://sagitter.fedorapeople.org/ascend/ascend.spec
SRPM URL: http://sagitter.fedorapeople.org/ascend/ascend-0.9.8-5.20140211svn4638.fc20.src.rpm

OK, looks like we have a winner, APPROVED.

Thank you very much Rex.

New Package SCM Request
=======================
Package Name: ascend
Short Description:  ASCEND modelling environment
Upstream URL: http://ascend4.org/
Owners: sagitter
Branches: f20

Git done (by process-git-requests).

ascend-0.9.8-6.20140211svn4638.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/ascend-0.9.8-6.20140211svn4638.fc20

Package ascend-0.9.8-6.20140211svn4638.fc20:
* should fix your issue,
* was pushed to the Fedora 20 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing ascend-0.9.8-6.20140211svn4638.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-7058/ascend-0.9.8-6.20140211svn4638.fc20
then log in and leave karma (feedback).

ascend-0.9.8-6.20140211svn4638.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.