Bug 1758622 - Review Request: octave-mcxlab - A GPU Monte Carlo 3-D photon transport simulator for MATLAB/Octave
Summary: Review Request: octave-mcxlab - A GPU Monte Carlo 3-D photon transport simula...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: fedora-neuro, NeuroFedora
TreeView+ depends on / blocked
 
Reported: 2019-10-04 16:00 UTC by Qianqian Fang
Modified: 2019-10-26 17:23 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-10-07 20:13:49 UTC
Type: ---
Embargoed:
zebob.m: fedora-review+


Attachments (Terms of Use)
screenshot of the installed software - everything works (135.10 KB, image/png)
2019-10-04 16:36 UTC, Qianqian Fang
no flags Details

Description Qianqian Fang 2019-10-04 16:00:17 UTC
Spec URL: https://github.com/fangq/fedorapkg/blob/mcxlab/octave-mcxlab.spec
SRPM URL: https://kwafoo.coe.neu.edu/~fangq/share/temp/octave-mcxlab-0.9.5-1.fc30.src.rpm

Description: 

Monte Carlo eXtreme OpenCL (MCX-CL) is a fast photon transport simulation 
software for 3D heterogeneous turbid media, accelerated by GPUs.
MCXLAB-CL is the native MEX version of MCX-CL for Matlab and GNU Octave. 
It contains the entire MCX-CL code into a MEX function which can be called 
directly inside Matlab or Octave. The input and output files in MCX are 
replaced by convenient in-memory struct variables in MCXLAB-CL, thus, 
making it much easier to use and interact. Matlab/Octave also provides 
convenient plotting and data analysis functions. With MCXLAB-CL, your 
analysis can be streamlined and speed-up without involving disk files.

Fedora Account System Username: fangq

Comment 1 Qianqian Fang 2019-10-04 16:24:37 UTC
rpmlint output for srpm:
------------------------------------------------------------------------------
fangq@localhost:~/rpmbuild/SRPMS$ rpmlint octave-mcxlab-0.9.5-1.fc30.src.rpm
octave-mcxlab.src: W: spelling-error %description -l en_US eXtreme -> extreme, extremes, extremer
octave-mcxlab.src: W: spelling-error %description -l en_US struct -> strict, strut, struck
octave-mcxlab.src: W: spelling-error %description -l en_US Matlab -> Mat lab, Mat-lab, Mazatlan
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
------------------------------------------------------------------------------


rpmlint output for rpm:
------------------------------------------------------------------------------
fangq@localhost:~/rpmbuild/RPMS/noarch$ rpmlint octave-mcxlab-0.9.5-1.fc30.noarch.rpm
octave-mcxlab.noarch: W: spelling-error %description -l en_US eXtreme -> extreme, extremes, extremer
octave-mcxlab.noarch: W: spelling-error %description -l en_US struct -> strict, strut, struck
octave-mcxlab.noarch: W: spelling-error %description -l en_US Matlab -> Mat lab, Mat-lab, Mazatlan
octave-mcxlab.noarch: E: arch-independent-package-contains-binary-or-object /usr/share/octave/packages/mcxlab-0.9.5/mcxcl.mex
octave-mcxlab.noarch: W: dangerous-command-in-%preun mv
1 packages and 0 specfiles checked; 1 errors, 4 warnings.
------------------------------------------------------------------------------

Comment 2 Qianqian Fang 2019-10-04 16:25:34 UTC
similar to #1758626, this is also configured as a semi-noarch package.

Comment 3 Qianqian Fang 2019-10-04 16:36:43 UTC
Created attachment 1622600 [details]
screenshot of the installed software - everything works

screenshot of the installed software - everything works as expected

Comment 4 Robert-André Mauchin 🐧 2019-10-06 20:27:04 UTC
BuildArch:      noarch
ExclusiveArch:  %{ix86} x86_64

 That doesn't really makes sense, either this is a noarch package, or it is arched, can't have both. Remove 

