Bug 1760617
| Summary: | Review Request: mmc - A GPU mesh-based Monte Carlo photon simulator | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Qianqian Fang <fangqq> | ||||
| Component: | Package Review | Assignee: | Ankur Sinha (FranciscoD) <sanjay.ankur> | ||||
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
| Severity: | medium | Docs Contact: | |||||
| Priority: | medium | ||||||
| Version: | rawhide | CC: | neuro-sig, orion, package-review, sanjay.ankur, zebob.m | ||||
| Target Milestone: | --- | Flags: | sanjay.ankur:
fedora-review+
|
||||
| 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: | 2020-03-03 21:41:48 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: | |||||||
| Bug Depends On: | |||||||
| Bug Blocks: | 1276941 | ||||||
| Attachments: |
|
||||||
|
Description
Qianqian Fang
2019-10-10 23:27:55 UTC
rpmlint output for the srpm package
----------------------------------------------------------------------------
fangq@localhost SRPMS]$ rpmlint octave-mmclab-1.7.9-1.fc30.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
----------------------------------------------------------------------------
rpmlint output for the 4 output rpm packages (octave-mmclab, mmclab-demos, mmc, mmc-demos)
----------------------------------------------------------------------------
[fangq@localhost RPMS]$ find . -name "*mmc*.rpm" -print -exec rpmlint '{}' \;
----------------------------------------------------------------------------
./noarch/mmc-demos-1.7.9-1.fc30.noarch.rpm
mmc-demos.noarch: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases
mmc-demos.noarch: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/colin27/prop_brain.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/dcs/dcs.inp
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g1_Db.m
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g1_Db_fms.m
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g2_Db.m
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g2_Db_fms.m
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/dcs/prop_dcs.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/meshtest/prop_mesh0.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/meshtest/prop_mesh1.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/meshtest/prop_mesh2.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/onecube/prop_onecube.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/reftest/prop_onecube.dat
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/regression/exitangle/phantom.m
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/replay/prop_replaytest.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/replaywide/prop_replaywide.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/rngtest/Makefile
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/README.txt
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/createmesh.m
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/prop_sfdi.dat
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/sfdi.inp
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sharing/README.txt
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sharing/prop_sharing.dat
mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sharing/sharing.inp
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/validation/prop_cube.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/validation/prop_cube2.dat
mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/widedet/prop_widedet.dat
1 packages and 0 specfiles checked; 0 errors, 28 warnings.
----------------------------------------------------------------------------
./noarch/mmclab-demos-1.7.9-1.fc30.noarch.rpm
----------------------------------------------------------------------------
mmclab-demos.noarch: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases
mmclab-demos.noarch: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases
mmclab-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmclab-demos/example/demo_sfdi_2layer.m
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
----------------------------------------------------------------------------
./x86_64/octave-mmclab-1.7.9-1.fc30.x86_64.rpm
octave-mmclab.x86_64: W: dangerous-command-in-%preun mv
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
----------------------------------------------------------------------------
./x86_64/mmc-1.7.9-1.fc30.x86_64.rpm
mmc.x86_64: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases
mmc.x86_64: W: no-manual-page-for-binary mmc
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
----------------------------------------------------------------------------
In this package, I also moved the .mex file to /usr/libexec (if this is the right place), and create a link in the toolbox folder. I will do the same for octave-zmat/octave-mcxlab. Can you please provide the "raw" link to the spec file please? The fedora-review tool cannot work with the link in the current form since it returns HTML, not the raw spec file. (In reply to Qianqian Fang from comment #2) > In this package, I also moved the .mex file to /usr/libexec (if this is the > right place), and create a link in the toolbox folder. I will do the same > for octave-zmat/octave-mcxlab. I don't think mex files need to be moved. You simply put these files (along with other arch specific files) into %{octpkglibdir} which expands to %{_libdir}/octave/packages/.. %{octpkglibdir} should be known to octave too, so nothing else should be needed here. libexec was a special case because the package bundled binaries. PS: please block the "fedora-neuro" tracker bug for NeuroFedora related packages. That'll ensure that the whole team gets notifications automatically. Spec URL: https://raw.githubusercontent.com/fangq/fedorapkg/mmclab/octave-mmclab.spec SRPM URL: https://kwafoo.coe.neu.edu/~fangq/share/temp/octave-mmclab-1.7.9-1.fc30.src.rpm Description: MMCLAB is the native MEX version of MMC - Mesh-based Monte Carlo - for MATLAB and GNU Octave. By converting the input and output files into convenient in-memory variables, MMCLAB is very intuitive to use and straightforward to be integrated with mesh generation and post-simulation analyses. Because MMCLAB contains the same computational codes for OpenCL-based photon simulation as in a MMC binary, running MMCLAB inside MATLAB is expected to give similar speed as running a standalone MMC binary using either a CPU or a GPU. Fedora Account System Username: fangq done. the raw link is posted, and moved .mex back with the .m files. @Ankur, happy to set the block flag, but I can't find the field, can you let me know which item to change? also, let me know if the updated spec file is acceptable. thanks The new bugzilla hides the blocks/depends field under "Advanced fields". If you click the button, it should show the fields. Here is the review, but I'll have to do it again once you regenerate the SRPM from the new spec and upload that: Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - Should the main spec/package be simply called mmc and then octave-.. as sub-packages instead of the other way round? - Why does the build require vim-common? - 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 COPYING is required for the octave package, so this looks OK. - Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 5703680 bytes in 190 files. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_documentation Please move the docs to the a sub-package. - Lots of bundling here too. Is this required? $ licensecheck -r . | sed '/UNKNOWN/ d' ./LICENSE.txt: GPL (v3 or later) ./mmclab/LICENSE.txt: GPL (v3 or later) ./src/drand48_r_libgw32c.c: GNU Lesser General Public License (v2.1 or later) ./src/mmcdoxy.cfg: *No copyright* GENERATED FILE ./src/nifti1.h: Public domain ./examples/onecube/README.txt: *No copyright* GENERATED FILE ./examples/reftest/README.txt: *No copyright* GENERATED FILE ./mmclab/example/demo_head_atlas.m: *No copyright* GENERATED FILE ./src/SFMT/LICENSE.txt: BSD 3-clause "New" or "Revised" License ./src/cjson/LICENSE: Expat License ./src/cjson/README: Expat License ./src/cjson/cJSON.c: Expat License ./src/cjson/cJSON.h: Expat License ./src/cjson/test.c: Expat License ./src/sse_math/sse_math.h: zlib/libpng license ./src/waitmex/license.txt: BSD 3-clause "New" or "Revised" License - Build flags aren't used. Example from the build.log file: Building built/mmc.o cc -c -Wall -g -DMCX_EMBED_CL -fno-strict-aliasing -m64 -DMMC_USE_SSE -DHAVE_SSE2 -msse -msse2 -msse3 -mssse3 -msse4.1 -O3 -fopenmp -fPIC -DMCX_CONTAINER -DUSE_OS_TIMER -DUSE_OPENCL -I../src -o built/mmc.o mmc.c Please use %{set_opt_flags} before you run %make_build - You do not need to use pushd, you can use %make_build -C <directory name> - There's still the ln -s mex etc. Is this the latest srpm? (Each time you update the spec file, you need to regenerate the srpm and upload the latest versions of both.) - Quite a few rpmlint issues. Please correct them. Some hints here: https://fedoraproject.org/wiki/Common_Rpmlint_issues ===== MUST items ===== C/C++: [-]: Provides: bundled(gnulib) in place as required. Note: Sources not installed [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [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. [!]: License field in the package spec file matches the actual license. Note: There is no build directory. Running licensecheck on vanilla upstream sources. No licenses found. Please check the source files for licenses manually. ^ Is this also bundling lots of things? Is this required? $ licensecheck -r . | sed '/UNKNOWN/ d' ./LICENSE.txt: GPL (v3 or later) ./mmclab/LICENSE.txt: GPL (v3 or later) ./src/drand48_r_libgw32c.c: GNU Lesser General Public License (v2.1 or later) ./src/mmcdoxy.cfg: *No copyright* GENERATED FILE ./src/nifti1.h: Public domain ./examples/onecube/README.txt: *No copyright* GENERATED FILE ./examples/reftest/README.txt: *No copyright* GENERATED FILE ./mmclab/example/demo_head_atlas.m: *No copyright* GENERATED FILE ./src/SFMT/LICENSE.txt: BSD 3-clause "New" or "Revised" License ./src/cjson/LICENSE: Expat License ./src/cjson/README: Expat License ./src/cjson/cJSON.c: Expat License ./src/cjson/cJSON.h: Expat License ./src/cjson/test.c: Expat License ./src/sse_math/sse_math.h: zlib/libpng license ./src/waitmex/license.txt: BSD 3-clause "New" or "Revised" License [x]: License file installed when any subpackage combination is installed. [x]: Package does not own files or directories owned by other packages. [!]: %build honors applicable compiler flags or justifies otherwise. They do not. Example from the build.log file: Building built/mmc.o cc -c -Wall -g -DMCX_EMBED_CL -fno-strict-aliasing -m64 -DMMC_USE_SSE -DHAVE_SSE2 -msse -msse2 -msse3 -mssse3 -msse4.1 -O3 -fopenmp -fPIC -DMCX_CONTAINER -DUSE_OS_TIMER -DUSE_OPENCL -I../src -o built/mmc.o mmc.c Please use %{set_opt_flags} before you run %make_build [!]: Package contains no bundled libraries without FPC exception. cjson, SFMT, sse_math, waitmex and others are bundled. Either they need to be unbundled, or specified, and license updated accordingly. [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. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [!]: Package complies to the Packaging Guidelines ^ Issues noted. [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 must own all directories that it creates. [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 must not depend on deprecated() packages. [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]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [ ]: Final provides and requires are sane (see attachments). [!]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in mmclab- demos , mmc , mmc-demos Do the Requires be versioned? https://docs.fedoraproject.org/en-US/packaging-guidelines/#_explicit_requires [?]: Package functions as described. ^ Not yet tested [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: 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]: 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: [!]: Spec file according to URL is the same as in SRPM. Note: Spec file as given by url is not the same as in SRPM (see attached diff). See: (this test has no URL) ^ This confirms that the generated srpm does not match the latest spec. Please post the newest ones. [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]: Large data in /usr/share should live in a noarch subpackage if package is arched. Rpmlint ------- Checking: octave-mmclab-1.7.9-1.fc32.x86_64.rpm mmclab-demos-1.7.9-1.fc32.noarch.rpm mmc-1.7.9-1.fc32.x86_64.rpm mmc-demos-1.7.9-1.fc32.noarch.rpm octave-mmclab-debuginfo-1.7.9-1.fc32.x86_64.rpm octave-mmclab-debugsource-1.7.9-1.fc32.x86_64.rpm octave-mmclab-1.7.9-1.fc32.src.rpm octave-mmclab.x86_64: W: dangerous-command-in-%preun mv mmclab-demos.noarch: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases mmclab-demos.noarch: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases mmclab-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmclab-demos/example/demo_sfdi_2layer.m mmc.x86_64: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases mmc.x86_64: W: no-manual-page-for-binary mmc mmc-demos.noarch: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases mmc-demos.noarch: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/colin27/prop_brain.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/dcs/dcs.inp ^ Please remove the executable perm mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g1_Db.m mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g1_Db_fms.m mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g2_Db.m mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g2_Db_fms.m ^ Please correct this mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/dcs/prop_dcs.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/meshtest/prop_mesh0.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/meshtest/prop_mesh1.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/meshtest/prop_mesh2.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/onecube/prop_onecube.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/reftest/prop_onecube.dat ^ Please correct this mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/regression/exitangle/phantom.m ^ Please correct this mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/replay/prop_replaytest.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/replaywide/prop_replaywide.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/rngtest/Makefile ^ Please correct this mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/README.txt mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/createmesh.m mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/prop_sfdi.dat mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/sfdi.inp mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sharing/README.txt mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sharing/prop_sharing.dat mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sharing/sharing.inp ^ Please correct this mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/validation/prop_cube.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/validation/prop_cube2.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/widedet/prop_widedet.dat ^ Please correct this 7 packages and 0 specfiles checked; 0 errors, 34 warnings. Rpmlint (debuginfo) ------------------- Checking: octave-mmclab-debuginfo-1.7.9-1.fc32.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- perl: warning: Setting locale failed. perl: warning: Please check that your locale settings: LANGUAGE = (unset), LC_ALL = (unset), LC_CTYPE = "C.UTF-8", LANG = "en_GB.UTF-8" are supported and installed on your system. perl: warning: Falling back to the standard locale ("C"). perl: warning: Setting locale failed. perl: warning: Please check that your locale settings: LANGUAGE = (unset), LC_ALL = (unset), LC_CTYPE = "C.UTF-8", LANG = "en_GB.UTF-8" are supported and installed on your system. perl: warning: Falling back to the standard locale ("C"). mmc-demos.noarch: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases mmc-demos.noarch: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases mmc-demos.noarch: W: invalid-url URL: http://mcx.space/#mmc <urlopen error [Errno -2] Name or service not known> mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/colin27/prop_brain.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/dcs/dcs.inp mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g1_Db.m mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g1_Db_fms.m mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g2_Db.m mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/dcs/dcs_g2_Db_fms.m mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/dcs/prop_dcs.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/meshtest/prop_mesh0.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/meshtest/prop_mesh1.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/meshtest/prop_mesh2.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/onecube/prop_onecube.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/reftest/prop_onecube.dat mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/regression/exitangle/phantom.m mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/replay/prop_replaytest.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/replaywide/prop_replaywide.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/rngtest/Makefile mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/README.txt mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/createmesh.m mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/prop_sfdi.dat mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sfdi2layer/sfdi.inp mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sharing/README.txt mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sharing/prop_sharing.dat mmc-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmc-demos/examples/sharing/sharing.inp mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/validation/prop_cube.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/validation/prop_cube2.dat mmc-demos.noarch: W: spurious-executable-perm /usr/share/doc/mmc-demos/examples/widedet/prop_widedet.dat octave-mmclab-debugsource.x86_64: W: invalid-url URL: http://mcx.space/#mmc <urlopen error [Errno -2] Name or service not known> octave-mmclab.x86_64: W: invalid-url URL: http://mcx.space/#mmc <urlopen error [Errno -2] Name or service not known> octave-mmclab.x86_64: W: dangerous-command-in-%preun mv mmc.x86_64: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases mmc.x86_64: W: invalid-url URL: http://mcx.space/#mmc <urlopen error [Errno -2] Name or service not known> mmc.x86_64: W: no-manual-page-for-binary mmc octave-mmclab-debuginfo.x86_64: W: invalid-url URL: http://mcx.space/#mmc <urlopen error [Errno -2] Name or service not known> mmclab-demos.noarch: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases mmclab-demos.noarch: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases mmclab-demos.noarch: W: invalid-url URL: http://mcx.space/#mmc <urlopen error [Errno -2] Name or service not known> mmclab-demos.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/mmclab-demos/example/demo_sfdi_2layer.m 6 packages and 0 specfiles checked; 0 errors, 40 warnings. Source checksums ---------------- https://github.com/fangq/mmc/archive/v1.7.9/mmc-1.7.9.tar.gz : CHECKSUM(SHA256) this package : b5533a639a0efe7de2785358c6b2ef1ba2228f00fffd35faf9fd0b42f8040e64 CHECKSUM(SHA256) upstream package : b5533a639a0efe7de2785358c6b2ef1ba2228f00fffd35faf9fd0b42f8040e64 Requires -------- octave-mmclab (rpmlib, GLIBC filtered): /bin/sh libOpenCL.so.1()(64bit) libOpenCL.so.1(OPENCL_1.0)(64bit) libOpenCL.so.1(OPENCL_1.2)(64bit) libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgomp.so.1()(64bit) libgomp.so.1(GOMP_1.0)(64bit) libgomp.so.1(GOMP_4.0)(64bit) libgomp.so.1(OMP_1.0)(64bit) libm.so.6()(64bit) libpthread.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) octave octave-iso2mesh opencl-filesystem rtld(GNU_HASH) mmclab-demos (rpmlib, GLIBC filtered): octave octave-iso2mesh octave-mmclab mmc (rpmlib, GLIBC filtered): libOpenCL.so.1()(64bit) libOpenCL.so.1(OPENCL_1.0)(64bit) libOpenCL.so.1(OPENCL_1.2)(64bit) libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgomp.so.1()(64bit) libgomp.so.1(GOMP_1.0)(64bit) libgomp.so.1(GOMP_4.0)(64bit) libgomp.so.1(OMP_1.0)(64bit) libm.so.6()(64bit) libpthread.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) octave octave-iso2mesh opencl-filesystem rtld(GNU_HASH) mmc-demos (rpmlib, GLIBC filtered): octave octave-iso2mesh octave-mmclab-debuginfo (rpmlib, GLIBC filtered): octave-mmclab-debugsource (rpmlib, GLIBC filtered): Provides -------- octave-mmclab: octave-mmclab octave-mmclab(x86-64) mmclab-demos: mmclab-demos mmc: mmc mmc(x86-64) mmc-demos: mmc-demos octave-mmclab-debuginfo: debuginfo(build-id) octave-mmclab-debuginfo octave-mmclab-debuginfo(x86-64) octave-mmclab-debugsource: octave-mmclab-debugsource octave-mmclab-debugsource(x86-64) Diff spec file in url and in SRPM --------------------------------- --- /home/asinha/dump/fedora-review/1760617-octave-mmclab/srpm/octave-mmclab.spec 2019-10-13 18:11:35.017723858 +0100 +++ /home/asinha/dump/fedora-review/1760617-octave-mmclab/srpm-unpacked/octave-mmclab.spec 2019-10-11 00:04:24.000000000 +0100 @@ -6,9 +6,9 @@ Version: 1.7.9 Release: 1%{?dist} -Summary: A GPU mesh-based Monte Carlo photon simulator for MATLAB/Octave +Summary: A GPU Mesh-based Monte Carlo photon simulator for MATLAB/Octave License: GPLv3+ URL: http://mcx.space/#mmc Source0: https://github.com/fangq/%{project}/archive/v%{version}/%{project}-%{version}.tar.gz -BuildRequires: octave-devel gcc-c++ vim-common opencl-headers ocl-icd-devel +BuildRequires: octave-devel gcc-c++ vim-common opencl-headers ocl-icd-devel Requires: octave opencl-filesystem octave-iso2mesh @@ -28,5 +28,5 @@ %package -n %{octpkg}-demos -Summary: Example datasets and scripts for MMCLAB toolbox +Summary: Example datasets and scripts for the MMCLAB toolbox BuildArch: noarch Requires: octave octave-%{octpkg} octave-iso2mesh @@ -36,5 +36,5 @@ %package -n %{project} -Summary: Example datasets and scripts for mesh-based Monte Carlo (MMC) +Summary: Example datasets and scripts for the MMCLAB toolbox BuildRequires: octave-devel gcc-c++ vim-common opencl-headers ocl-icd-devel Requires: octave opencl-filesystem octave-iso2mesh @@ -146,6 +146,8 @@ rm %{octpkg}/*.txt mv %{octpkg}/example . +mv %{octpkg}/*.mex . mv %{octpkg} inst mv src/Makefile . +ln -s %{_libexecdir}/%{octpkg}/%{branch}.mex inst/%{branch}.mex %octave_pkg_build @@ -165,6 +167,9 @@ %octave_pkg_install +install -m 0755 -vd %{buildroot}%{_libexecdir}/%{octpkg} +install -m 0755 -vp %{branch}.mex %{buildroot}%{_libexecdir}/%{octpkg}/ + install -m 0755 -vd %{buildroot}%{_bindir} -install -m 0755 -vt %{buildroot}%{_bindir} bin/%{project} +install -m 0755 -t %{buildroot}%{_bindir} bin/%{project} %post @@ -180,4 +185,6 @@ %license LICENSE.txt %doc README.txt AUTHORS.txt +%dir %{_libexecdir}/%{octpkg} +%{_libexecdir}/%{octpkg}/*.mex %dir %{octpkgdir} %{octpkgdir}/*.m Generated by fedora-review 0.7.3 (44b83c7) last change: 2019-09-18 Command line :/usr/bin/fedora-review -b 1760617 Buildroot used: fedora-rawhide-{{ target_arch }} Active plugins: C/C++, Shell-api, Generic Disabled plugins: Java, Python, Perl, fonts, PHP, R, SugarActivity, Ocaml, Haskell Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH Spec URL: https://raw.githubusercontent.com/fangq/fedorapkg/mmclab/mmc.spec SRPM URL: https://kwafoo.coe.neu.edu/~fangq/share/temp/mmc-1.7.9-1.fc30.src.rpm Description: MMCLAB is the native MEX version of MMC - Mesh-based Monte Carlo - for MATLAB and GNU Octave. By converting the input and output files into convenient in-memory variables, MMCLAB is very intuitive to use and straightforward to be integrated with mesh generation and post-simulation analyses. Because MMCLAB contains the same computational codes for OpenCL-based photon simulation as in a MMC binary, running MMCLAB inside MATLAB is expected to give similar speed as running a standalone MMC binary using either a CPU or a GPU. Fedora Account System Username: fangq @Ankur thanks for the feedback, my updated spec file and srpm file are updated in the above post here are my replies to some of the top issues you spotted, let me know what you think. ------------------------------------------------------------------------- > Should the main spec/package be simply called mmc and then octave-.. as > sub-packages instead of the other way round? done, the main package is now mmc, and octave-mmclab etc are now subpkgs ------------------------------------------------------------------------- > This confirms that the generated srpm does not match the latest spec. Please > post the newest ones. sorry, I must have forgotten to update the srpm previously. Now, the new srpm can be found int he above post. ------------------------------------------------------------------------- > Please remove the executable perm all the permission and end-of-line issues are now corrected in the upstream, and I made a new upstream package to create the rpms https://github.com/fangq/mmc/commit/2139721339907362f269562540b2d63ef88f0561 I confirm that these issues are fixed in the new srpm ------------------------------------------------------------------------- > Lots of bundling here too. Is this required? this case is not bundling - the licenses found by the licensecheck are in the source-code files. They are all compatible with the final binary's license (GPLv3+) according to https://www.gnu.org/licenses/license-list.en.html#GPLCompatibleLicenses as required by GPL license, once all these source units are compiled and linked into a binary, the final generated software shall be licensed under GPL https://www.gnu.org/licenses/gpl-faq.en.html#MereAggregation so, as long as the licenses of the source units are compatible with GPL, we should label the final executable/package as GPL. So I believe what I did here is correct. ------------------------------------------------------------------------- here are some of the minor comments: > Why does the build require vim-common? I use the xxd utility from vim-common to convert the OpenCL kernel file *.cl to .h file in order to compile it in the binary; this is similar to the case in mcxlab (Bug 1758622). ------------------------------------------------------------------------- > Please move the docs to the a sub-package. I've already put all mmc/examples into mmc-demos and mmc/mmclab/example to mmclab-demos. ------------------------------------------------------------------------- > - Build flags aren't used. Example from the build.log file: > Please use %{set_opt_flags} before you run %make_build done ------------------------------------------------------------------------- > You do not need to use pushd, you can use %make_build -C <directory name> done ------------------------------------------------------------------------- > There's still the ln -s mex etc. Is this the latest srpm? (Each time you > update the spec file, you need to regenerate the srpm and upload the latest > versions of both.) I believe this was a result of obsolete srpm files. please run it again with the new srpm. ------------------------------------------------------------------------- > [!]: Fully versioned dependency in subpackages if applicable. > Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in mmclab- > demos , mmc , mmc-demos > Do the Requires be versioned? I think it is fine. this is the initial package, all the features in the demos are supported by this version. In the future, if some demos requires newer versions, I will add version in Requires. ------------------------------------------------------------------------- Please drop %{project}, just use %{name}.
@Orion - agree, the redundant macro is now removed https://github.com/fangq/fedorapkg/commit/873fba99c50592bd1da840bab3fb489ccc2fbd7b @Ankur - I also manually added the build flags, as my Makefile does not use the standard flags, see https://github.com/fangq/fedorapkg/commit/873fba99c50592bd1da840bab3fb489ccc2fbd7b#diff-22ce4fe606fb9f0c2db4bcb72337bc66R147 both the spec and srpm files are updated. let me know if this can be approved. thanks @Ankur, let me know if you have any further feedback regarding the spec file. thanks Running it through Fedora review again now. (In reply to Qianqian Fang from comment #10) > ------------------------------------------------------------------------- > > Lots of bundling here too. Is this required? > > > this case is not bundling - the licenses found by the licensecheck are in > the source-code files. They are all compatible with the final binary's > license (GPLv3+) according to > > https://www.gnu.org/licenses/license-list.en.html#GPLCompatibleLicenses > > as required by GPL license, once all these source units are compiled and > linked into a binary, the final generated software shall be licensed under > GPL > > https://www.gnu.org/licenses/gpl-faq.en.html#MereAggregation > > so, as long as the licenses of the source units are compatible with GPL, we > should label the final executable/package as GPL. So I believe what I did > here is correct. > No, you misunderstand. Bundling is not only about the compatibility of licenses these tools are provided under. That's a different question. The question w.r.t bundling here is: why are we carrying copies of these external libraries in the sources and building individual copies of them especially for this package? SFMT, for example is available here: http://www.math.sci.hiroshima-u.ac.jp/~m-mat/MT/SFMT/ So, what we should try to do is package SFMT separately, and use it as a BuildRequires in all packages that use it. This has also come up in other reviews. Bundling is not forbidden any more, but reasonable effort should be made to avoid it. We really do not want different packages each carrying their own copies of libraries---it greatly increases the maintenance burden, and of course, if all packages carry their own private copies of libraries, there is a high likelihood that they'll deviate from upstream's code (and the currently supported version). As package maintainers, we try to work with upstream (here, that is you :)) to help them use the latest versions of general libraries (which are generally available in Fedora). This applies to: SFMT, cjson, and waitmex. They appear to be general purpose libraries/tools. Is there some reason they cannot be packaged as separate packages and used as BuildRequires? The following documents are relevant: - https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling -> if at all possible, we use unbundled versions, otherwise we clearly indicate what versions are bundled - https://fedoraproject.org/wiki/Staying_close_to_upstream_projects Cheers, @Ankur I agree with you that it makes software more modular and scalable if all dependencies can be built as separate packages. However, I think the scenario here is slightly different. the 4 external components that mmc depends on, namely, sse_math, waitmex, cjson and SFMT, are extremely lightweight units, and are designed primarily for embedded use. For example - sse_math is a header-only single-unit header file - waitmex and cJSON are both single-unit (waitmex.c/.h, cJSON.c/.h) libraries - SFMT is also a single unit (SFMT.c with multiple header files) library if you search on github, the majority of the appearance of SFMT are for embedded use, similar to mmc, https://github.com/search?q=SFMT.h&type=Code same for cJSON https://github.com/search?q=cjson&type=Code in fact, the embedded use of these utilities are seen as an advantage by the authors of these tools, and they intentionally designed the unit as portable and lightweight as possible to enable such use, and also to ensure such by using a more permissive license (BSD, MIT, ZLIB). In a way, I am permitted, by the respective licenses, to convert each of the cited units into GPL and place a uniform license disclaimers in each of the units, so that it looks more like an integrated/single-licensed source-code tree, but I prefer not to do that because I have to make such edits again if I want to sync some of the units to new releases. I also thought it would give upstream authors more visibility by keeping their original disclaimers in the source codes; but I did not expect this visibility to turn into a restriction for me to distribute the final software in the license that I desire (which is known to be compliant with the upstream licenses). again, I am not against packaging these lightweight libraries separately (I am happy to help), but it is not as beneficial in the case of mmc. Because these libraries are generally not available even in open-source distributions, not mention about windows/mac where the majority of mmc users use, to make mmc dependent to such libraries makes the code hard to compile for a large portion of my existing users. I will also have to deal with future changes of the API interfaces and data structures that may happen independent to my codes. So, even we can create separate packages for these utilities, I still prefer my code to embed these lightweight units for portability - perhaps in the future if these libraries become widely available like libz, libblas or libOpenCL, then, I may consider switching to dynamic linking mode. let me know if this makes sense to you. to add another clarification - mmc uses these libraries internally for file IO of specific inputs and computations purposes, and has no public interface to call these libraries externally and allow to use them individually. You can treat them as private/internal functions (compiled in a single GPL-licensed binary), not a way to encapsulate/ship the libraries per se to allow reuse - from this perspective, I felt that calling it "bundling" is not exactly accurate. (In reply to Qianqian Fang from comment #16) > to add another clarification - mmc uses these libraries internally for file > IO of specific inputs and computations purposes, and has no public interface > to call these libraries externally and allow to use them individually. You > can treat them as private/internal functions (compiled in a single > GPL-licensed binary), not a way to encapsulate/ship the libraries per se to > allow reuse - from this perspective, I felt that calling it "bundling" is > not exactly accurate. Replying to this first. No, bundling simply refers to "not using system versions of libraries". How you use them, or whether or not they are made available by your package is not a factor here. The point is simply that you are using a private version of these libraries. The guidelines should clarify this: https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling Now on to the second part: (In reply to Qianqian Fang from comment #15) > @Ankur > > I agree with you that it makes software more modular and scalable if all > dependencies can be built as separate packages. However, I think the > scenario here is slightly different. No, again---this is not why bundling is avoided. Avoiding bundling simply results in software being more modular. The main issue we are trying to avoid is explained better with the help of an example, mmc here: - mmc is bundling SFMT - The current release of SFMT is 5.1 from 2017 Now, since mmc is bundling this SFMT version, as SFMT moves forward with bugfixes/optimisations/enhancements, mmc does not receive any of these. In fact, since we don't know the version of SFMT in use in mmc (I can't find any version information in the code somehow), we cannot say for sure if the SFMT developers are still supporting it. So, if a user finds a bug in the bundled version of SFMT, who then looks into it? If mmc modifies SFMT, then you have effectively forked an older version of SFMT and now responsible for maintaining it---separate from SFMT upstream. > > the 4 external components that mmc depends on, namely, sse_math, waitmex, > cjson and SFMT, are extremely lightweight units, and are designed primarily > for embedded use. For example > > - sse_math is a header-only single-unit header file > - waitmex and cJSON are both single-unit (waitmex.c/.h, cJSON.c/.h) libraries > - SFMT is also a single unit (SFMT.c with multiple header files) library > > if you search on github, the majority of the appearance of SFMT are for > embedded use, similar to mmc, > > https://github.com/search?q=SFMT.h&type=Code That does not make it the right practice from a software development point of view---and the point of the review process is to ensure that we follow software development best practices while adding software to Fedora. (A goal of the NeuroFedora group is also to try and push these software development standards upstream to academics who write software but may not necessarily be trained in software development.) > > same for cJSON > > https://github.com/search?q=cjson&type=Code > > in fact, the embedded use of these utilities are seen as an advantage by the > authors of these tools, and they intentionally designed the unit as portable > and lightweight as possible to enable such use, and also to ensure such by > using a more permissive license (BSD, MIT, ZLIB). If that's the case, then these developers are also not following suggested software development best practices. It is unfortunate. Embedding libraries has more disadvantages than advantages. > > In a way, I am permitted, by the respective licenses, to convert each of the > cited units into GPL and place a uniform license disclaimers in each of the > units, so that it looks more like an integrated/single-licensed source-code > tree, but I prefer not to do that because I have to make such edits again if > I want to sync some of the units to new releases. I also thought it would > give upstream authors more visibility by keeping their original disclaimers > in the source codes; but I did not expect this visibility to turn into a > restriction for me to distribute the final software in the license that I > desire (which is known to be compliant with the upstream licenses). The licensing is fine. > > > again, I am not against packaging these lightweight libraries separately (I > am happy to help), but it is not as beneficial in the case of mmc. Because > these libraries are generally not available even in open-source > distributions, not mention about windows/mac where the majority of mmc users > use, to make mmc dependent to such libraries makes the code hard to compile > for a large portion of my existing users. I will also have to deal with > future changes of the API interfaces and data structures that may happen > independent to my codes. But isn't this a good thing? As the developer, don't you want to use the latest versions of these libraries? What is the advantage of limiting your tool to older versions, (apart from it being convenient)? If the concern is that SFMT etc are not available on most installations, and bundling helps here, can we at least: - version the bundled libraries so we know what snapshot of these in time we're depending on, and - maybe try to always bundle the latest versions? > So, even we can create separate packages for these > utilities, I still prefer my code to embed these lightweight units for > portability This is fine if you want to distribute pre-built binaries directly from Github, but isn't quite relevant when it comes to packaging for distributions. The common way upstreams go about it is by providing an easy compilation option where one can choose between the bundled version of the library or a system version. > - perhaps in the future if these libraries become widely > available like libz, libblas or libOpenCL, then, I may consider switching to > dynamic linking mode. This is a catch-22. If everyone keeps bundling them, there will be no incentive to make them available in distributions XD > let me know if this makes sense to you. Sure, I understand where you are coming from. As I've said, bundling is not forbidden anymore---it used to be. It is up to you as developer + package maintainer to decide if bundling is the way to go. At the same time, though, it is our role as reviewers to have this discussion to see if bundling can be avoided. If we're going ahead with bundling, please follow the guidelines and include the required Provides: .. statements in the spec.: https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling @Ankur your points are taken - I have no objection to the current policy regarding bundling: if a system library already exists and it makes sense to dynamically link to it to avoid duplication, but embedding open-source codes/units in a larger project is also a very common practice and there are even dedicated tools to facilitate such use (such as git submodule, or header-only libraries). I understand your standing point of pushing reusable libraries in the context of building a healthy, modular and dynamic distribution/software environment, but as a software developer and distributor, I also have to pay extensive attention to 1) portability - i.e. to ensure smooth/easy software compilation not only on a specific distribution, but other platforms including Windows/Mac, 2) stability - including both forward and backward compatibility that are independent to external library interface changes, 3) performance - some of the libraries, such as sse_math and SFMT, are primarily used for their "inline" functions that are important for the software performance/speed, unless the library was specifically coded to ensure inlined functions, moving towards dynamic linking will likely result in speed loss. again, your points were loud and clear, but my assessment, as an upstream author and packger, is that the suggestion for decoupling the embedded libraries for separate packages and dynamic link with those will require a lot more work but no clear benefit (or even detrimental in terms of speed) for the software itself. So, I prefer to move forward in the current package/source code settings. Regarding the suggested "Provides: bundled(...)" line, I just want to make sure - this package neither compiles nor installs private copies of separate library files for sse_math/sfmt/json; it only statically compiles/links with them in our mmc/mmc.mex binaries. No one could directly call the functions in these embedded libraries from the generated binaries (mmc/mmc.mex). In this case, is a "Provide" directive still appropriate? I don't see how using packaged versions of static/header libraries could be detrimental. The roles of the "Provides: bundled()" idiom is to determine who makes use of a library in any way. The primary use is in case of a security issue with a library - how would you determine what packages would need to get fixed? For static libraries you would look at the BuildRequires. But if a package uses a bundled version, how would you tell? By examining the Provides: bundled() items. ok, I added the "Provides:bundled()" statements, see https://github.com/fangq/fedorapkg/commit/b86488daa0f88931ce15011de13ac898580592d1 but I want to mention that neither "ssemath" nor "cjson" currently exist in Fedora, is this a problem? rpmlint only complained about missing versions in provides. I confirm that SFMT is currently not compiled in this package - it will only be included when I use "%make_build -f makefile_sfmt", which is not the current building option. please let me know if you see any other places that I need to update. both the .spec and srpm files were updated with URL unchanged. Looks pretty good. A few issues are noted below. Once clarified, this can be approved. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - Package installs properly. Note: Installation errors (see attachment) See: https://docs.fedoraproject.org/en-US/packaging-guidelines/ ^ Please check this---fedora review failed to install the packages. - Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 5703680 bytes in 190 files. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_documentation ^ This is in a demos sub-package so that's OK. - octave-mmc has a mex file, so it should not be noarch, should it. Please check. The files should go to %octpkglibdir instead of %octpkgdir too. - Why is vim-common a BuildRequires? - Do we know what versions of libraries are being bundled here? If so, please version the Provides commands for them. - if SFMT is not being used, please delete it's source directory in %prep. This is done to ensure that it does not get enabled by mistake during a rebuild etc. - Cosmetic: you don't need to repeat BuildRequires in the sub-package definitions. ===== MUST items ===== C/C++: [x]: Provides: bundled(gnulib) in place as required. Note: Sources not installed [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [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: There is no build directory. Running licensecheck on vanilla upstream sources. No licenses found. Please check the source files for licenses manually. [x]: License file installed when any subpackage combination is installed. [x]: Package does not own files or directories owned by other packages. [x]: %build honors applicable compiler flags or justifies otherwise. ^ The build log has lots of compiler warnings, though. Please take a look. [-]: 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. [-]: Package contains systemd file(s) if in need. [x]: 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]: 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 must own all directories that it creates. [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 must not depend on deprecated() packages. [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: [-]: 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. [?]: Package functions as described. ^ Not tested [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Scriptlets must be sane, if used. [-]: 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]: 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: [!]: Rpmlint is run on all installed packages. Note: Mock build failed See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_use_rpmlint [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Installation errors ------------------- INFO: mock.py version 1.4.20 starting (python version = 3.7.5)... Start: init plugins INFO: selinux enabled Finish: init plugins INFO: Signal handler active Start: run Start: chroot init INFO: calling preinit hooks INFO: enabled root cache INFO: enabled dnf cache Start: cleaning dnf metadata Finish: cleaning dnf metadata INFO: enabled HW Info plugin Mock Version: 1.4.20 INFO: Mock Version: 1.4.20 Finish: chroot init INFO: installing package(s): /home/asinha/dump/fedora-review/1760617-mmc/results/mmc-debugsource-1.7.9-1.fc32.x86_64.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/mmc-debuginfo-1.7.9-1.fc32.x86_64.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/octave-mmclab-1.7.9-1.fc32.x86_64.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/mmc-1.7.9-1.fc32.x86_64.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/mmclab-demos-1.7.9-1.fc32.noarch.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/mmc-demos-1.7.9-1.fc32.noarch.rpm ERROR: Command failed: # /usr/bin/dnf --installroot /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 32 --setopt=deltarpm=False --allowerasing --disableplugin=local --disableplugin=spacewalk install /home/asinha/dump/fedora-review/1760617-mmc/results/mmc-debugsource-1.7.9-1.fc32.x86_64.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/mmc-debuginfo-1.7.9-1.fc32.x86_64.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/octave-mmclab-1.7.9-1.fc32.x86_64.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/mmc-1.7.9-1.fc32.x86_64.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/mmclab-demos-1.7.9-1.fc32.noarch.rpm /home/asinha/dump/fedora-review/1760617-mmc/results/mmc-demos-1.7.9-1.fc32.noarch.rpm --setopt=tsflags=nocontexts Rpmlint ------- Checking: mmc-1.7.9-1.fc32.x86_64.rpm octave-mmclab-1.7.9-1.fc32.x86_64.rpm mmclab-demos-1.7.9-1.fc32.noarch.rpm mmc-demos-1.7.9-1.fc32.noarch.rpm mmc-debuginfo-1.7.9-1.fc32.x86_64.rpm mmc-debugsource-1.7.9-1.fc32.x86_64.rpm mmc-1.7.9-1.fc32.src.rpm mmc.x86_64: W: no-manual-page-for-binary mmc octave-mmclab.x86_64: E: arch-dependent-file-in-usr-share /usr/share/octave/packages/mmclab-1.7.9/mmcl.mex octave-mmclab.x86_64: W: dangerous-command-in-%preun mv mmclab-demos.noarch: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases mmclab-demos.noarch: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases mmc-demos.noarch: W: spelling-error Summary(en_US) datasets -> data sets, data-sets, databases mmc-demos.noarch: W: spelling-error %description -l en_US datasets -> data sets, data-sets, databases mmc.src:15: W: unversioned-explicit-provides bundled(ssemath) mmc.src:16: W: unversioned-explicit-provides bundled(cjson) 7 packages and 0 specfiles checked; 1 errors, 8 warnings. Source checksums ---------------- https://github.com/fangq/mmc/archive/v1.7.9/mmc-1.7.9.tar.gz : CHECKSUM(SHA256) this package : 5c30e86d60cae5d9233165ed50684ae7bb37b5a503266c3aceacd07fca3c85fd CHECKSUM(SHA256) upstream package : 5c30e86d60cae5d9233165ed50684ae7bb37b5a503266c3aceacd07fca3c85fd Requires -------- mmc (rpmlib, GLIBC filtered): libOpenCL.so.1()(64bit) libOpenCL.so.1(OPENCL_1.0)(64bit) libOpenCL.so.1(OPENCL_1.2)(64bit) libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libgomp.so.1()(64bit) libgomp.so.1(GOMP_1.0)(64bit) libgomp.so.1(GOMP_4.0)(64bit) libgomp.so.1(OMP_1.0)(64bit) libm.so.6()(64bit) libpthread.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) octave octave-iso2mesh opencl-filesystem rtld(GNU_HASH) octave-mmclab (rpmlib, GLIBC filtered): /bin/sh libOpenCL.so.1()(64bit) libOpenCL.so.1(OPENCL_1.0)(64bit) libOpenCL.so.1(OPENCL_1.2)(64bit) libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libgomp.so.1()(64bit) libgomp.so.1(GOMP_1.0)(64bit) libgomp.so.1(GOMP_4.0)(64bit) libgomp.so.1(OMP_1.0)(64bit) libm.so.6()(64bit) libpthread.so.0()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) octave octave-iso2mesh opencl-filesystem rtld(GNU_HASH) mmclab-demos (rpmlib, GLIBC filtered): octave octave-iso2mesh octave-mmclab mmc-demos (rpmlib, GLIBC filtered): octave octave-iso2mesh mmc-debuginfo (rpmlib, GLIBC filtered): mmc-debugsource (rpmlib, GLIBC filtered): Provides -------- mmc: bundled(cjson) bundled(ssemath) mmc mmc(x86-64) octave-mmclab: octave-mmclab octave-mmclab(x86-64) mmclab-demos: mmclab-demos mmc-demos: mmc-demos mmc-debuginfo: debuginfo(build-id) mmc-debuginfo mmc-debuginfo(x86-64) mmc-debugsource: mmc-debugsource mmc-debugsource(x86-64) Generated by fedora-review 0.7.3 (44b83c7) last change: 2019-09-18 Command line :/usr/bin/fedora-review -b 1760617 Buildroot used: fedora-rawhide-{{ target_arch }} Active plugins: C/C++, Generic, Shell-api Disabled plugins: Perl, R, SugarActivity, PHP, Java, Python, Haskell, Ocaml, fonts Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH Created attachment 1629741 [details]
Build log showing compiler warnings
Build log is attached for your information. While these warnings do not need to be fixed for this review, it's worth looking into them.
thanks for the review. the updated spec file can be found at https://raw.githubusercontent.com/fangq/fedorapkg/mmclab/mmc.spec but it does not build any more. the issue is related to how to "%octpkglibdir". I would like to know how to fix this. > Please check this---fedora review failed to install the packages. I managed to reproduce this issue using mock, see the mock log attached at the bottom. basically, the issue is that mmc/octave-mmclab has a dependency to octave-iso2mesh, but it complains libmpfr.so.4 could not be installed. I found that this .so file is part of mpfr package. I already listed mpfr-devel in octave-iso2mesh's spec file, and from the mpfr's spec, it the devel package depends on the main package, so assuming that both mpfr and mpfr-devel should have been installed for octave-iso2mesh. but somehow, they were not. can you let me know if this is a problem for iso2mesh? or I did not use mock properly? > octave-mmc has a mex file, so it should not be noarch, should it. Please > check. The files should go to %octpkglibdir instead of %octpkgdir too. I did not specify BuildArch for octave-mmclab. From the generated rpms, it seems is not noarch. the only two noarch packages are mmc-demos and mmclab-demos, can you verify if this is the case? I tried to follow the arch-specific octave template https://fedoraproject.org/wiki/Packaging:Octave#Arch_specific_Octave_spec_template and made the follow change, but rpmbuild failed, complaining that directory not found. is this the right way to add %{octpkglibdir}? I also removed %dir as the template showed, then it complains both file missing and dir missing. error: Directory not found: /home/fangq/rpmbuild/BUILDROOT/mmc-1.7.9-1.fc30.x86_64/usr/lib64/octave/packages/mmclab-1.7.9 ------------------------------------- diff --git a/mmc.spec b/mmc.spec index ca632bd..5a7c194 100644 --- a/mmc.spec +++ b/mmc.spec @@ -196,7 +196,7 @@ install -m 0755 -pt %{buildroot}%{_bindir} bin/%{name} %doc README.txt AUTHORS.txt %dir %{octpkgdir} %{octpkgdir}/*.m -%{octpkgdir}/*.mex +%dir %{octpkglibdir} %doc %{octpkgdir}/doc-cache %{octpkgdir}/packinfo ---------------------------------------- > Why is vim-common a BuildRequires? see my earlier replies. I need the "xxd" command in vim-common to preprocess the OpenCL kernel. so, building mmc requires this utility. > Do we know what versions of libraries are being bundled here? If so, please > version the Provides commands for them. the cjson I have is from 2015-12-20, which is older than v0.0.0 in the github, so I listed 0.0 in the version; the ssemath's website has a version updated in 2008, which is the one I used. Because it is not version controlled, I also listed 0.0. https://github.com/fangq/fedorapkg/commit/26960e9fc798068a49bb54e0ecdc55f6f7685083 > if SFMT is not being used, please delete it's source directory in %prep. This > is done to ensure that it does not get enabled by mistake during a rebuild > etc. done https://github.com/fangq/fedorapkg/commit/d32719a7245e3f49f445d7eaca059fff47018b06 > Cosmetic: you don't need to repeat BuildRequires in the sub-package > definitions. done https://github.com/fangq/fedorapkg/commit/c3e71ef59d37714af0a526f6e27d92807f91b117 ------------------------------------------------- fangq@localhost:/var/lib/mock/fedora-rawhide-x86_64/result$ mock -r fedora-32-x86_64 --install octave-mmclab-debuginfo-1.7.9-1.fc32.x86_64.rpm mmc-debuginfo-1.7.9-1.fc32.x86_64.rpm mmc-debugsource-1.7.9-1.fc32.x86_64.rpm mmc-demos-1.7.9-1.fc32.noarch.rpm mmclab-demos-1.7.9-1.fc32.noarch.rpm octave-mmclab-1.7.9-1.fc32.x86_64.rpm mmc-1.7.9-1.fc32.x86_64.rpm INFO: mock.py version 1.4.20 starting (python version = 3.7.5)... Start: init plugins INFO: selinux enabled Finish: init plugins INFO: Signal handler active Start: run Start: chroot init INFO: calling preinit hooks INFO: enabled root cache INFO: enabled dnf cache Start: cleaning dnf metadata Finish: cleaning dnf metadata INFO: enabled HW Info plugin Mock Version: 1.4.20 INFO: Mock Version: 1.4.20 Finish: chroot init INFO: installing package(s): octave-mmclab-debuginfo-1.7.9-1.fc32.x86_64.rpm mmc-debuginfo-1.7.9-1.fc32.x86_64.rpm mmc-debugsource-1.7.9-1.fc32.x86_64.rpm mmc-demos-1.7.9-1.fc32.noarch.rpm mmclab-demos-1.7.9-1.fc32.noarch.rpm octave-mmclab-1.7.9-1.fc32.x86_64.rpm mmc-1.7.9-1.fc32.x86_64.rpm No matches found for the following disable plugin patterns: local, spacewalk fedora 29 kB/s | 12 kB 00:00 Error: Problem 1: package mmc-1.7.9-1.fc32.x86_64 requires octave-iso2mesh, but none of the providers can be installed - conflicting requests - nothing provides libmpfr.so.4()(64bit) needed by octave-iso2mesh-1.9.1-3.fc32.x86_64 Problem 2: package octave-mmclab-1.7.9-1.fc32.x86_64 requires octave-iso2mesh, but none of the providers can be installed - conflicting requests - nothing provides libmpfr.so.4()(64bit) needed by octave-iso2mesh-1.9.1-3.fc32.x86_64 Problem 3: package mmclab-demos-1.7.9-1.fc32.noarch requires octave-iso2mesh, but none of the providers can be installed - conflicting requests - nothing provides libmpfr.so.4()(64bit) needed by octave-iso2mesh-1.9.1-3.fc32.x86_64 Problem 4: package mmc-demos-1.7.9-1.fc32.noarch requires octave-iso2mesh, but none of the providers can be installed - conflicting requests - nothing provides libmpfr.so.4()(64bit) needed by octave-iso2mesh-1.9.1-3.fc32.x86_64 (try to add '--skip-broken' to skip uninstallable packages or '--nobest' to use not only best candidate packages) ERROR: Command failed: # /usr/bin/dnf --installroot /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 32 --setopt=deltarpm=False --allowerasing --disableplugin=local --disableplugin=spacewalk install octave-mmclab-debuginfo-1.7.9-1.fc32.x86_64.rpm mmc-debuginfo-1.7.9-1.fc32.x86_64.rpm mmc-debugsource-1.7.9-1.fc32.x86_64.rpm mmc-demos-1.7.9-1.fc32.noarch.rpm mmclab-demos-1.7.9-1.fc32.noarch.rpm octave-mmclab-1.7.9-1.fc32.x86_64.rpm mmc-1.7.9-1.fc32.x86_64.rpm --setopt=tsflags=nocontexts @ankur, I'd like to reviving this thread, and fix the issue you suggested - can you take a look at my last post and let me know 1) what's the correct way to set octpkglibdir and 2) are other changes sufficient? thanks I just realised this was still WIP! Sorry about that! I'll report back in a day or two. It's back on my list now. :) (In reply to Qianqian Fang from comment #23) > thanks for the review. > > the updated spec file can be found at > > https://raw.githubusercontent.com/fangq/fedorapkg/mmclab/mmc.spec > > but it does not build any more. the issue is related to how to > "%octpkglibdir". I would like to know how to fix this. > > > > Please check this---fedora review failed to install the packages. > > I managed to reproduce this issue using mock, see the mock log attached at > the bottom. > > basically, the issue is that mmc/octave-mmclab has a dependency to > octave-iso2mesh, but it complains libmpfr.so.4 could not be installed. I > found that this .so file is part of mpfr package. > > I already listed mpfr-devel in octave-iso2mesh's spec file, and from the > mpfr's spec, it the devel package depends on the main package, so assuming > that both mpfr and mpfr-devel should have been installed for > octave-iso2mesh. but somehow, they were not. > > can you let me know if this is a problem for iso2mesh? or I did not use mock > properly? I'm not seeing the dep error anymore. So I'm not sure what was happening here. I don't think iso2mesh should Require: mpfr-devel, but we can discuss that in a different bug (or on the ML). > > > octave-mmc has a mex file, so it should not be noarch, should it. Please > > check. The files should go to %octpkglibdir instead of %octpkgdir too. > > I did not specify BuildArch for octave-mmclab. From the generated rpms, it > seems is not noarch. the only two noarch packages are mmc-demos and > mmclab-demos, can you verify if this is the case? > > I tried to follow the arch-specific octave template > > https://fedoraproject.org/wiki/Packaging: > Octave#Arch_specific_Octave_spec_template > > and made the follow change, but rpmbuild failed, complaining that directory > not found. is this the right way to add %{octpkglibdir}? I also removed %dir > as the template showed, then it complains both file missing and dir missing. > I've opened a PR with a tweak that just puts the mex file in the right place. The octave macros don't seem to do it---but I will email the ML to confirm. If you can have a look at it and submit the updated spec/srpm links, I'll re-run fedora-review on everything. I think we're quite close to completion here. :) @Ankur, thanks. I finally got my virtualbox working again. I did a few minor changes (re-tagging upstream release, sync source file, and then drop the branch name), now I was able to build mmc/octave-mmclab/mmc-demos/mmclab-demos packages without any error. Spec URL: https://github.com/fangq/fedorapkg/blob/mmclab/mmc.spec SRPM URL: https://kwafoo.coe.neu.edu/~fangq/share/temp/mmc-1.7.9-1.fc30.src.rpm I tested the binary package (mmc) and it works fine. But when testing the octave-mmclab package, for some reason, octave fails to find the mex file. The .mex file is now located in /usr/lib64/octave/packages/mmclab-1.7.9/mmc.mex after installation, however, when I type "pkg mmclab load" in octave, I could only load the noarch .m files in the package (they are stored in /usr/share/octave/packages/mmclab-1.7.9/mmclab.m), but not the .mex file. did I miss any step in order to use the .mex file? thanks Can you give me the complete steps on how to check this please? I built it for f31, and it loads OK. What should one do next to verify if the mex file can be used?
(ins)>> pkg list
Package Name | Version | Installation directory
--------------+---------+-----------------------
iso2mesh | 1.9.1 | /usr/share/octave/packages/iso2mesh-1.9.1
jnifti | 0.5 | /usr/share/octave/packages/jnifti-0.5
jsonlab | 1.9.8 | /usr/share/octave/packages/jsonlab-1.9.8
mmclab | 1.7.9 | /usr/share/octave/packages/mmclab-1.7.9
zmat | 0.9 | /usr/share/octave/packages/zmat-0.9
(ins)>> pkg load mmclab
(ins)>> pkg mmclab load
(ins)>> pkg list
Package Name | Version | Installation directory
--------------+---------+-----------------------
iso2mesh *| 1.9.1 | /usr/share/octave/packages/iso2mesh-1.9.1
jnifti *| 0.5 | /usr/share/octave/packages/jnifti-0.5
jsonlab *| 1.9.8 | /usr/share/octave/packages/jsonlab-1.9.8
mmclab *| 1.7.9 | /usr/share/octave/packages/mmclab-1.7.9
zmat *| 0.9 | /usr/share/octave/packages/zmat-0.9
(ins)>>
/usr/share/octave/octave_packages correctly lists the package directories, so that looks OK too:
# name: dir
# type: sq_string
# elements: 1
# length: 39
/usr/share/octave/packages/mmclab-1.7.9
# name: archprefix
# type: sq_string
# elements: 1
# length: 39
/usr/lib64/octave/packages/mmclab-1.7.9
I do not, however, see this in the output of path():
(ins)>> path()
Octave's search path contains the following directories:
.
/usr/share/octave/packages/zmat-0.9
/usr/share/octave/packages/jsonlab-1.9.8
/usr/share/octave/packages/jnifti-0.5
/usr/share/octave/packages/iso2mesh-1.9.1
/usr/share/octave/packages/mmclab-1.7.9
/usr/lib64/octave/5.1.0/site/oct/x86_64-redhat-linux-gnu
/usr/lib64/octave/site/oct/api-v53/x86_64-redhat-linux-gnu
/usr/lib64/octave/site/oct/x86_64-redhat-linux-gnu
/usr/share/octave/5.1.0/site/m
/usr/share/octave/site/api-v53/m
/usr/share/octave/site/m
/usr/share/octave/site/m/startup
/usr/lib64/octave/5.1.0/oct/x86_64-redhat-linux-gnu
/usr/share/octave/5.1.0/m
/usr/share/octave/5.1.0/m/audio
/usr/share/octave/5.1.0/m/deprecated
/usr/share/octave/5.1.0/m/elfun
/usr/share/octave/5.1.0/m/general
/usr/share/octave/5.1.0/m/geometry
/usr/share/octave/5.1.0/m/gui
/usr/share/octave/5.1.0/m/help
/usr/share/octave/5.1.0/m/image
/usr/share/octave/5.1.0/m/io
/usr/share/octave/5.1.0/m/java
/usr/share/octave/5.1.0/m/legacy
/usr/share/octave/5.1.0/m/linear-algebra
/usr/share/octave/5.1.0/m/miscellaneous
/usr/share/octave/5.1.0/m/ode
/usr/share/octave/5.1.0/m/optimization
/usr/share/octave/5.1.0/m/path
/usr/share/octave/5.1.0/m/pkg
/usr/share/octave/5.1.0/m/plot
/usr/share/octave/5.1.0/m/plot/appearance
/usr/share/octave/5.1.0/m/plot/draw
/usr/share/octave/5.1.0/m/plot/util
/usr/share/octave/5.1.0/m/polynomial
/usr/share/octave/5.1.0/m/prefs
/usr/share/octave/5.1.0/m/profiler
/usr/share/octave/5.1.0/m/set
/usr/share/octave/5.1.0/m/signal
/usr/share/octave/5.1.0/m/sparse
/usr/share/octave/5.1.0/m/specfun
/usr/share/octave/5.1.0/m/special-matrix
/usr/share/octave/5.1.0/m/startup
/usr/share/octave/5.1.0/m/statistics
/usr/share/octave/5.1.0/m/strings
/usr/share/octave/5.1.0/m/testfun
/usr/share/octave/5.1.0/m/time
/usr/share/octave/5.1.0/data
to verify the toolbox, simply type "mmc" and enter in Octave, you should see a short help info.
also, run mmclab('gpuinfo'), this should call mmc.mex and list all supported OpenCL devices.
because 'which mmc' currently does not return anything, so the above two command will not succeed.
Orion answered the query on the mailing list. I've opened another PR that seems to do the trick. Please have a look. Cheers
```
(ins)>> pkg list
Package Name | Version | Installation directory
--------------+---------+-----------------------
iso2mesh | 1.9.1 | /usr/share/octave/packages/iso2mesh-1.9.1
jnifti | 0.5 | /usr/share/octave/packages/jnifti-0.5
jsonlab | 1.9.8 | /usr/share/octave/packages/jsonlab-1.9.8
mmclab | 1.7.9 | /usr/share/octave/packages/mmclab-1.7.9
zmat | 0.9 | /usr/share/octave/packages/zmat-0.9
(ins)>> pkg load mmclab
(ins)>> mmc
Usage:
[flux,detphoton]=mmclab(cfg);
Please run 'help mmclab' for more details.
(ins)>> mmclab('gpuinfo')
Platform [0] Name Clover
error: mmc: no active GPU device found
error: called from
mmclab at line 265 column 17
(ins)>> which mmc
'mmc' is a function from the file /usr/lib64/octave/packages/mmclab-1.7.9/x86_64-redhat-linux-gnu-api-v53/mmc.mex
(ins)>>
```
I forgot to say. Please run a scratch build on koji to test if it builds on all architectures. When I tried it here, it didn't compile on them all. koji build rawhide <path to srpm> --scratch @Ankur, the error was caused by the lack of support of SSE instructions on ARM, here is part of the log ``` Building built/xorshift128p_rand.o cc -c -Wall -g -DMCX_EMBED_CL -fno-strict-aliasing -DMMC_USE_SSE -DHAVE_SSE2 -msse -msse2 -msse3 -mssse3 -msse4.1 -O3 -fopenmp -fPIC -DMCX_CONTAINER -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -march=armv7-a -mfpu=vfpv3-d16 -mtune=generic-armv7-a -mabi=aapcs-linux -mfloat-abi=hard -DUSE_OS_TIMER -DUSE_OPENCL -I../src -o built/xorshift128p_rand.o xorshift128p_rand.c cc1: error: unrecognized command-line option '-msse' cc1: error: unrecognized command-line option '-msse2' cc1: error: unrecognized command-line option '-msse3' cc1: error: unrecognized command-line option '-mssse3' cc1: error: unrecognized command-line option '-msse4.1' make: *** [../commons/Makefile_common.mk:188: built/xorshift128p_rand.o] Error 1 ``` I can use a conditional flag to let it compile without SSE, but I don't think it is much useful, and requires some additional tweaks of my makefile, so I think the best way moving forward is to exclude arm in this spec for now, and support it for future updates. I am wondering what is the right way to exclude arch? I added ExcludeArch: armv7h1 but does not seem to skip arm at in my koji --scratch build. That sounds OK. Please take a look at the guidelines, they explain how such cases should be handled: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_support ExcludeArch: %{arm} would probably do the trick. As the guidelines say, one must file bugs to document that the package does not support ARM. Additionally, no packages that require this one can be added to ARM either. Cheers, Ankur thanks. it was not just arm, but all platforms other than i686 and x86_64. see https://koji.fedoraproject.org/koji/taskinfo?taskID=41672272 so I did the following: 1. tweaked my Makefile so that it can compile on non ix86/x86_64 platforms 2. recreated a release, and updated the source file 3. updated the spec file using %ifarch after this update, I was able to compile on all platforms, see https://koji.fedoraproject.org/koji/taskinfo?taskID=41698953 However I want to mention that the simulation results on other platforms are not quite correct. I will leave this for further source code updates. For packaging, I think this should be sufficient. Please let me know what you think. both spec and srpm files are updated. Spec URL: https://github.com/fangq/fedorapkg/blob/mmclab/mmc.spec SRPM URL: https://kwafoo.coe.neu.edu/~fangq/share/temp/mmc-1.7.9-1.fc30.src.rpm (In reply to Qianqian Fang from comment #34) > thanks. it was not just arm, but all platforms other than i686 and x86_64. > see > > https://koji.fedoraproject.org/koji/taskinfo?taskID=41672272 > > > so I did the following: > > 1. tweaked my Makefile so that it can compile on non ix86/x86_64 platforms > > 2. recreated a release, and updated the source file > > 3. updated the spec file using %ifarch > > after this update, I was able to compile on all platforms, see > > https://koji.fedoraproject.org/koji/taskinfo?taskID=41698953 That looks good. > However I want to mention that the simulation results on other platforms are > not quite correct. I will leave this for further source code updates. For > packaging, I think this should be sufficient. If the simulation results aren't correct, do we want to provide mmc for these arches (I worry about the scenario where someone does use the tool on these arches and comes up with wrong scientific results)? Maybe excluding them would be the correct thing to do? That's perfectly OK as long as we follow the system and file bugs to record it. My preference is to get the package out first. I will create a bug ticket in my mmc github repo, and track this. there is a special combination of flags can make the results correct, but I will have to educate my users in the documentation. let me know if you can approve this package. thanks for all the help. (In reply to Qianqian Fang from comment #36) > My preference is to get the package out first. Sure, but what I'm saying is that you do *not* have to build the package for all arches for the package to be approved. If upstream (you) do not support all arches, they can be excluded as per the guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_support : "If a Fedora package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch needs to have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture..." So, all you have to do is file the tracker bugs. Isn't that better than providing the package for all arches, but it working incorrectly on a few? It's totally your call. If this weren't a scientific tool where correctness is paramount, I wouldn't care so much either :) From an rpm perspective, the package looks good. XXX APPROVED XXX I'll let you decide what arches etc. you want to support @fangq, since you are upstream here anyway. Cheers, thanks Ankur for approving. I was able to fix the incorrect results with this patch https://github.com/fangq/mmc/commit/99312c3c1c8b9cfb932930a932c94f7a11aab976 now I free comfortable to move forward to create the package. (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/mmc FEDORA-2020-402060f3b7 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2020-402060f3b7 FEDORA-2020-626c7c163e has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-626c7c163e mmc-1.7.9-1.fc32 has been pushed to the Fedora 32 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-2020-626c7c163e mmc-1.7.9-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-2020-e50e3bb623 mmc-1.7.9-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-2020-402060f3b7 mmc-1.7.9-1.fc31 has been pushed to the Fedora 31 stable repository. If problems still persist, please make note of it in this bug report. mmc-1.7.9-1.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report. mmc-1.7.9-1.fc32 has been pushed to the Fedora 32 stable repository. If problems still persist, please make note of it in this bug report. mmc-1.7.9-1.fc32 has been pushed to the Fedora 32 stable repository. If problems still persist, please make note of it in this bug report. |