https://bugzilla.redhat.com/show_bug.cgi?id=1723013
Bug ID: 1723013 Summary: Review Request: mne-cpp - A Framework for Electrophysiology Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: b38ghk29@contbay.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://drive.google.com/file/d/1SxTNt7ZTMgHkEihgUvZhtjR0vny5XQPo/view?usp=s... SRPM URL: https://drive.google.com/file/d/1e3Y5CKKvGXYrKF6qw-WsudN2al0MRcQE/view?usp=s... Description: The mne-cpp (cpp = c++) code base is a framework for acquiring/capturing electrophysiology data (in real-time) and/or browsing and analyzing such data. It is a re-write of the original C code base. See this URL for some of the different programs available in the mne-cpp distribution, https://www.mne-cpp.org/index.php/info/
Fedora Account System Username: redhat bugzilla email b38ghk29@contbay.com
I have done build and install so far only on fc28 updated to have Qt 5.11.3.
"sudo dnf builddep mne-cpp.spec" installed dependent Qt packages for build.
"mock -v mne-cpp-master-1.0.fc28.src.rpm" built the source RPM and existed with status 0.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
David Auer dreua@posteo.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dreua@posteo.de
--- Comment #1 from David Auer dreua@posteo.de --- These are not direct download links, did you read https://fedoraproject.org/wiki/Package_Review_Process ?
This way the fedora-review tool works and people need less clicks: Spec URL: https://drive.google.com/uc?export=download&id=1SxTNt7ZTMgHkEihgUvZhtjR0... SRPM URL: https://drive.google.com/uc?export=download&id=1e3Y5CKKvGXYrKF6qw-WsudN2...
I am also a bit confused about your Fedora account system username and the fact you are using Fedora 28 (it is end of life).
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #2 from David Auer dreua@posteo.de --- Okay, it turns out that fedora-review looks for the file ending, so maybe this hack works:
Spec URL: https://drive.google.com/uc?export=download&id=1SxTNt7ZTMgHkEihgUvZhtjR0... SRPM URL: https://drive.google.com/uc?export=download&id=1e3Y5CKKvGXYrKF6qw-WsudN2...
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #3 from b38ghk29@contbay.com --- I have rebuilt/created RPM's on fc29 and have direct download URL's, but where do you edit the previous URL entries?
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #4 from David Auer dreua@posteo.de --- AFAIK you can not edit previous messages. Just post the URLs in a new comment. Unfortunately my tries to convert your URLs to direct download weren't successful.
When I last commented I took a short peek at your spec file and if I recall correctly there are some things that need to be fixed, if you haven't already please make sure your Version/Release, License and makro usage meets the guidelines (https://docs.fedoraproject.org/en-US/packaging-guidelines/). I will try to make a full unofficial review in the next days. (I am not a packager yet so all of this is unofficial!)
I think downloading sources via curl in %setup is not allowed, but not entirely sure in this case, because it wouldn't happen for Fedora but only for rhel builds.
Additionally it would be a good idea to do a koji sratch build (also post a link when it's done) and I personally ran the fedora-review tool on my own package, it might just help you to find some mistakes and fix them yourself.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #5 from b38ghk29@contbay.com --- Thank you for the info and the update. I saw in the packaging docs that downloading is discouraged, but given there is no release yet of the code (or official package), then I was not sure what else to do besides fetch the source from github. I did run mock, but I will look at the other things you mentioned. For now here are the updated URLs,
Spec URL: https://drive.google.com/uc?export=download&id=1SxTNt7ZTMgHkEihgUvZhtjR0... SRPM URL: https://drive.google.com/uc?export=download&id=1Os82KO4Bmqv5uARWZa-PUm3c...
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
Artur Iwicki fedora@svgames.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@svgames.pl
--- Comment #6 from Artur Iwicki fedora@svgames.pl ---
Version: master Release: 1.0%{?dist}
You should use the upstream version number (1.0.0) and include snapshot info in the Release field. See the Versioning Guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
Source0: https://github.com/mne-tools/mne-cpp/archive/master.zip
Related to the point above - you should explicitly name a specific commit here. "master.zip" is a moving target.
curl -s https://raw.githubusercontent.com/jcfr/qt-easy-build/5.10.0/Build-qt.sh -o Build-qt.sh
This will fail since official package builds are done with disabled Internet access. Everything that's needed for building the package should be listed in Source: and Patch: tags.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zebob.m@gmail.com
--- Comment #7 from Robert-André Mauchin zebob.m@gmail.com --- - This is a no: you don't have internet access in Koji/Mock during the build
%if 0%{?rhel} echo in_setup_for_RedHat curl -s https://raw.githubusercontent.com/jcfr/qt-easy-build/5.10.0/Build-qt.sh -o Build-qt.sh chmod 755 Build-qt.sh ./Build-qt.sh -y -c -j4 -q $HOME/qt %endif pwd
Product: Fedora Version: rawhide Component: Package Review
b38ghk29@contbay.com has canceled Package Review package-review@lists.fedoraproject.org's request for b38ghk29@contbay.com's needinfo: Bug 1723013: Review Request: mne-cpp - A Framework for Electrophysiology https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #9 from b38ghk29@contbay.com --- OK, so per your feedback above on 2019-07-13 14:14:21 UTC, then the current Qt distribution has to be built from source prior to building the software applications that use it?
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
b38ghk29@contbay.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(b38ghk29@contbay. | |com) |
--- Comment #9 from b38ghk29@contbay.com --- OK, so per your feedback above on 2019-07-13 14:14:21 UTC, then the current Qt distribution has to be built from source prior to building the software applications that use it?
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #10 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- # CentOS/RedHat needs Qt 5.10 to be downloaded and built (no rpm for install) %if 0%{?rhel} echo in_setup_for_RedHat curl -s https://raw.githubusercontent.com/jcfr/qt-easy-build/5.10.0/Build-qt.sh -o Build-qt.sh chmod 755 Build-qt.sh ./Build-qt.sh -y -c -j4 -q $HOME/qt %endif pwd
If Qt > 5.10 is required then you can't support RHEL < 7. You can target RHEL 8 (not sure of Qt's version here, please check).
- Secondly you need to use nacros:
%build PATH=/usr/lib64/qt5/bin:$PATH qmake --version qmake -makefile -recursive make
→
%{qmake_qt5} %make_build
- Do not install in /usr/local, use %{_prefix} macro which is /usr
- No idea what you are trying to do here:
%install # echo %{buildroot} # copy over subdirs from build mkdir -p %{buildroot}/%{_prefix}/mne-cpp tar cpf - ./bin ./lib ./doc | (cd %{buildroot}/%{_prefix}/mne-cpp; tar xpf -)
- Also
Source0: https://github.com/mne-tools/mne-cpp/archive/master.zip
you can't just pount to master like this, you need to d/l a specific release, if not are provided, then target a specific commit. It seems 0.1.4 is the latest release.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
b38ghk29@contbay.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |nobody@fedoraproject.org Flags| |needinfo?(nobody@fedoraproj | |ect.org)
--- Comment #11 from b38ghk29@contbay.com --- Please correct me if I am wrong, but my understanding is that a *src* rpm is still required, and so the source code must build from scratch (apart from subsequently creating a binary RPM).
Also please advise on what Fedora releases are acceptable to compile/test on, i.e., fc31 or fc32. The reason I ask is because with newer compilers, the current 0.1.6 pre-release may not compile w/o warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #12 from Artur Frenszek-Iwicki fedora@svgames.pl --- I don't think we have a requirement as to which release one has is allowed to use for building SRPMs for review, though I guess the unspoken expectation is "something that's not EOL". That being said, you can use mock, or koji scratch builds to verify that your package builds fine on Fedora Rawhide. https://fedoraproject.org/wiki/Using_the_Koji_build_system#Scratch_Builds https://fedoraproject.org/wiki/Using_Mock_to_test_package_builds
Once you have a successful scratch build, it's good to add a link to it alongside the SPEC and SRPM - just for showing reviewers that the package builds fine.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
code@musicinmybrain.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |code@musicinmybrain.net
--- Comment #17 from code@musicinmybrain.net --- Are you still working on this? It looks like the actual software is probably feasible for inclusion in Fedora, but you will need to take the time to do quite a bit of listening to feedback and reading the guidelines to get there.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #18 from b38ghk29@contbay.com --- Please advise as to the current version of fedora core in use for package testing/review.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #19 from Ben Beasley code@musicinmybrain.net --- (In reply to b38ghk29 from comment #18)
Please advise as to the current version of fedora core in use for package testing/review.
Reviews are generally done using Rawhide, the development version of Fedora Linux. Reviewers use the “fedora-review” tool to help find things that need a second look, and you can run it too; it saves a lot of time if you’ve already handled the issues that the tool can find automatically.
Rawhide is where an approved package would initially appear. You could then request branches for other supported releases (f34, f33), or for EPEL (epel8/epel7). See https://docs.fedoraproject.org/en-US/fesco/Updates_Policy/.
You can do your development on another version, like Fedora 33 or 34, using mock (https://github.com/rpm-software-management/mock/wiki). Some people even use CentOS or RHEL. You’ll have the best results using a release that’s currently receiving updates rather than one that’s end-of-life (Fedora 32 or older).
Please see https://fedoraproject.org/wiki/Package_Review_Process and https://fedoraproject.org/wiki/New_package_process_for_existing_contributors, and https://fedoraproject.org/wiki/Join_the_package_collection_maintainers if you are not yet a Fedora packager.
(The name “Fedora Core” was last used for Fedora Core 6, in 2006.)
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #20 from Ben Beasley code@musicinmybrain.net --- Note that a source RPM built on Fedora 33 or 34 works just fine as an input for a Rawhide mock build; there’s no expectation that your source RPM has to have a “.fc35” (soon “.fc36”) in its name.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
k4hw8m62b@convergenceservice.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(k4hw8m62b@converg | |enceservice.com) |
--- Comment #22 from k4hw8m62b@convergenceservice.com --- Please advise as to how python distributions (to host python based tools that are part of the release) are handled as part of a C/C++ source rpm build. It's not clear to me if including a python distribution is allowed to be included in the src rpm or perhaps python would need to be a separate package apart from the source rpm package where the python distribution package would be a dependency listed in the source rpm package.
Product: Fedora Version: rawhide Component: Package Review
k4hw8m62b@convergenceservice.com has canceled Package Review package-review@lists.fedoraproject.org's request for k4hw8m62b@convergenceservice.com's needinfo: Bug 1723013: Review Request: mne-cpp - A Framework for Electrophysiology https://bugzilla.redhat.com/show_bug.cgi?id=1723013
--- Comment #22 from k4hw8m62b@convergenceservice.com --- Please advise as to how python distributions (to host python based tools that are part of the release) are handled as part of a C/C++ source rpm build. It's not clear to me if including a python distribution is allowed to be included in the src rpm or perhaps python would need to be a separate package apart from the source rpm package where the python distribution package would be a dependency listed in the source rpm package.
https://bugzilla.redhat.com/show_bug.cgi?id=1723013
Troy Curtis troy@troycurtisjr.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(k4hw8m62b@converg | |enceservice.com) CC| |troy@troycurtisjr.com
--- Comment #23 from Troy Curtis troy@troycurtisjr.com --- A few points:
* All dependencies need to be available in the Fedora/EPEL release you are targeting. If a new enough version of a library/tool isn't available, then a separate package request with that tool will need to be submitted and added to the distribution. * The consequence of the above point is that all that should be built as part of the source RPM is the software being packaged itself.
On the question of the Python tools, the only Python I see is the mnetracer.py script. Since this is a helper tool intended to be used as a cli application, it probably doesn't need it's own sub-package. Unless there is another piece that I'm missing?
Lastly, do you have a FAS account already? If so what is your FAS username. If you do not, the first step will be going through the process described here https://docs.fedoraproject.org/en-US/package-maintainers/Joining_the_Package....
Until the FAS account is given, and an updated spec & source RPM is provided addressing the existing comments, we should leave the NEEDSINFO state in place to alert other potential reviewers that you are working on updates.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com