Bug 1490054 - Review Request: scidavis - Application for Scientific Data Analysis and Visualization
Summary: Review Request: scidavis - Application for Scientific Data Analysis and Visua...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Antonio T. (sagitter)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-09-09 17:56 UTC by Alexander Ploumistos
Modified: 2017-10-17 02:19 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-10-09 19:58:30 UTC
Type: ---
Embargoed:
anto.trande: fedora-review+


Attachments (Terms of Use)
SPEC file changes (3.88 KB, patch)
2017-09-13 19:20 UTC, Antonio T. (sagitter)
no flags Details | Diff
config.pri changes (967 bytes, patch)
2017-09-13 19:22 UTC, Antonio T. (sagitter)
no flags Details | Diff
Test results (3.58 KB, text/plain)
2017-09-14 17:52 UTC, Alexander Ploumistos
no flags Details

Description Alexander Ploumistos 2017-09-09 17:56:49 UTC
Spec URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis.spec
SRPM URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis-1.21-1.fc28.src.rpm

Description: SciDAVis is a free interactive application aimed at data analysis and publication-quality plotting. It combines a shallow learning curve and an intuitive, easy-to-use graphical user interface with powerful features such as scriptability and extensibility.

Fedora Account System Username: alexpl

no koji builds, as it depends on the unbundled version of liborigin3 (actually liborigin3-devel)
https://bugzilla.redhat.com/show_bug.cgi?id=1490053

COPR: https://copr.fedorainfracloud.org/coprs/alexpl/scidavis/

Note: An AppData file has been submitted upstream, it will be included in the next version of SciDAVis
https://github.com/highperformancecoder/scidavis/pull/50

Note 2: Towards the end of the build, there is a warning about files in /usr/share/doc/ and /usr/share/scidavis/translations/ being listed twice. See for example https://copr-be.cloud.fedoraproject.org/results/alexpl/scidavis/fedora-rawhide-i386/00591823-scidavis/build.log.gz )
 What have I missed that is causing this?

Comment 1 Artur Frenszek-Iwicki 2017-09-09 19:32:38 UTC
>%{__mkdir_p}
>%{__cp} -p
>...
"Macro forms of system executables SHOULD NOT be used except when there is a need to allow the location of those executables to be configurable."
https://fedoraproject.org/wiki/Packaging:Guidelines#Macros

The "x-sciprj.desktop" file is not passed through desktop-file-validate - is this intentional?
Apart from those two issues, seems fine to me.

Comment 2 Alexander Ploumistos 2017-09-09 20:31:51 UTC
Hello Artur,

Thanks for looking into this.

