Bug 1431748

Summary: Review Request: golang-github-cznic-ql - Embedded SQL database written in Go
Product: [Fedora] Fedora Reporter: Fabio Valentini <decathorpe>
Component: Package ReviewAssignee: Athos Ribeiro <athoscribeiro>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: athoscribeiro, package-review
Target Milestone: ---Flags: athoscribeiro: 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: 2017-08-05 19:36:16 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: 1246526, 1431587, 1431736, 1431741, 1431745, 1465885    
Bug Blocks: 1427634    

Description Fabio Valentini 2017-03-13 16:59:22 UTC
Spec URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql.spec

SRPM URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql-1.1.0-1.0.gitbd2055c.fc25.src.rpm

Description: Embedded SQL database written in Go

Fedora Account System Username: decathorpe


This package is one of the (indirect) dependencies of syncthing. I can't provide a koji scratch build yet, since it depends on golang-github-cznic-{mathutil,strutil,b,lldb}.

Comment 1 Fabio Valentini 2017-05-07 09:54:41 UTC
I've updated the .spec for the latest commit in git master of the project, and also incorporated changes for some changed packaging guidelines (snapshot date must be present in the "Release" string for snapshots).

Spec URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql.spec

SRPM URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql-1.1.0-1.20170505.git26248ea.fc26.src.rpm


A successful build when all dependencies are present can be viewed at https://copr.fedorainfracloud.org/coprs/decathorpe/golang-staging/build/548651/.

Comment 2 Fabio Valentini 2017-05-11 21:38:53 UTC
I've also enabled building the CLI tool accompanying this go package. A successful build (with the binary and a -debuginfo package) can be found at [1].

[1]: https://copr.fedorainfracloud.org/coprs/decathorpe/golang-staging/build/550861/

Comment 3 Fabio Valentini 2017-05-13 10:49:08 UTC
Updated files for latest state of git master (as of May 13, 2017):

Spec URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql.spec

SRPM URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql-1.1.0-1.20170512.gitfa0b368.fc26.src.rpm

Comment 4 Fabio Valentini 2017-05-20 11:59:32 UTC
Updated files for latest git master (commit f39e59d; May 17, 2017):

Spec URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql.spec

SRPM URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql-1.1.0-1.20170517.gitf39e59d.fc26.src.rpm

Comment 5 Fabio Valentini 2017-06-28 12:22:21 UTC
Updated .spec and SRPM files for latest git master (commit ba9eea9; May 22, 2017):

Spec URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql.spec

SRPM URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql-1.1.0-1.20170522.gitba9eea9.fc26.src.rpm


This also introduces some new dependencies (marked as blockers).

Comment 6 Athos Ribeiro 2017-07-25 14:58:48 UTC
Hi Fabio, here we go:

- The Spec file differs from the one in the SRPM.

- As per the golang packaging guidelines draft, it would be nice to rename the package or Provide the application name (ql) [1].

- License and documentation files should also be shipped with the main package.

- The License tag for Apache Software License 2.0 is 'ASL 2.0' [3]. The ASL 2.0 licese text should also be shipped if you are bundling the go4/lock. Although the best thing to do here would be to find a way to un-bundle the go4/lock from the project (there is only one file importing it - file.go) and remove the ASL 2.0 from the License tag. Maybe you could talk to upstram about it (but it does not seem it would be that hard to patch it)... See [4] and [5] for reference.

- When you bump a post-release vcs revision, you should also bump the package release (see the spec changelog) [2].

[1] https://fedoraproject.org/wiki/PackagingDrafts/Go#Packaging_Binaries
[2] https://fedoraproject.org/wiki/Package_Versioning_Examples#Complex_versioning_examples
[3] https://fedoraproject.org/wiki/Licensing:Main#Good_Licenses
[4] https://fedoraproject.org/wiki/Packaging:Guidelines#Bundling_and_Duplication_of_system_libraries
[5] https://fedoraproject.org/wiki/PackagingDrafts/Go#Bundled_or_de-bundled

Comment 7 Fabio Valentini 2017-07-25 15:36:19 UTC
> - The Spec file differs from the one in the SRPM.

I don't know how this happened, but it should be fixed now.

> - As per the golang packaging guidelines draft, it would be nice to rename the package or Provide the application name (ql) [1].

I've added a "Provides: ql" to the package (nothing else provides it - I checked).

> - License and documentation files should also be shipped with the main package.

You're right, of course. It's fixed now.

