https://bugzilla.redhat.com/show_bug.cgi?id=1509034
Bug ID: 1509034 Summary: Review Request: phd2 - Telescope guiding software Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: mattia.verga@email.it QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://mattia.fedorapeople.org/phd2.spec SRPM URL: https://mattia.fedorapeople.org/phd2-2.6.4-1.fc28.src.rpm Description: PHD2 is telescope guiding software that simplifies the process of tracking a guide star, letting you concentrate on other aspects of deep-sky imaging or spectroscopy. Fedora Account System Username: mattia
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
--- Comment #1 from Mattia Verga mattia.verga@email.it --- Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=22869267
$ rpmlint SRPMS/phd2-2.6.4-1.fc28.src.rpm RPMS/x86_64/phd2-2.6.4-1.fc28.x86_64.rpm RPMS/x86_64/phd2-debuginfo-2.6.4-1.fc28.x86_64.rpm RPMS/x86_64/phd2-debugsource-2.6.4-1.fc28.x86_64.rpm phd2.src: W: strange-permission generate-tarball.sh 775 phd2.src: W: invalid-url Source0: phd2-2.6.4-purged.tar.xz phd2.x86_64: W: only-non-binary-in-usr-lib phd2.x86_64: W: no-documentation phd2.x86_64: W: no-manual-page-for-binary phd2 phd2-debugsource.x86_64: W: no-documentation 4 packages and 0 specfiles checked; 0 errors, 6 warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
Mattia Verga mattia.verga@email.it changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1159999 (Astronomy-SIG)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1159999 [Bug 1159999] Astronomy SIG - review tracker
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
--- Comment #2 from Mattia Verga mattia.verga@email.it --- Spec URL: https://mattia.fedorapeople.org/phd2.spec SRPM URL: https://mattia.fedorapeople.org/phd2-2.6.4-2.fc28.src.rpm Description: PHD2 is telescope guiding software that simplifies the process of tracking a guide star, letting you concentrate on other aspects of deep-sky imaging or spectroscopy. Fedora Account System Username: mattia
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hobbes1069@gmail.com Assignee|nobody@fedoraproject.org |hobbes1069@gmail.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
--- Comment #3 from Richard Shaw hobbes1069@gmail.com --- Quick spec review first:
Could change:
In %prep: %setup
to
%autosetup -p1
---
In %build: make %{?_smp_mflags} -C %{_target_platform}
to
%make_build
There's no reason to popd out of the directory, you can run make there and then you don't need the -C...
---
In %install make install -C %{_target_platform} DESTDIR=%{buildroot}
to
pushd %{_target_platform} %make_install popd (for find_lang)
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
--- Comment #4 from Richard Shaw hobbes1069@gmail.com --- Ok, a couple of questions:
1. There is one directory left in the "thirdparty" directory. Is it required and not bundled?
2. The build generates a LOT of warnings, it was difficult to find a compile line to verify the build flags were being honored.
The project seems to be using C++11 but in Fedora 26 and up, C++14 is standard. Unless the project will not build with C++14, the flag should be removed.
3. rpmlint doesn't like the ICU license.. It looks like an MIT variant, correct?
4. These are only warnings but they should be fixed:
phd2.x86_64: W: spurious-executable-perm /usr/share/doc/phd2/PHD_2.0_Architecture.docx
In %install: chmod 0644 %{buildroot}%{_docdir}/%{name}/PHD_2.0_Architecture.docx
phd2.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/phd2/README-PHD2.txt
There are several options for fixing line endings...
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
--- Comment #5 from Mattia Verga mattia.verga@email.it --- (In reply to Richard Shaw from comment #4)
Ok, a couple of questions:
- There is one directory left in the "thirdparty" directory. Is it required
and not bundled?
Removed.
- The build generates a LOT of warnings, it was difficult to find a compile
line to verify the build flags were being honored.
The project seems to be using C++11 but in Fedora 26 and up, C++14 is standard. Unless the project will not build with C++14, the flag should be removed.
Added a patch to use C++14. I might ask upstream if the patch is feasible to be included upstream.
- rpmlint doesn't like the ICU license.. It looks like an MIT variant,
correct?
Yes, changed to MIT: https://fedoraproject.org/wiki/Licensing:MIT?rd=Licensing/MIT#Modern_style_....
- These are only warnings but they should be fixed:
phd2.x86_64: W: spurious-executable-perm /usr/share/doc/phd2/PHD_2.0_Architecture.docx
In %install: chmod 0644 %{buildroot}%{_docdir}/%{name}/PHD_2.0_Architecture.docx
phd2.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/phd2/README-PHD2.txt
There are several options for fixing line endings...
Fixed.
Spec URL: https://mattia.fedorapeople.org/phd2.spec SRPM URL: https://mattia.fedorapeople.org/phd2-2.6.4-3.fc28.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #6 from Richard Shaw hobbes1069@gmail.com --- Ideally you would not add the -std flag if it's not needed but that would take a lot more logic and is something cmake should be able to handle automatically. I may post to their mailing list to ask about it, but your fix is sufficient.
*** APPROVED ***
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
--- Comment #8 from Gwyn Ciesla limburgher@gmail.com --- (fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/phd2. You may commit to the branch "f27" in about 10 minutes.
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
--- Comment #7 from Mattia Verga mattia.verga@email.it --- Thank you for the review. Request sent.
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
--- Comment #9 from Fedora Update System updates@fedoraproject.org --- phd2-2.6.4-3.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2017-e7916749ca
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #10 from Fedora Update System updates@fedoraproject.org --- phd2-2.6.4-3.fc27 has been pushed to the Fedora 27 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-e7916749ca
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
--- Comment #11 from Fedora Update System updates@fedoraproject.org --- phd2-2.6.4-3.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1509034
Mattia Verga mattia.verga@email.it changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |CURRENTRELEASE Last Closed| |2017-12-18 05:32:34
--- Comment #12 from Mattia Verga mattia.verga@email.it --- Strangely this wasn't automatically closed...
package-review@lists.fedoraproject.org