BuildArch:      noarch

and:

%global _binaries_in_noarch_packages_terminate_build   0
%global debug_package %{nil}

 - Build errors:


Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.N9Jv8u
+ umask 022
+ cd /builddir/build/BUILD
+ cd mcxcl-0.9.5
+ cd src
++ octave-config -p OCTLIBDIR
+ make oct LIBOPENCLDIR=/usr/lib64/octave/5.1.0
which: no xxd in (/usr/lib64/ccache:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/sbin)
Please first install 'xxd' utility. For Ubuntu/Debian, use 'sudo apt-get install vim-common'; for Windows, please select xxd in cygwin64 installer.
make: *** [Makefile:134: xxd] Error 1

BR vim-common


++ octave-config -p OCTLIBDIR
+ make oct LIBOPENCLDIR=/usr/lib64/octave/5.1.0
xxd -i mcx_core.cl | sed 's/\([0-9a-f]\)$/\0, 0x00/' > mcx_core.clh
g++  -g -pedantic -Wall -O3 -DMCX_EMBED_CL -DMCX_OPENCL -DUSE_OS_TIMER  -Wno-variadic-macros -fPIC -DMCX_CONTAINER -c -fPIC -DMCX_CONTAINER -o mcx_host.o  mcx_host.cpp
make: g++: Command not found

BR gcc-c++

CXXFLAGS=' ' LFLAGS='-g -L/usr/lib64/octave/5.1.0 -lOpenCL ' LDFLAGS='' mkoctfile mcx_host.o mcx_utils.o tictoc.o mcxcl.o mcx_shapes.o cjson/cJSON.o --mex mcxlabcl.cpp  -o ../mcxlabcl/mcxcl.mex
/bin/sh: mkoctfile: command not found

BR octave-devel


 - Not useful: cp LICENSE.txt COPYING

 - Seems to build on any arches: https://koji.fedoraproject.org/koji/taskinfo?taskID=38099766

So:

ExclusiveArch:  %{ix86} x86_64

is not needed either.



Package Review
==============

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