> - The License tag for Apache Software License 2.0 is 'ASL 2.0' [3]. The ASL 2.0 licese text should also be shipped if you are bundling the go4/lock. Although the best thing to do here would be to find a way to un-bundle the go4/lock from the project (there is only one file importing it - file.go) and remove the ASL 2.0 from the License tag. Maybe you could talk to upstram about it (but it does not seem it would be that hard to patch it)... See [4] and [5] for reference.

- The License tag is fixed.
- The License file is now included as LICENSE.camlistore-go4-lock
- I've tried de-bundling the go4 package, but it's just not possible for me to do that (countless new dependencies, and the source repo has moved from github to a third-party site). Additionally, they state this in their project's README [1]:

> - no backwards compatibility. go4 makes no backwards compatibility promises. If you want to use go4, vendor it.

[1]: https://github.com/camlistore/go4/blob/master/README.md

I added "Provides: bundled(github.com/camlistore/go4/lock)" to the -devel subpackage, since that's the only sensible course of action I see.

> - When you bump a post-release vcs revision, you should also bump the package release (see the spec changelog) [2].

Those guidelines are stupid. Why make the (always incrementing) date the first field of the VCS info, if I have to bump the "release" number regardless?

Comment 8 Fabio Valentini 2017-07-25 15:38:29 UTC
Damn. Sorry, I accidentally pushed the "Save changes" button before I was finished ... I'll update the bug report as soon as the updated .spec and SRPM files are ready.

Comment 9 Fabio Valentini 2017-07-25 16:02:38 UTC
Spec URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql.spec

SRPM URL: https://decathorpe.fedorapeople.org/packages/golang-github-cznic-ql-1.1.0-1.20170522.gitba9eea9.fc26.src.rpm

koji scratch build for rawhide (ppc64 failing as expected, with no more golang on ppc64, and scratch builds somehow ignoring the ExclusiveArch tag):
https://koji.fedoraproject.org/koji/taskinfo?taskID=20725339

If you consider it a blocker, I'll abide by the stupid snapshot versioning guidelines and bump the release tag for version updates ...

Comment 10 Fabio Valentini 2017-07-31 13:12:05 UTC
The previously missing dependencies have now been pushed to the rawhide repositories and to the mirrors (which took some time due to the mass rebuild).

Comment 11 Athos Ribeiro 2017-07-31 17:36:19 UTC
Is there any reason for the second license file not being under %license?

Package looks good. Approved!

As part of the review swap email, if you can, it would be great to get hugo review: https://bugzilla.redhat.com/show_bug.cgi?id=1426972

Comment 12 Fabio Valentini 2017-07-31 18:10:12 UTC
Thank you for the review! I'll have a look at hugo.

> Is there any reason for the second license file not being under %license?

Yes. Writing "%license LICENSE" and "%license foo/bar/LICENSE" will result in the second LICENSE file overwriting the first one, since they are copied to the same location (%{_licensedir}/%{name}).

Comment 13 Athos Ribeiro 2017-07-31 18:24:30 UTC
(In reply to Fabio Valentini from comment #12)
> Thank you for the review! I'll have a look at hugo.
> 
> > Is there any reason for the second license file not being under %license?
> 
> Yes. Writing "%license LICENSE" and "%license foo/bar/LICENSE" will result
> in the second LICENSE file overwriting the first one, since they are copied
> to the same location (%{_licensedir}/%{name}).

Oh, I didn't now that. Will it happen even in this case, where the second license name is LICENSE.sometring ?

Comment 14 Fabio Valentini 2017-07-31 19:01:25 UTC
Well, in this case, I explicitly copied the file to a *different* filename myself, because both files are named "LICENSE" in the tarball, which would generate a conflict.

Using something like
 %license LICENSE
 %license foo/bar/LICENSE-foobar
should work just fine though.

Comment 15 Gwyn Ciesla 2017-07-31 21:56:14 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/golang-github-cznic-ql

Comment 16 Fedora Update System 2017-08-01 09:26:03 UTC
golang-github-cznic-ql-1.1.0-1.20170522.gitba9eea9.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-5d0f679b95

Comment 17 Fedora Update System 2017-08-01 23:49:33 UTC
golang-github-cznic-ql-1.1.0-1.20170522.gitba9eea9.fc25 has been pushed to the Fedora 25 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-1cde795c7a

Comment 18 Fedora Update System 2017-08-02 01:53:04 UTC
golang-github-cznic-ql-1.1.0-1.20170522.gitba9eea9.fc26 has been pushed to the Fedora 26 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-5d0f679b95

Comment 19 Fedora Update System 2017-08-09 15:59:02 UTC
golang-github-cznic-ql-1.1.0-1.20170522.gitba9eea9.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report.

Comment 20 Fedora Update System 2017-08-09 19:59:12 UTC
golang-github-cznic-ql-1.1.0-1.20170522.gitba9eea9.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.