Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.

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.