Issues:
=======
- 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.
  Note: License file COPYING is not marked as %license
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/LicensingGuidelines/#_license_text
- Large documentation must go in a -doc subpackage. Large could be size
  (~1MB) or number of files.
  Note: Documentation size is 1116160 bytes in 29 files.
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/#_documentation


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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[!]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[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: "Unknown or generated", "GPL (v3 or later)", "Public domain",
     "Expat License". 109 files have unknown license. Detailed output of
     licensecheck in /home/bob/packaging/review/octave-mcxlab/review-
     octave-mcxlab/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[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.
[-]: Package contains desktop file if it is a GUI application.
[-]: 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.
[x]: Package contains systemd file(s) if in need.
[!]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[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]: Package requires other packages for directories it uses.
[x]: Package does not own files or directories owned by other packages.
[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]: 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 does not use a name that already exists.
[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

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

Generic:
[x]: Uses parallel make %{?_smp_mflags} macro.
[-]: 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).
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[-]: 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.
[-]: %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]: Fully versioned dependency in subpackages if applicable.
[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 1607680 bytes in /usr/share
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[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: octave-mcxlab-0.9.5-1.fc32.x86_64.rpm
          octave-mcxlab-debuginfo-0.9.5-1.fc32.x86_64.rpm
          octave-mcxlab-debugsource-0.9.5-1.fc32.x86_64.rpm
          octave-mcxlab-0.9.5-1.fc32.src.rpm
octave-mcxlab.x86_64: W: spelling-error %description -l en_US eXtreme -> extreme, extremes, extremer
octave-mcxlab.x86_64: W: spelling-error %description -l en_US struct -> strict, strut, struck
octave-mcxlab.x86_64: W: spelling-error %description -l en_US Matlab -> Mat lab, Mat-lab, Mazatlan
octave-mcxlab.x86_64: E: arch-dependent-file-in-usr-share /usr/share/octave/packages/mcxlab-0.9.5/mcxcl.mex
octave-mcxlab.x86_64: W: dangerous-command-in-%preun mv
octave-mcxlab.src: W: spelling-error %description -l en_US eXtreme -> extreme, extremes, extremer
octave-mcxlab.src: W: spelling-error %description -l en_US struct -> strict, strut, struck
octave-mcxlab.src: W: spelling-error %description -l en_US Matlab -> Mat lab, Mat-lab, Mazatlan
4 packages and 0 specfiles checked; 1 errors, 7 warnings.

Comment 5 Qianqian Fang 2019-10-07 04:36:45 UTC
I made the suggested changes,

https://github.com/fangq/fedorapkg/commit/2850a787f4e99d9a14290c9e5107da11befc6799

but similar to octave-iso2mesh (#1758626, comment 7), I am not able to build the package due to the same error. But from your koji log, you clearly did not have such issue. I am wondering if I miss something in my build environment. any suggestion?

Comment 6 Ankur Sinha (FranciscoD) 2019-10-07 09:25:27 UTC
(In reply to Qianqian Fang from comment #5)
> I made the suggested changes,
> 
> https://github.com/fangq/fedorapkg/commit/
> 2850a787f4e99d9a14290c9e5107da11befc6799
> 
> but similar to octave-iso2mesh (#1758626, comment 7), I am not able to build
> the package due to the same error. But from your koji log, you clearly did
> not have such issue. I am wondering if I miss something in my build
> environment. any suggestion?

Are you building on F30 for F30? If yes, could you please please build for rawhide and see how that goes?

New packages first get added to rawhide (currently F32) and then make it to stable releases if that's doable---since rawhide leads stable releases when it comes to newer software versions. Octave in rawhide is now 5.x while F30 is still 4.4---that could be one of the reasons things seem to work on rawhide but not F30.

Comment 7 Robert-André Mauchin 🐧 2019-10-07 14:57:30 UTC
Probably related to bug https://bugzilla.redhat.com/show_bug.cgi?id=1733898 and https://src.fedoraproject.org/rpms/octave/c/c32f37ff6c776af9486f4f128b2a87e1055e420a?branch=master

It should work on F30 by redefining octave_tar_suffix after octave_pkg_build:

%octave_pkg_build
%global octave_tar_suffix any-none

Comment 8 Qianqian Fang 2019-10-07 15:52:17 UTC
@eclipseo, the bug you found was exactly the one that caused the earlier problem, and I confirm the patch works. 

I also added a condition to wrap around this for portability (I intend to backport this as far as I can)

https://github.com/fangq/fedorapkg/commit/e3c77efe03655793e6b263b220caf5296454b919

when I built it on my fc30 vm, it failed with a complain that COPYING was not found. So, I brought the cp LICENSE.txt COPYING line back. I can make it conditional if you prefer.


Also, one of the warning messages says "large doc folder". the current installed doc folder is about 1.1MB, with 80% of those are demo script and sample data. do you think creating a -doc package is necessary in this case?



@both of you:

since my last package built for fedora was ~10 years ago (using Bohi), my memory was quite vague now when testing a new package. I was mostly following this link for new package 

https://fedoraproject.org/wiki/New_package_process_for_existing_contributors

and had the impression that fedpkg can only be called if someone approve this package first. I tried fedpkg on the fc30 with request-repo and this ticket number, but it says it was not approved.

other than installing a new vm, are there instructions for testing this package on rawhide?

Comment 9 Robert-André Mauchin 🐧 2019-10-07 16:53:48 UTC
Seems ok to me. Package approved.

Comment 10 Gwyn Ciesla 2019-10-07 18:47:42 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/octave-mcxlab

Comment 11 Qianqian Fang 2019-10-07 19:39:56 UTC
fedpkg build gives me an error

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

error: Bad source: /builddir/build/SOURCES/mcxcl-0.9.5.tar.gz: No such file or directory


I checked the URL and was able to download the tarball either in a browser or using wget. I also tried to provide the sha512sum in the originally empty sources file, but it failed even earlier.

any suggestions?

Comment 12 Qianqian Fang 2019-10-07 20:13:49 UTC
never mind, figured it out. the initial fedpkg import was not successful because I did not run kinit first. So, the source file was not uploaded. After running kinit and re-importing the srpm, it builds without any problem. 

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

Robert-André and Ankur, thank you both for the prompt reply and help! closing the ticket now.

Comment 13 Fedora Update System 2019-10-07 20:51:19 UTC
FEDORA-2019-0d03b32c8d has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-0d03b32c8d

Comment 14 Fedora Update System 2019-10-07 20:55:36 UTC
FEDORA-2019-c809ee9857 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-c809ee9857

Comment 15 Fedora Update System 2019-10-07 20:56:21 UTC
FEDORA-2019-3e7b73a7fe has been submitted as an update to Fedora 31. https://bodhi.fedoraproject.org/updates/FEDORA-2019-3e7b73a7fe

Comment 16 Ankur Sinha (FranciscoD) 2019-10-07 23:07:18 UTC
That's great---thanks very much for working on these.

Can you please give the neuro-sig commit access to the package (in https://src.fedoraproject.org/rpms/octave-mcxlab, go to "settings" -> "add group" -> neuro-sig -> commit access). That way, everyone can help maintain these, and they will show up in our list of tools here also: https://src.fedoraproject.org/group/neuro-sig

When it reaches stable in a week's time, we must remember to add it to the documentation. Would this page be the right one? https://docs.fedoraproject.org/en-US/neurofedora/imaging-tools/

Comment 17 Qianqian Fang 2019-10-07 23:29:22 UTC
@FranciscoD: done.

since I have backported the package to f29-f31, I am wondering if someone need to approve the update in order for the package to appear in the update repo? for some reason, I remember this happened automatically in the past.

for the documentation, happy to update, do I have access?

Comment 18 Ankur Sinha (FranciscoD) 2019-10-07 23:36:42 UTC
(In reply to Qianqian Fang from comment #17)
> @FranciscoD: done.
> 
> since I have backported the package to f29-f31, I am wondering if someone
> need to approve the update in order for the package to appear in the update
> repo? for some reason, I remember this happened automatically in the past.

If it does not receive negative karma, it will be pushed to stable in 7 days on its own. Bodhi implements the policy detailed here:
https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/#stable-releases


> for the documentation, happy to update, do I have access?

Yes---everyone in the neuro-sig pagure group has commit access to the docs sources, and everyone can open pull requests. It's an asciidoc + antora based system now:
https://docs.fedoraproject.org/en-US/fedora-docs/contributing/

The sources for our docs are here: https://pagure.io/neuro-sig/documentation
If you're unsure about anything, you could open a PR and someone will review and merge it.

Thanks again,
Ankur

Comment 19 Fedora Update System 2019-10-08 20:22:43 UTC
octave-mcxlab-0.9.5-1.fc29 has been pushed to the Fedora 29 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-2019-0d03b32c8d

Comment 20 Fedora Update System 2019-10-08 20:32:53 UTC
octave-mcxlab-0.9.5-1.fc30 has been pushed to the Fedora 30 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-2019-c809ee9857

Comment 21 Fedora Update System 2019-10-08 21:42:24 UTC
octave-mcxlab-0.9.5-1.fc31 has been pushed to the Fedora 31 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-2019-3e7b73a7fe

Comment 22 Fedora Update System 2019-10-15 22:39:37 UTC
octave-mcxlab-0.9.5-1.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.

Comment 23 Fedora Update System 2019-10-15 23:13:23 UTC
octave-mcxlab-0.9.5-1.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.

Comment 24 Fedora Update System 2019-10-26 17:23:20 UTC
octave-mcxlab-0.9.5-1.fc31 has been pushed to the Fedora 31 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.