[Bug 1431748] New: Review Request: golang-github-cznic-ql - Embedded SQL database written in Go
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Bug ID: 1431748 Summary: Review Request: golang-github-cznic-ql - Embedded SQL database written in Go Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: decathorpe@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
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....
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}.
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |1431587, 1431736, 1431741, | |1431745
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1431587 [Bug 1431587] Review Request: golang-github-cznic-mathutil - Supplemental utilities for Go's rand and math packages https://bugzilla.redhat.com/show_bug.cgi?id=1431736 [Bug 1431736] Review Request: golang-github-cznic-strutil - Supplemental utilities for Go's strings package https://bugzilla.redhat.com/show_bug.cgi?id=1431741 [Bug 1431741] Review Request: golang-github-cznic-b - B+ Tree implementation in Go https://bugzilla.redhat.com/show_bug.cgi?id=1431745 [Bug 1431745] Review Request: golang-github-cznic-lldb - Low-level database engine implementation in Go
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1427634
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1427634 [Bug 1427634] Review Request: syncthing - Continuous File Synchronization
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |1246526
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1246526 [Bug 1246526] Review Request: golang-github-cznic-mathutil - Utilities supplemental to the Go standard "rand" and "math" packages
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1431587, which changed state.
Bug 1431587 Summary: Review Request: golang-github-cznic-mathutil - Supplemental utilities for Go's rand and math packages https://bugzilla.redhat.com/show_bug.cgi?id=1431587
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |DUPLICATE
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1431587, which changed state.
Bug 1431587 Summary: Review Request: golang-github-cznic-mathutil - Supplemental utilities for Go's rand and math packages https://bugzilla.redhat.com/show_bug.cgi?id=1431587
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |ASSIGNED Resolution|DUPLICATE |---
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1246526, which changed state.
Bug 1246526 Summary: Review Request: golang-github-cznic-mathutil - Utilities supplemental to the Go standard "rand" and "math" packages https://bugzilla.redhat.com/show_bug.cgi?id=1246526
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |NOTABUG
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1431587, which changed state.
Bug 1431587 Summary: Review Request: golang-github-cznic-mathutil - Supplemental utilities for Go's rand and math packages https://bugzilla.redhat.com/show_bug.cgi?id=1431587
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1431736, which changed state.
Bug 1431736 Summary: Review Request: golang-github-cznic-strutil - Supplemental utilities for Go's strings package https://bugzilla.redhat.com/show_bug.cgi?id=1431736
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1431741, which changed state.
Bug 1431741 Summary: Review Request: golang-github-cznic-b - B+ Tree implementation in Go https://bugzilla.redhat.com/show_bug.cgi?id=1431741
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #1 from Fabio Valentini decathorpe@gmail.com --- 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....
A successful build when all dependencies are present can be viewed at https://copr.fedorainfracloud.org/coprs/decathorpe/golang-staging/build/5486....
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #2 from Fabio Valentini decathorpe@gmail.com --- 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/5508...
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #3 from Fabio Valentini decathorpe@gmail.com --- 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....
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #4 from Fabio Valentini decathorpe@gmail.com --- 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....
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |1465885
--- Comment #5 from Fabio Valentini decathorpe@gmail.com --- 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....
This also introduces some new dependencies (marked as blockers).
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1465885 [Bug 1465885] Review Request: golang-github-cznic-golex - Lex/Flex-like utility written in Go
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |athoscribeiro@gmail.com Assignee|nobody@fedoraproject.org |athoscribeiro@gmail.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1431745, which changed state.
Bug 1431745 Summary: Review Request: golang-github-cznic-lldb - Low-level database engine implementation in Go https://bugzilla.redhat.com/show_bug.cgi?id=1431745
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |RAWHIDE
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1431745, which changed state.
Bug 1431745 Summary: Review Request: golang-github-cznic-lldb - Low-level database engine implementation in Go https://bugzilla.redhat.com/show_bug.cgi?id=1431745
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |ON_QA Resolution|RAWHIDE |---
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1431745, which changed state.
Bug 1431745 Summary: Review Request: golang-github-cznic-lldb - Low-level database engine implementation in Go https://bugzilla.redhat.com/show_bug.cgi?id=1431745
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #6 from Athos Ribeiro athoscribeiro@gmail.com --- 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_versionin... [3] https://fedoraproject.org/wiki/Licensing:Main#Good_Licenses [4] https://fedoraproject.org/wiki/Packaging:Guidelines#Bundling_and_Duplication... [5] https://fedoraproject.org/wiki/PackagingDrafts/Go#Bundled_or_de-bundled
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #7 from Fabio Valentini decathorpe@gmail.com ---
- 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?
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #8 from Fabio Valentini decathorpe@gmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #9 from Fabio Valentini decathorpe@gmail.com --- 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....
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 ...
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #10 from Fabio Valentini decathorpe@gmail.com --- 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).
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Athos Ribeiro athoscribeiro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #11 from Athos Ribeiro athoscribeiro@gmail.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #12 from Fabio Valentini decathorpe@gmail.com --- 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}).
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #13 from Athos Ribeiro athoscribeiro@gmail.com --- (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 ?
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #15 from Gwyn Ciesla limburgher@gmail.com --- Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/golang-github-cznic-ql
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #14 from Fabio Valentini decathorpe@gmail.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=1431748 Bug 1431748 depends on bug 1465885, which changed state.
Bug 1465885 Summary: Review Request: golang-github-cznic-golex - Lex/Flex-like utility written in Go https://bugzilla.redhat.com/show_bug.cgi?id=1465885
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |RAWHIDE Last Closed| |2017-08-05 15:36:16
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|RAWHIDE |ERRATA
--- Comment #19 from Fedora Update System updates@fedoraproject.org --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=1431748
--- Comment #20 from Fedora Update System updates@fedoraproject.org --- 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.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com