Bug 2314746 - Review Request: gdu - Fast disk usage analyzer with console interface written in Go
Summary: Review Request: gdu - Fast disk usage analyzer with console interface written...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Mikel Olasagasti Uranga
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2024-09-25 18:59 UTC by Alexey Lunev
Modified: 2024-11-10 17:38 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-11-10 17:38:34 UTC
Type: ---
Embargoed:
mikel: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 8071542 to 8091455 (1.22 KB, patch)
2024-09-30 05:59 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 8091455 to 8102935 (7.42 KB, patch)
2024-10-02 16:16 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 8102935 to 8105286 (1.10 KB, patch)
2024-10-03 12:11 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 8105286 to 8180474 (5.71 KB, patch)
2024-10-27 05:15 UTC, Fedora Review Service
no flags Details | Diff

Description Alexey Lunev 2024-09-25 18:59:35 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08071436/gdu.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08071436/gdu-5.29.0-1.fc40.src.rpm
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

Comment 1 Fedora Review Service 2024-09-25 19:06:13 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8071542
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314746-gdu/fedora-rawhide-x86_64/08071542-gdu/fedora-review/review.txt

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.

Comment 3 Fedora Review Service 2024-09-30 05:59:12 UTC
Created attachment 2049471 [details]
The .spec file difference from Copr build 8071542 to 8091455

Comment 4 Fedora Review Service 2024-09-30 05:59:14 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8091455
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314746-gdu/fedora-rawhide-x86_64/08091455-gdu/fedora-review/review.txt

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.

Comment 5 Mikel Olasagasti Uranga 2024-10-02 11:42:07 UTC
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.

Comment 6 Alexey Lunev 2024-10-02 11:51:58 UTC
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.

Comment 8 Fedora Review Service 2024-10-02 16:16:51 UTC
Created attachment 2050069 [details]
The .spec file difference from Copr build 8091455 to 8102935

Comment 9 Fedora Review Service 2024-10-02 16:16:53 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8102935
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314746-gdu/fedora-rawhide-x86_64/08102935-gdu/fedora-review/review.txt

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.

Comment 10 Mikel Olasagasti Uranga 2024-10-03 11:18:19 UTC
> Source:         https://github.com/dundee/gdu/releases/download/v%{version}/gdu-%{version}.tgz

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*

Comment 11 Alexey Lunev 2024-10-03 12:05:02 UTC
> 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_64/08105264-gdu/gdu.spec
New SRPM URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/fedora-40-x86_64/08105264-gdu/gdu-5.29.0-1.fc40.src.rpm
Link to successful copr build: https://copr.fedorainfracloud.org/coprs/xbt573/gdu/build/8105264/

Comment 12 Fedora Review Service 2024-10-03 12:11:13 UTC
Created attachment 2050231 [details]
The .spec file difference from Copr build 8102935 to 8105286

Comment 13 Fedora Review Service 2024-10-03 12:11:16 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8105286
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314746-gdu/fedora-rawhide-x86_64/08105286-gdu/fedora-review/review.txt

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.

Comment 14 Alexey Lunev 2024-10-07 19:48:49 UTC
whoops, another fix (`gdu.1.*` -> `gdu.1*`)
New spec URL: https://copr-dist-git.fedorainfracloud.org/cgit/xbt573/gdu/gdu.git/plain/gdu.spec?h=f40
Looks like copr don't want to upload my SRPM (maybe because it's pretty much identical) ¯\_(ツ)_/¯

Comment 15 Mikel Olasagasti Uranga 2024-10-21 21:36:18 UTC
> 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?

Comment 16 Alexey Lunev 2024-10-27 05:10:25 UTC
> > 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/08180427/gdu.spec
New SRPM URL: https://download.copr.fedorainfracloud.org/results/xbt573/gdu/srpm-builds/08180427/gdu-5.29.0-1.src.rpm
Link to successful copr build: https://copr.fedorainfracloud.org/coprs/xbt573/gdu/build/8180427/

Comment 17 Fedora Review Service 2024-10-27 05:15:53 UTC
Created attachment 2053927 [details]
The .spec file difference from Copr build 8105286 to 8180474

Comment 18 Fedora Review Service 2024-10-27 05:15:55 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8180474
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314746-gdu/fedora-rawhide-x86_64/08180474-gdu/fedora-review/review.txt

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.

Comment 19 Mikel Olasagasti Uranga 2024-10-27 08:57:17 UTC
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.

Comment 20 Alexey Lunev 2024-10-27 11:14:32 UTC
> 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.spec?h=f40&id=79b1503e94c8a57f8cd7bba13eb0326eb6631879 (copr didn't upload SRPM again)
Link to successful copr build: https://copr.fedorainfracloud.org/coprs/xbt573/gdu/build/8180739

Comment 21 Mikel Olasagasti Uranga 2024-10-27 21:11:07 UTC
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!

Comment 22 Fedora Review Service 2024-10-27 21:11:36 UTC
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

Comment 23 Fedora Admin user for bugzilla script actions 2024-11-10 16:51:37 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/gdu

Comment 24 Fedora Update System 2024-11-10 17:17:21 UTC
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

Comment 25 Fedora Update System 2024-11-10 17:38:34 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.