https://bugzilla.redhat.com/show_bug.cgi?id=2314746
Bug ID: 2314746 Summary: Review Request: gdu - Fast disk usage analyzer with console interface written in Go Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: cheembox573@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08... SRPM URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08... Link to successful copr build: https://copr.fedorainfracloud.org/coprs/xbt573/gdu/build/8071436/ Description: Pretty fast disk usage analyzer written in Go. Gdu is intended primarily for SSD disks where it can fully utilize parallel processing. However HDDs work as well, but the performance gain is not so huge. Fedora Account System Username: xbt573
This is my first package and i would like to be sponsored into the packager group
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #1 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8071542 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- Not a valid SPDX expression 'MIT AND Apache-2.0 AND BSD-3-Clause AND BSD-2-Clause AND ISC and (BSD-3-Clause AND Apache-2.0 AND MIT) and (MIT AND Apache-2.0)'. Read more: https://fedoraproject.org/wiki/Changes/SPDX_Licenses_Phase_1
Please know that there can be false-positives.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #2 from Alexey Lunev cheembox573@gmail.com --- New spec URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08... New SRPM URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08... New link to successful copr build: https://copr.fedorainfracloud.org/coprs/xbt573/gdu/build/8089555/
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #3 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2049471 --> https://bugzilla.redhat.com/attachment.cgi?id=2049471&action=edit The .spec file difference from Copr build 8071542 to 8091455
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |AutomationTriaged
--- Comment #4 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8091455 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
Mikel Olasagasti Uranga mikel@olasagasti.info changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |mikel@olasagasti.info Status|NEW |ASSIGNED CC| |mikel@olasagasti.info Doc Type|--- |If docs needed, set a value
--- Comment #5 from Mikel Olasagasti Uranga mikel@olasagasti.info --- Do you plan to build also for EPEL or would be limited to Fedora?
I'm asking because I'll suggest to use `go2rpm` to use the common template and, if it will be focused just for Fedora, then using `go2rpm -q -p vendor` to use `go-vendor-tools` underneath to take care of the vendoring part for you. The tool is not yet ready for EPEL9, so if it would be just for Fedora it would be preferable to use that method. Check https://fedora.gitlab.io/sigs/go/go-vendor-tools/ for more info.
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #6 from Alexey Lunev cheembox573@gmail.com --- I plan to also build gdu for EPEL, it will be very useful there. I will try to use `go2rpm` common template and update review when spec will be ready and tested.
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #7 from Alexey Lunev cheembox573@gmail.com --- I reworked this to use `go2rpm` common template New spec URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08... New SRPM URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08... Link to copr build: https://copr.fedorainfracloud.org/coprs/xbt573/gdu/build/8102880/ (successful build except RHEL)
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #8 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2050069 --> https://bugzilla.redhat.com/attachment.cgi?id=2050069&action=edit The .spec file difference from Copr build 8091455 to 8102935
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #9 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8102935 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #10 from Mikel Olasagasti Uranga mikel@olasagasti.info ---
Source: https://github.com/dundee/gdu/releases/download/v%%7Bversion%7D/gdu-%%7Bvers...
Is there any issue with using "Source: %{gosource}" ?
for cmd in cmd/* ; do %gobuild -o %{gobuilddir}/bin/$(basename $cmd) %{goipath}/$cmd done
Optionally can be replaced with
%gobuild -o %{gobuilddir}/bin/%{name} %{goipath}/cmd/%{name}
export GOPATH=$(pwd)/_build:%{gopath} # all ignored tests fails because of root user %gotest %{goipath}/cmd/gdu/app || : %gotest %{goipath}/internal/common %gotest %{goipath}/pkg/analyze || : %gotest %{goipath}/pkg/device %gotest %{goipath}/pkg/path %gotest %{goipath}/pkg/remove || : %gotest %{goipath}/report %gotest %{goipath}/stdout || : %gotest %{goipath}/tui || :
You can use -d or -t to skip certain directories or dir-tree in case tests are failing or require credentials, special conditions, etc.
If a single test is failing, you can try to skip it using the method used in `doctl` pacakge for example (using awk).
%{_mandir}/man1/gdu*
should be:
%{_mandir}/man1/gdu.1*
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #11 from Alexey Lunev cheembox573@gmail.com ---
Is there any issue with using "Source: %{gosource}" ?
Official tarball has vendored dependencies for gdu to build
If a single test is failing, you can try to skip it using the method used in `doctl` pacakge for example (using awk).
I will skip directories fully for now, i will work with upstream to make all tests pass with `root` user.
New spec URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/fedora-40-x86_... New SRPM URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/fedora-40-x86_... Link to successful copr build: https://copr.fedorainfracloud.org/coprs/xbt573/gdu/build/8105264/
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #12 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2050231 --> https://bugzilla.redhat.com/attachment.cgi?id=2050231&action=edit The .spec file difference from Copr build 8102935 to 8105286
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #13 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8105286 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #14 from Alexey Lunev cheembox573@gmail.com --- whoops, another fix (`gdu.1.*` -> `gdu.1*`) New spec URL: https://copr-dist-git.fedorainfracloud.org/cgit/xbt573/gdu/gdu.git/plain/gdu... Looks like copr don't want to upload my SRPM (maybe because it's pretty much identical) ¯_(ツ)_/¯
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #15 from Mikel Olasagasti Uranga mikel@olasagasti.info ---
install -m 0755 -vp gdu.1 %{buildroot}%{_mandir}/man1/
Should be 0644
export GOPATH=$(pwd)/_build:%{gopath}
Is this required?
%doc README.md gdu.1.md
%{_bindir}/gdu
Pure cosmetic thing, but can you remove the empty line between %doc and %{_bindir}?
%global godocs docs INSTALL.md README.md gdu.1.md (...) %doc README.md gdu.1.md
- Does it make sense to install gdu.1.md when the manpage is created from it? - Does it make sense to ship INSTALL.md? - Why not add docs to %doc?
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #16 from Alexey Lunev cheembox573@gmail.com ---
install -m 0755 -vp gdu.1 %{buildroot}%{_mandir}/man1/
Should be 0644
Fixed. (compressed man page was 0644 anyway)
export GOPATH=$(pwd)/_build:%{gopath}
Is this required?
Remains of individual `%gotest`s, removed.
%doc README.md gdu.1.md
%{_bindir}/gdu
Pure cosmetic thing, but can you remove the empty line between %doc and %{_bindir}?
Sure, removed.
%global godocs docs INSTALL.md README.md gdu.1.md (...) %doc README.md gdu.1.md
- Does it make sense to install gdu.1.md when the manpage is created from it?
Forgot about it, removed `gdu.1.md` from installing.
- Does it make sense to ship INSTALL.md?
No, removed from installing.
- Why not add docs to %doc?
`docs` only has one file, `run-books.md`, and it is hints for release process (useful only for developers). I currently removed `docs` from installing anywhere in spec file, tell me if this folder should be packaged.
New spec URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08... New SRPM URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08... Link to successful copr build: https://copr.fedorainfracloud.org/coprs/xbt573/gdu/build/8180427/
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #17 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2053927 --> https://bugzilla.redhat.com/attachment.cgi?id=2053927&action=edit The .spec file difference from Copr build 8105286 to 8180474
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #18 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8180474 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #19 from Mikel Olasagasti Uranga mikel@olasagasti.info --- Thanks for your patience Alexey.
%global goipath github.com/dundee/gdu
Should be github.com/dundee/gdu/v5 as in https://github.com/dundee/gdu/blob/master/go.mod#L1
That would be the last required change from my side.
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #20 from Alexey Lunev cheembox573@gmail.com ---
Thanks for your patience Alexey.
You too! :)
%global goipath github.com/dundee/gdu
Should be github.com/dundee/gdu/v5 as in https://github.com/dundee/gdu/blob/master/go.mod#L1
Fixed.
New spec URL: https://copr-dist-git.fedorainfracloud.org/cgit/xbt573/gdu/gdu.git/plain/gdu... (copr didn't upload SRPM again) Link to successful copr build: https://copr.fedorainfracloud.org/coprs/xbt573/gdu/build/8180739
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
Mikel Olasagasti Uranga mikel@olasagasti.info changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #21 from Mikel Olasagasti Uranga mikel@olasagasti.info --- Golang Package Review ==============
This package was generated using go2rpm, which simplifies the review.
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
- [x] The latest version is packaged or packaging an earlier version is justified. - [x] The License tag reflects the package contents and uses the correct identifiers. - [x] The package builds successfully in mock. - [x] Package is installable (checked by fedora-review). - [x] There are no relevant rpmlint errors. - [x] The package runs tests in %check. - [x] `%goipath` is set correctly. - [x] The package's binaries don't conflict with binaries already in the distribution. (Some Go projects include utility binaries with very generic names) - [x] There are no `%{_bindir}/*` wildcards in %files. (go2rpm includes these by default) - [x] The package does not use `%gometa -f` if it has dependents that still build for %ix86. - [x] The package complies with the Golang and general Packaging Guidelines.
Package approved! On import, don't forget to do the following:
- [ ] Add the package to release-monitoring.org - [ ] Give go-sig privileges (at least commit) on the package - [ ] Close the review bug by referencing its ID in the rpm changelog and the Bodhi ticket. - [ ] Consider configuring Packit service to help with maintenance
Thanks!
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
--- Comment #22 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Hello @xbt573, since this is your first Fedora package, you need to get sponsored by a package sponsor before it can be accepted.
A sponsor is an experienced package maintainer who will guide you through the processes that you will follow and the tools that you will use as a future maintainer. A sponsor will also be there to answer your questions related to packaging.
You can find all active sponsors here: https://docs.pagure.org/fedora-sponsors/
I created a sponsorship request for you: https://pagure.io/packager-sponsors/issue/689 Please take a look and make sure the information is correct.
Thank you, and best of luck on your packaging journey.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RELEASE_PENDING
--- Comment #23 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/gdu
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RELEASE_PENDING |MODIFIED
--- Comment #24 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-11e3474f29 (gdu-5.29.0-1.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2024-11e3474f29
https://bugzilla.redhat.com/show_bug.cgi?id=2314746
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |ERRATA Status|MODIFIED |CLOSED Last Closed| |2024-11-10 17:38:34
--- Comment #25 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-11e3474f29 (gdu-5.29.0-1.fc42) has been pushed to the Fedora 42 stable repository. If problem still persists, please make note of it in this bug report.
package-review@lists.fedoraproject.org