(In reply to Iwicki Artur from comment #1)
> >%{__mkdir_p}
> >%{__cp} -p
> >...
> "Macro forms of system executables SHOULD NOT be used except when there is a
> need to allow the location of those executables to be configurable."
> https://fedoraproject.org/wiki/Packaging:Guidelines#Macros

I think I had picked this up from firefox's spec file. I had seen the section you have quoted, but as I was uncertain about the part of needing "to allow the location of those executables to be configurable", I went with that. Should those be plain cp or /usr/bin/cp instead?

> The "x-sciprj.desktop" file is not passed through desktop-file-validate - is
> this intentional?

When I first saw this file, I noticed it wasn't a "regular" desktop file and the only thing on the wiki about its location (/usr/share/mimelnk) was this KDE SIG page
https://fedoraproject.org/wiki/SIGs/KDE/Packaging/Cleanup
so I just left it there. Now that you've mentioned it, I can't find anything about desktop files referencing only mimetypes on freedesktop.org, plus on my fedora machines, all the subdirectories under /usr/share/mimelnk are empty; only on one of them, that has been updated since f17 or f19, I have some mimetype.desktop files, which date back to 2008. So, I guess they might have been deprecated in the meantime and the file should be removed? Do you know something about any of that?

> Apart from those two issues, seems fine to me.

Do you have any insights on those "File listed twice" warnings?

Comment 3 Artur Frenszek-Iwicki 2017-09-11 14:52:16 UTC
>Should those be plain cp or /usr/bin/cp instead?
I don't think I've ever seen absolute paths for commands while doing package reviews. Plain cp/mkdir/... should be okay.

>Do you have any insights on those "File listed twice" warnings?
My guess is that some of the files found by %find_lang also match the wildcards in %files. (Sorry, I haven't tried building the package locally, yet. Will confirm later.)

Comment 4 Alexander Ploumistos 2017-09-11 15:01:44 UTC
Great, thanks. I'll hold off submitting an updated spec file, until I get some input on the other review request, #1490053, so take your time.

Comment 5 Alexander Ploumistos 2017-09-12 02:13:44 UTC
Turns out that desktop files in /usr/share/mimelnk were used in KDE3 and they are obsolete, so I have removed the file from the package.

I've also identified an issue with the manpage - I fixed it and I will report it upstream.

While trying to figure out why those files are listed twice, I realized that the developers had fixed a number of issues with their makefiles, so a lot of code from the spec file was no longer necessary - I removed it.

By the way you were right, that warning about files being listed twice came from an overlap between %doc, %find_lang and the directives in the generated makefile. I couldn't patch it, but I will take this upstream as well.

After a discussion with Antonio Trande (see #1490053) we decided it would be better to keep liborigin bundled until there is an official liborigin-3.0.0 release. At least nothing depends on it and it's only required at build time.

Updated spec file and source rpm:

Spec URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis.spec
SRPM URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis-1.21-2.fc28.src.rpm

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

copr build: https://copr.fedorainfracloud.org/coprs/alexpl/scidavis/build/601104/

Comment 6 Alexander Ploumistos 2017-09-12 16:19:28 UTC
Sorry for the late addition, but I just realized that while the build dependencies where there in the spec file, Python scripting support was silently dropped in one of the 1.D8-x releases. My guess is that it probably failed to build at that time.

I have amended the spec file and submitted new builds. I also did some tests in the program and it seems to work fine.

However, it has failed to build on armv7hl:
https://koji.fedoraproject.org/koji/taskinfo?taskID=21822435

Should I revert the change or should I ExcludeArch armv7hl while I investigate the issue?


Spec URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis.spec
SRPM URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis-1.21-3.fc28.src.rpm

copr build: https://copr.fedorainfracloud.org/coprs/alexpl/scidavis/build/601418/

Comment 7 Antonio T. (sagitter) 2017-09-12 17:16:15 UTC
> Should I revert the change or should I ExcludeArch armv7hl while I investigate 
> the issue?

No; the compiler aborts because it is missing the sipAPIscidavis.h file; it's generated during compilation.

Use 1 job only on ARM:

make -j1

Comment 8 Alexander Ploumistos 2017-09-12 17:28:19 UTC
Do you mean like this?


%build
%if 0%{?__isa_bits} == 64
%qmake_qt4 PRESET=linux_package libsuff="64" CONFIG+=liborigin CONFIG+=python
%else
%qmake_qt4 PRESET=linux_package CONFIG+=liborigin CONFIG+=python
%endif

%ifarch armv7hl
make -j1
%else
make %{?_smp_mflags}
%endif

Comment 9 Antonio T. (sagitter) 2017-09-12 17:31:25 UTC
(In reply to Alexander Ploumistos from comment #8)
> Do you mean like this?
> 
> 
...
> 
> %ifarch armv7hl
> make -j1
> %else
> make %{?_smp_mflags}
> %endif

Almost,

%ifarch %{arm}
make -j1
%else
%make_build
%endif

Comment 10 Alexander Ploumistos 2017-09-12 18:15:56 UTC
It failed again:

https://koji.fedoraproject.org/koji/taskinfo?taskID=21825105

Is it the same error? I still see

src/PythonScripting.cpp:65:10: fatal error: sipAPIscidavis.h: No such file or directory
 #include "sipAPIscidavis.h"

Comment 11 Alexander Ploumistos 2017-09-12 22:40:54 UTC
I have reported the issue upstream:

https://sourceforge.net/p/scidavis/scidavis-bugs/317/

Comment 12 Antonio T. (sagitter) 2017-09-13 09:27:08 UTC
> I have reported the issue upstream:
> 
> https://sourceforge.net/p/scidavis/scidavis-bugs/317/

Okay.

> %config(noreplace) %{_sysconfdir}/scidavisrc.py
> %exclude %{_sysconfdir}/scidavisrc.pyc
> %exclude %{_sysconfdir}/scidavisrc.pyo
> %exclude %{_datadir}/%{name}/scidavisUtil.pyc
> %exclude %{_datadir}/%{name}/scidavisUtil.pyo

All Python files must be installed in their related paths inside a dedicated python2-scidavis (and/or python3) sub-package: http://fedoraproject.org/wiki/Packaging:Python#Macros

> /usr/bin/update-desktop-database &> /dev/null || :

Deprecated on Fedora > 25.

- Please, add an appdata file.
- You can avoid using %find_lang because does not work in this case.
- Use %doc or %{_docdir}/%{name}, not both.

Comment 13 Alexander Ploumistos 2017-09-13 10:40:05 UTC
(In reply to Antonio Trande from comment #12)
> > I have reported the issue upstream:
> > 
> > https://sourceforge.net/p/scidavis/scidavis-bugs/317/
> 
> Okay.

Robert-André Mauchin provided a patch on the devel mailing list. I will send a PR upstream and submit an updated spec and srpm once we get through the other issues.


> 
> > %config(noreplace) %{_sysconfdir}/scidavisrc.py
> > %exclude %{_sysconfdir}/scidavisrc.pyc
> > %exclude %{_sysconfdir}/scidavisrc.pyo
> > %exclude %{_datadir}/%{name}/scidavisUtil.pyc
> > %exclude %{_datadir}/%{name}/scidavisUtil.pyo
> 
> All Python files must be installed in their related paths inside a dedicated
> python2-scidavis (and/or python3) sub-package:
> http://fedoraproject.org/wiki/Packaging:Python#Macros

I don't understand how exactly that is supposed to work. It did seem weird to have those python scripts there, but supposedly they are some sort of configuration(?) files.

Should the subpackage place them in the same locations, or do I need to patch the makefile to get them to other paths?

Am I to use %{__python2}, %{python2_sitelib}, %{python2_sitearch} or none of them in this case?

Do I need to keep the byte compiled files as well?

Do I need extra Provides/Requires in the spec file?

Can you point me to a package (or more) that has such a subpackage, because I'm really lost on that one?


> 
> > /usr/bin/update-desktop-database &> /dev/null || :
> 
> Deprecated on Fedora > 25.

I will remove it. I suppose I can ignore the update-mime-database suggested by fedora-review as well?


> 
> - Please, add an appdata file.

I have submitted a file upstream, which is to be included in 1.22:
https://github.com/highperformancecoder/scidavis/pull/50
https://github.com/highperformancecoder/scidavis/pull/52

I will add it to 1.21 as well.


> - You can avoid using %find_lang because does not work in this case.

So I just copy the files over to %{_datadir}/%{name}/translations/ and leave it at that, no other declaration in the %files section?


> - Use %doc or %{_docdir}/%{name}, not both.

There is that CHANGES file that should go to /usr/share/doc/scidavis/ and then there is an html manual (under construction) that gets installed there from the makefile. If I don't use %doc, CHANGES does not get installed and if I use
%doc CHANGES manual/index.html
I get an added index.html outside the manual directory. Should I just drop %doc and say goodbye to the (unmaintained) CHANGES file?


I'm willing to bet that you are starting to regret picking this up… Please bear with me!

Comment 14 Alexander Ploumistos 2017-09-13 10:52:29 UTC
(In reply to Alexander Ploumistos from comment #13)
> Should the subpackage place them in the same locations, or do I need to
> patch the makefile to get them to other paths?

And a bonus question, if I keep them at the same locations, is there a problem with
%config(noreplace) %{_sysconfdir}/scidavisrc.py
since it's supposed to be a configuration file?

Comment 15 Antonio T. (sagitter) 2017-09-13 19:20:03 UTC
(In reply to Alexander Ploumistos from comment #13)
> 
> 
> > 
> > > %config(noreplace) %{_sysconfdir}/scidavisrc.py
> > > %exclude %{_sysconfdir}/scidavisrc.pyc
> > > %exclude %{_sysconfdir}/scidavisrc.pyo
> > > %exclude %{_datadir}/%{name}/scidavisUtil.pyc
> > > %exclude %{_datadir}/%{name}/scidavisUtil.pyo
> > 
> > All Python files must be installed in their related paths inside a dedicated
> > python2-scidavis (and/or python3) sub-package:
> > http://fedoraproject.org/wiki/Packaging:Python#Macros
> 
> I don't understand how exactly that is supposed to work. It did seem weird
> to have those python scripts there, but supposedly they are some sort of
> configuration(?) files.
> 
> Should the subpackage place them in the same locations, or do I need to
> patch the makefile to get them to other paths?
> 
> Am I to use %{__python2}, %{python2_sitelib}, %{python2_sitearch} or none of
> them in this case?
> 
> Do I need to keep the byte compiled files as well?
> 
> Do I need extra Provides/Requires in the spec file?
> 
> Can you point me to a package (or more) that has such a subpackage, because
> I'm really lost on that one?
> 
> 
> > 
> > > /usr/bin/update-desktop-database &> /dev/null || :
> > 
> > Deprecated on Fedora > 25.
> 
> I will remove it. I suppose I can ignore the update-mime-database suggested
> by fedora-review as well?
> 
> 

Yes.

> > 
> > - Please, add an appdata file.
> 
> I have submitted a file upstream, which is to be included in 1.22:
> https://github.com/highperformancecoder/scidavis/pull/50
> https://github.com/highperformancecoder/scidavis/pull/52
> 
> I will add it to 1.21 as well.
> 
> 
> > - You can avoid using %find_lang because does not work in this case.
> 
> So I just copy the files over to %{_datadir}/%{name}/translations/ and leave
> it at that, no other declaration in the %files section?

Yes.

> 
> 
> > - Use %doc or %{_docdir}/%{name}, not both.
> 
> There is that CHANGES file that should go to /usr/share/doc/scidavis/ and
> then there is an html manual (under construction) that gets installed there
> from the makefile. If I don't use %doc, CHANGES does not get installed and
> if I use
> %doc CHANGES manual/index.html
> I get an added index.html outside the manual directory. Should I just drop
> %doc and say goodbye to the (unmaintained) CHANGES file?

See attached diff files.

> 
> 
> I'm willing to bet that you are starting to regret picking this up… Please
> bear with me!

It isn't a big deal. ;)

Comment 16 Antonio T. (sagitter) 2017-09-13 19:20:56 UTC
Created attachment 1325591 [details]
SPEC file changes

Comment 17 Antonio T. (sagitter) 2017-09-13 19:22:46 UTC
Created attachment 1325592 [details]
config.pri changes

Patch for setting Python paths.
Set QWT library.

Comment 18 Antonio T. (sagitter) 2017-09-13 19:26:41 UTC
(In reply to Antonio Trande from comment #17)
> Created attachment 1325592 [details]
> config.pri changes
> 
> Patch for setting Python paths.
> Set QWT library.

There is an error in this patch:

modify

> pythonutils.path = "$$INSTALLBASE/share/scidavis"

with

> pythonutils.path = "$$INSTALLBASE/..$$QMAKE_LIBDIR_QT/python2.7/site-packages/scidavis"

Comment 19 Alexander Ploumistos 2017-09-14 02:00:48 UTC
Thank you so much Antonio, it would take me months to figure out the python bits.

I have amended the spec file, added the new patches and I also added a weak dependency on the subpackage.


(In reply to Antonio Trande from comment #17)
> Set QWT library.

Was this necessary? This was the original conditional, and the second branch was what we have in Fedora, no?

system (ls /usr/lib*/libqwt5.so) {
   LIBS+=-lqwt5
} else {
   system (ls /usr/lib*/libqwt5-qt4.so) {
      LIBS+=-lqwt5-qt4
   } else {
     system (ls /usr/lib*/libqwt-qt4.so) {
       LIBS+=-lqwt-qt4
     } else {LIBS+=-lqwt}
   }
}


Spec URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis.spec
SRPM URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis-1.21-4.fc28.src.rpm

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

Comment 20 Antonio T. (sagitter) 2017-09-14 10:29:35 UTC
(In reply to Alexander Ploumistos from comment #19)
> 
> (In reply to Antonio Trande from comment #17)
> > Set QWT library.
> 
> Was this necessary? This was the original conditional, and the second branch
> was what we have in Fedora, no?
> 
> system (ls /usr/lib*/libqwt5.so) {
>    LIBS+=-lqwt5
> } else {
>    system (ls /usr/lib*/libqwt5-qt4.so) {
>       LIBS+=-lqwt5-qt4
>    } else {
>      system (ls /usr/lib*/libqwt-qt4.so) {
>        LIBS+=-lqwt-qt4
>      } else {LIBS+=-lqwt}
>    }
> }
> 
> 

It is needed just to avoid an access error:

ls: cannot access '/usr/lib*/libqwt5.so': No such file or directory
/usr/lib64/libqwt5-qt4.so
/usr/lib64/libqwtplot3d-qt4.so

I will post the review.

Comment 21 Antonio T. (sagitter) 2017-09-14 10:45:47 UTC
Before reviewing, please add the patch for fixing the bug #316: https://sourceforge.net/p/scidavis/scidavis-bugs/316/

You can also run the tests by running 'unittests' file inside 'test' directory:

xvfb-run -a ./unittests

Comment 22 Alexander Ploumistos 2017-09-14 16:19:03 UTC
(In reply to Antonio Trande from comment #21)
> Before reviewing, please add the patch for fixing the bug #316:
> https://sourceforge.net/p/scidavis/scidavis-bugs/316/

I have backported the patch, but I didn't bump the version, so the links remain the same.

Fresh build: https://koji.fedoraproject.org/koji/taskinfo?taskID=21862381

> You can also run the tests by running 'unittests' file inside 'test'
> directory:
> 
> xvfb-run -a ./unittests

Are these worth the Xorg dependencies? If you think they are, I will add them too.

Comment 23 Antonio T. (sagitter) 2017-09-14 17:07:48 UTC
(In reply to Alexander Ploumistos from comment #22)
> > You can also run the tests by running 'unittests' file inside 'test'
> > directory:
> > 
> > xvfb-run -a ./unittests
> 
> Are these worth the Xorg dependencies? If you think they are, I will add
> them too.

Just 'xorg-x11-server-Xvfb', i think.

Comment 24 Alexander Ploumistos 2017-09-14 17:52:49 UTC
Created attachment 1326150 [details]
Test results

(In reply to Antonio Trande from comment #23)
> (In reply to Alexander Ploumistos from comment #22)
> > > You can also run the tests by running 'unittests' file inside 'test'
> > > directory:
> > > 
> > > xvfb-run -a ./unittests
> > 
> > Are these worth the Xorg dependencies? If you think they are, I will add
> > them too.
> 
> Just 'xorg-x11-server-Xvfb', i think.

The dependencies that get pulled in are ~2MB of downloads, plus another ~10MB on disk (in other words, a drop in the ocean).

I added xorg-x11-server-Xvfb to the BuildRequires and "cd test && xvfb-run -a ./unittests" in %check.

I am attaching the test results from a local mock build, are they interesting enough to keep testing enabled?

Comment 25 Antonio T. (sagitter) 2017-09-14 18:13:55 UTC
Choice is your; they're not essential in my opinion.

Comment 26 Alexander Ploumistos 2017-09-14 18:22:17 UTC
It's good to have an extra check, so I'll keep them enabled for local builds and disable them before submitting to koji.

Comment 27 Antonio T. (sagitter) 2017-09-16 12:28:13 UTC
Package Review
==============

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


Issues:
=======
- Package does not use a name that already exists.
  Note: A package with this name already exists. Please check
  https://admin.fedoraproject.org/pkgdb/package/scidavis
  See:
  https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Conflicting_Package_Names

This is review is considered as an "unorphaning" process.

- update-desktop-database is invoked in %post and %postun if package
  contains desktop file(s) with a MimeType: entry.
  Note: desktop file(s) with MimeType entry in scidavis
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-
  database

No longer used on Fedora.


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[-]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[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: "GPL", "GPL (v3)", "GPL (v2 or later)", "*No copyright* GPL
     (v2)", "Unknown or generated", "zlib/libpng", "*No copyright* GPL (v2
     or later)", "GPL (v2)", "GPL (v3 or later)". 343 files have unknown
     license. Detailed output of licensecheck in
     /home/sagitter/1490054-scidavis/licensecheck.txt

GPLv2+ and GPLv3+ are compatible licenses; you can use GPLv3+ as resultant 
license.

[x]: License file installed when any subpackage combination is installed.
[x]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[?]: Package must own all directories that it creates.
     Note: Directories without known owners:
     /usr/share/icons/locolor/16x16, /usr/share/icons/locolor/32x32/apps,
     /usr/share/icons/locolor/16x16/apps, /usr/share/mime,
     /usr/share/mime/packages, /usr/share/icons/locolor,
     /usr/share/icons/locolor/32x32, /usr/share/icons/locolor/22x22,
     /usr/share/icons/locolor/22x22/apps

'/usr/share/icons/locolor' looks not owned by any package.
I think you can permit 'scidavis' owns it, use:

%dir %{_datadir}/icons/locolor

[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.
[-]: Development files must be in a -devel package
[x]: 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.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
     contains icons.
     Note: icons in scidavis
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 30720 bytes in 4 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: 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.
[x]: Package requires other packages for directories it uses.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[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]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or
     desktop-file-validate if there is such a file.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[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

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: update-mime-database is invoked in %post and %postun if package stores
     mime configuration in /usr/share/mime/packages.
     Note: mimeinfo files in: scidavis
     See:
     http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#mimeinfo

No longer used on Fedora.
 
[-]: 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).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     python2-scidavis , scidavis-debuginfo
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[-]: 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.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[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]: 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 2191360 bytes in /usr/share
[x]: Rpmlint is run on debuginfo package(s).
     Note: There are rpmlint messages (see attachment).
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: scidavis-1.21-4.fc28.x86_64.rpm
          python2-scidavis-1.21-4.fc28.x86_64.rpm
          scidavis-debuginfo-1.21-4.fc28.x86_64.rpm
          scidavis-1.21-4.fc28.src.rpm
scidavis.x86_64: W: spelling-error %description -l en_US scriptability -> script ability, script-ability, inscrutability
scidavis.x86_64: W: spelling-error %description -l en_US extensibility -> sensibility, extensible
python2-scidavis.x86_64: W: only-non-binary-in-usr-lib
python2-scidavis.x86_64: W: no-documentation
scidavis-debuginfo.x86_64: E: useless-provides debuginfo(build-id)
scidavis.src: W: spelling-error %description -l en_US scriptability -> script ability, script-ability, inscrutability
scidavis.src: W: spelling-error %description -l en_US extensibility -> sensibility, extensible
scidavis.src:72: W: macro-in-comment %patch1
scidavis.src: W: %ifarch-applied-patch Patch0: scidavis-armv7hl-build.patch
4 packages and 0 specfiles checked; 1 errors, 8 warnings.




Rpmlint (debuginfo)
-------------------
Checking: scidavis-debuginfo-1.21-4.fc28.x86_64.rpm
scidavis-debuginfo.x86_64: E: useless-provides debuginfo(build-id)
1 packages and 0 specfiles checked; 1 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
scidavis.x86_64: W: spelling-error %description -l en_US scriptability -> script ability, script-ability, inscrutability
scidavis.x86_64: W: spelling-error %description -l en_US extensibility -> sensibility, extensible
python2-scidavis.x86_64: W: only-non-binary-in-usr-lib
python2-scidavis.x86_64: W: no-documentation
scidavis-debuginfo.x86_64: E: useless-provides debuginfo(build-id)
3 packages and 0 specfiles checked; 1 errors, 4 warnings.



Requires
--------
scidavis (rpmlib, GLIBC filtered):
    /bin/sh
    PyQt4
    hicolor-icon-theme
    libGL.so.1()(64bit)
    libGLU.so.1()(64bit)
    libQt3Support.so.4()(64bit)
    libQtCore.so.4()(64bit)
    libQtGui.so.4()(64bit)
    libQtNetwork.so.4()(64bit)
    libQtOpenGL.so.4()(64bit)
    libQtSql.so.4()(64bit)
    libQtSvg.so.4()(64bit)
    libQtXml.so.4()(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.0)(64bit)
    libgsl.so.23()(64bit)
    libgslcblas.so.0()(64bit)
    libm.so.6()(64bit)
    libmuparser.so.2()(64bit)
    libpthread.so.0()(64bit)
    libpython2.7.so.1.0()(64bit)
    libqwt.so.5()(64bit)
    libqwtplot3d-qt4.so.0()(64bit)
    libstdc++.so.6()(64bit)
    libstdc++.so.6(CXXABI_1.3)(64bit)
    libstdc++.so.6(CXXABI_1.3.8)(64bit)
    libz.so.1()(64bit)
    rtld(GNU_HASH)

python2-scidavis (rpmlib, GLIBC filtered):
    python(abi)
    python2-scipy
    scidavis(x86-64)

scidavis-debuginfo (rpmlib, GLIBC filtered):



Provides
--------
scidavis:
    application()
    application(scidavis.desktop)
    libexp_saturation.so.1()(64bit)
    libexplin.so.1()(64bit)
    libfitRational0.so.1()(64bit)
    libfitRational1.so.1()(64bit)
    libplanck_wavelength.so.1()(64bit)
    metainfo()
    metainfo(scidavis.appdata.xml)
    mimehandler(application/x-sciprj)
    scidavis
    scidavis(x86-64)

python2-scidavis:
    python-scidavis
    python-scidavis(x86-64)
    python2-scidavis
    python2-scidavis(x86-64)

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



Unversioned so-files
--------------------
scidavis: /usr/lib64/scidavis/plugins/libexp_saturation.so
scidavis: /usr/lib64/scidavis/plugins/libexplin.so
scidavis: /usr/lib64/scidavis/plugins/libfitRational0.so
scidavis: /usr/lib64/scidavis/plugins/libfitRational1.so
scidavis: /usr/lib64/scidavis/plugins/libplanck_wavelength.so

Source checksums
----------------
http://downloads.sourceforge.net/scidavis/scidavis-1.21.tar.gz :
  CHECKSUM(SHA256) this package     : 0e103d4669578205226568eb39b4d138bb99d1c32c3724b40d59810aa64ea05b
  CHECKSUM(SHA256) upstream package : 0e103d4669578205226568eb39b4d138bb99d1c32c3724b40d59810aa64ea05b


Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 1490054
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Generic, Shell-api, C/C++
Disabled plugins: Java, SugarActivity, fonts, Haskell, Ocaml, Perl, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 28 Alexander Ploumistos 2017-09-16 15:01:52 UTC
(In reply to Antonio Trande from comment #27)
> Issues:
> =======
> - Package does not use a name that already exists.
>   Note: A package with this name already exists. Please check
>   https://admin.fedoraproject.org/pkgdb/package/scidavis
>   See:
>  
> https://fedoraproject.org/wiki/Packaging/
> NamingGuidelines#Conflicting_Package_Names
> 
> This is review is considered as an "unorphaning" process.
> 
> - update-desktop-database is invoked in %post and %postun if package
>   contains desktop file(s) with a MimeType: entry.
>   Note: desktop file(s) with MimeType entry in scidavis
>   See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-
>   database
> 
> No longer used on Fedora.

Nothing to do here, right?

> GPLv2+ and GPLv3+ are compatible licenses; you can use GPLv3+ as resultant 
> license.

Really??? It always seemed logical, but since I am not proficient in legalese I didn't question it. Do you want me to update the spec, or should I leave it for the next build?

> [?]: Package must own all directories that it creates.
>      Note: Directories without known owners:
>      /usr/share/icons/locolor/16x16, /usr/share/icons/locolor/32x32/apps,
>      /usr/share/icons/locolor/16x16/apps, /usr/share/mime,
>      /usr/share/mime/packages, /usr/share/icons/locolor,
>      /usr/share/icons/locolor/32x32, /usr/share/icons/locolor/22x22,
>      /usr/share/icons/locolor/22x22/apps
> 
> '/usr/share/icons/locolor' looks not owned by any package.
> I think you can permit 'scidavis' owns it, use:
> 
> %dir %{_datadir}/icons/locolor

On my system, I see a bunch of icons under /usr/share/icons/locolor, belonging to libreoffice, kimagemapeditor, krename and kxsldbg. I don't think scidavis should take ownership…



> Rpmlint
> -------
> Checking: scidavis-1.21-4.fc28.x86_64.rpm
>           python2-scidavis-1.21-4.fc28.x86_64.rpm
>           scidavis-debuginfo-1.21-4.fc28.x86_64.rpm
>           scidavis-1.21-4.fc28.src.rpm
> scidavis.x86_64: W: spelling-error %description -l en_US scriptability ->
> script ability, script-ability, inscrutability
> scidavis.x86_64: W: spelling-error %description -l en_US extensibility ->
> sensibility, extensible
> python2-scidavis.x86_64: W: only-non-binary-in-usr-lib
> python2-scidavis.x86_64: W: no-documentation
> scidavis-debuginfo.x86_64: E: useless-provides debuginfo(build-id)

Are these errors because rpmlint hasn't caught up with the system wide debugging changes, or have I messed up something?

Comment 29 Antonio T. (sagitter) 2017-09-16 16:18:33 UTC
(In reply to Alexander Ploumistos from comment #28)
> (In reply to Antonio Trande from comment #27)
> > Issues:
> > =======
> > - Package does not use a name that already exists.
> >   Note: A package with this name already exists. Please check
> >   https://admin.fedoraproject.org/pkgdb/package/scidavis
> >   See:
> >  
> > https://fedoraproject.org/wiki/Packaging/
> > NamingGuidelines#Conflicting_Package_Names
> > 
> > This is review is considered as an "unorphaning" process.
> > 
> > - update-desktop-database is invoked in %post and %postun if package
> >   contains desktop file(s) with a MimeType: entry.
> >   Note: desktop file(s) with MimeType entry in scidavis
> >   See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-
> >   database
> > 
> > No longer used on Fedora.
> 
> Nothing to do here, right?

Yes.

> 
> > GPLv2+ and GPLv3+ are compatible licenses; you can use GPLv3+ as resultant 
> > license.
> 
> Really??? It always seemed logical, but since I am not proficient in
> legalese I didn't question it. Do you want me to update the spec, or should
> I leave it for the next build?

Update, please.

> 
> > [?]: Package must own all directories that it creates.
> >      Note: Directories without known owners:
> >      /usr/share/icons/locolor/16x16, /usr/share/icons/locolor/32x32/apps,
> >      /usr/share/icons/locolor/16x16/apps, /usr/share/mime,
> >      /usr/share/mime/packages, /usr/share/icons/locolor,
> >      /usr/share/icons/locolor/32x32, /usr/share/icons/locolor/22x22,
> >      /usr/share/icons/locolor/22x22/apps
> > 
> > '/usr/share/icons/locolor' looks not owned by any package.
> > I think you can permit 'scidavis' owns it, use:
> > 
> > %dir %{_datadir}/icons/locolor
> 
> On my system, I see a bunch of icons under /usr/share/icons/locolor,
> belonging to libreoffice, kimagemapeditor, krename and kxsldbg. I don't
> think scidavis should take ownership…
> 
> 

Fine.
'/usr/share/icons/locolor' is owned by kde-filesystem; this means that we need install 'scidavis' and 'kde-filesystem' together.

> 
> > Rpmlint
> > -------
> > Checking: scidavis-1.21-4.fc28.x86_64.rpm
> >           python2-scidavis-1.21-4.fc28.x86_64.rpm
> >           scidavis-debuginfo-1.21-4.fc28.x86_64.rpm
> >           scidavis-1.21-4.fc28.src.rpm
> > scidavis.x86_64: W: spelling-error %description -l en_US scriptability ->
> > script ability, script-ability, inscrutability
> > scidavis.x86_64: W: spelling-error %description -l en_US extensibility ->
> > sensibility, extensible
> > python2-scidavis.x86_64: W: only-non-binary-in-usr-lib
> > python2-scidavis.x86_64: W: no-documentation
> > scidavis-debuginfo.x86_64: E: useless-provides debuginfo(build-id)
> 
> Are these errors because rpmlint hasn't caught up with the system wide
> debugging changes, or have I messed up something?

https://bugzilla.redhat.com/show_bug.cgi?id=1489096

Comment 30 Alexander Ploumistos 2017-09-16 17:00:14 UTC
(In reply to Antonio Trande from comment #29)
> (In reply to Alexander Ploumistos from comment #28)
> > (In reply to Antonio Trande from comment #27)
> > > GPLv2+ and GPLv3+ are compatible licenses; you can use GPLv3+ as resultant 
> > > license.
> > 
> > Really??? It always seemed logical, but since I am not proficient in
> > legalese I didn't question it. Do you want me to update the spec, or should
> > I leave it for the next build?
> 
> Update, please.

Updated.

> 
> > 
> > > [?]: Package must own all directories that it creates.
> > >      Note: Directories without known owners:
> > >      /usr/share/icons/locolor/16x16, /usr/share/icons/locolor/32x32/apps,
> > >      /usr/share/icons/locolor/16x16/apps, /usr/share/mime,
> > >      /usr/share/mime/packages, /usr/share/icons/locolor,
> > >      /usr/share/icons/locolor/32x32, /usr/share/icons/locolor/22x22,
> > >      /usr/share/icons/locolor/22x22/apps
> > > 
> > > '/usr/share/icons/locolor' looks not owned by any package.
> > > I think you can permit 'scidavis' owns it, use:
> > > 
> > > %dir %{_datadir}/icons/locolor
> > 
> > On my system, I see a bunch of icons under /usr/share/icons/locolor,
> > belonging to libreoffice, kimagemapeditor, krename and kxsldbg. I don't
> > think scidavis should take ownership…
> > 
> > 
> 
> Fine.
> '/usr/share/icons/locolor' is owned by kde-filesystem; this means that we
> need install 'scidavis' and 'kde-filesystem' together.

Added "Requires: kde-filesystem".

New files:

Spec URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis.spec
SRPM URL: https://alexpl.fedorapeople.org/packages/SciDAVis/scidavis-1.21-5.fc28.src.rpm

I ran f25, f26, f27 & rawhide builds locally, no problems. Here is another koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=21903257
(still building)

Comment 31 Antonio T. (sagitter) 2017-09-16 17:02:28 UTC
Approved.

Comment 32 Alexander Ploumistos 2017-09-16 17:36:39 UTC
Again, thank you very much Antonio, it was highly educational!

Comment 33 Fedora Update System 2017-10-02 16:39:35 UTC
scidavis-1.21-6.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-149d198190

Comment 34 Fedora Update System 2017-10-02 16:39:45 UTC
scidavis-1.21-6.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-bd25beedab

Comment 35 Fedora Update System 2017-10-02 16:39:51 UTC
scidavis-1.21-6.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2017-1db5fc1618

Comment 36 Fedora Update System 2017-10-06 03:22:15 UTC
scidavis-1.21-6.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-149d198190

Comment 37 Fedora Update System 2017-10-06 03:24:26 UTC
scidavis-1.21-6.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-bd25beedab

Comment 38 Fedora Update System 2017-10-06 04:24:35 UTC
scidavis-1.21-6.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-1db5fc1618

Comment 39 Fedora Update System 2017-10-09 19:58:30 UTC
scidavis-1.21-6.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.

Comment 40 Fedora Update System 2017-10-16 17:52:07 UTC
scidavis-1.21-6.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

Comment 41 Fedora Update System 2017-10-17 02:19:21 UTC
scidavis-1.21-6.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.


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