Bug 1431748
Summary: | Review Request: golang-github-cznic-ql - Embedded SQL database written in Go | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Fabio Valentini <decathorpe> |
Component: | Package Review | Assignee: | Athos Ribeiro <athoscribeiro> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | 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
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/. 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/ 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 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 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). 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 > - 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? 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. 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 ... 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). 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 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}).
(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 ? 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. Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/golang-github-cznic-ql 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 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 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 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. 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. |