Bug 2053969

Summary: Review Request: gopass - The slightly more awesome standard unix password manager for teams
Product: [Fedora] Fedora Reporter: Fabio Alessandro Locati <me>
Component: Package ReviewAssignee: Mikel Olasagasti Uranga <mikel>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: mikel, package-review
Target Milestone: ---Flags: mikel: 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: 2022-04-07 15:26:02 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: 2035289, 2053960, 2053961, 2053962, 2053963, 2053964, 2053965, 2053966, 2053968, 2065054    
Bug Blocks:    

Description Fabio Alessandro Locati 2022-02-13 16:29:07 UTC
Spec URL: https://fale.fedorapeople.org/golang-github-gopasspw-gopass/golang-github-gopasspw-gopass.spec
SRPM URL: https://fale.fedorapeople.org/golang-github-gopasspw-gopass/golang-github-gopasspw-gopass-1.13.1-1.fc35.src.rpm
Description: The slightly more awesome standard unix password manager for teams
COPR build: http://copr.fedorainfracloud.org/coprs/fale/gopass/build/3486927/
Fedora Account System Username: fale

Comment 1 Mikel Olasagasti Uranga 2022-03-15 11:43:48 UTC
As gopass is a known app, it makes sense to rename golang-github-gopasspw-gopass.spec to gopass.spec, as done with doctl for example. You'll need to rename the BZ title accordingly.

- Define "%global goname gopass" as in https://src.fedoraproject.org/rpms/doctl/blob/rawhide/f/doctl.spec#_15

>for cmd in helpers/postrel pkg/pwgen/pwrules helpers/man helpers/changelog helpers/release; do
>  %gobuild -o %{gobuilddir}/bin/$(basename $cmd) %{goipath}/$cmd
>done

These are not required for gopass usage, can be removed.

Comment 2 Fabio Alessandro Locati 2022-03-16 23:11:13 UTC
Hi Mikel,

Looking doctl, I notice that the "master" package is also named that way. Should I close this request and create a new one with the right name as well?

Thanks a lot,
Fale

Comment 3 Mikel Olasagasti Uranga 2022-03-16 23:24:46 UTC
> Should I close this request and create a new one with the right name as well?

You can just rename the BZ title and upload a new spec/srpm with the new name for review.

Comment 5 Mikel Olasagasti Uranga 2022-03-17 10:52:25 UTC
As mentioned, the auxiliary commands are not required for gopass (not packaged in the official binary release for example), so unless you've a good reason they can be dropped. You should remove this parts of the spec:

> %build
> %gobuild -o %{gobuilddir}/bin/gopass %{goipath}
> for cmd in helpers/postrel pkg/pwgen/pwrules helpers/man helpers/changelog helpers/release; do
>   %gobuild -o %{gobuilddir}/bin/$(basename $cmd) %{goipath}/$cmd
> done

(...)

> install -m 0755 -vd                     %{buildroot}%{_bindir}
> install -m 0755 -vp %{gobuilddir}/bin/* %{buildroot}%{_bindir}/

(...)

> %files
> %license LICENSE
> %doc docs CONTRIBUTING.md GOVERNANCE.md README.md ARCHITECTURE.md CHANGELOG.md
> %{_bindir}/*

Comment 6 Fabio Alessandro Locati 2022-03-17 11:50:45 UTC
Sorry for that, I made a little bit of mess on my filesystem while renaming files and folders.

SPEC: https://fale.fedorapeople.org/gopass/gopass.spec
SRPM: https://fale.fedorapeople.org/gopass/gopass-1.13.1-1.fc35.src.rpm
COPR: https://copr.fedorainfracloud.org/coprs/fale/gopass/build/3730796/

Thanks a lot Mikel for your suggestions and patience :-)

Comment 7 Mikel Olasagasti Uranga 2022-03-17 12:36:31 UTC
Sorry, my previous comment was not correct, as you still need gopass binary of course. Anyhow, you did the correct thing, sorry again about that.

I just realized that check section is being ignored, you should use `%bcond_without check`

Comment 8 Mikel Olasagasti Uranga 2022-03-17 16:33:14 UTC
I did some changes and upload a new spec here: https://mikel.olasagasti.info/tmp/fedora/gopass.spec

- Enable tests
- Add BuildRequires for tests
- Add Requires
- Generate bash/fish/zsh completions. For some reason bash ones seem not to work
- Test TestFind fails, I added a skip for it
- Other tests, internal/backend/storage/fs/link_test.go pkg/appdir/appdir_xdg_test.go pkg/pwgen/validate_test.go pkg/termio/identity_test.go, fail with the following error, but I was not able to trace it correctly:

> code in directory /usr/share/gocode/src/gotest.tools/assert expects import "gotest.tools/v3/assert"

Check these changes and adapt them to your spec.

Comment 9 Fabio Alessandro Locati 2022-03-17 17:12:32 UTC
No problem and a huge thankyou for your SPEC file which allowed me to greatly improve the tests and SH completion :-).

SPEC: https://fale.fedorapeople.org/gopass/gopass.spec
SRPM: https://fale.fedorapeople.org/gopass/gopass-1.13.1-1.fc35.src.rpm
COPR: https://copr.fedorainfracloud.org/coprs/fale/gopass/build/3734378/

Comment 10 Mikel Olasagasti Uranga 2022-03-17 18:13:10 UTC
> Requires:       git
> Requires:       gnupg
> BuildRequires:  git-core
> BuildRequires:  gnupg2

I would use the same Requires as in BuildRequires.

> #rm internal/backend/storage/fs/link_test.go pkg/appdir/appdir_xdg_test.go pkg/pwgen/validate_test.go pkg/termio/identity_test.go

Remove this leftover.

After these changes I think package can be approved.

Comment 12 Mikel Olasagasti Uranga 2022-03-17 21:15:26 UTC
go2rpm package with some extras, fedora-review is correct:

- The specfile is sane
- License is correct
- Builds successfully in mock
- No rpmlint errors
- %check section passes
- The latest version is packaged
- The package complies with the Packaging Guidelines

Package approved! On import, don't forget to do the following:

- Add package to release-monitoring.org
- Add package to Koschei
- Give go-sig privileges on package
- Close the review bug by referencing it in the rpm changelog and/or the Bodhi ticket

Comment 13 Gwyn Ciesla 2022-03-17 21:31:01 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/gopass

Comment 14 Fedora Update System 2022-03-30 09:28:06 UTC
FEDORA-2022-4506cf983b has been submitted as an update to Fedora 35. https://bodhi.fedoraproject.org/updates/FEDORA-2022-4506cf983b

Comment 15 Fedora Update System 2022-03-30 09:28:07 UTC
FEDORA-2022-9036145a7b has been submitted as an update to Fedora 36. https://bodhi.fedoraproject.org/updates/FEDORA-2022-9036145a7b

Comment 16 Fedora Update System 2022-03-31 02:17:40 UTC
FEDORA-2022-4506cf983b has been pushed to the Fedora 35 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2022-4506cf983b \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-4506cf983b

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 17 Fedora Update System 2022-03-31 18:24:38 UTC
FEDORA-2022-9036145a7b has been pushed to the Fedora 36 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2022-9036145a7b \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-9036145a7b

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 18 Fedora Update System 2022-04-07 15:26:02 UTC
FEDORA-2022-4506cf983b has been pushed to the Fedora 35 stable repository.
If problem still persists, please make note of it in this bug report.