Bug 1409780 - Review Request: golang-github-hanwen-go-fuse - FUSE bindings for Go
Summary: Review Request: golang-github-hanwen-go-fuse - FUSE bindings for Go
Keywords:
Status: CLOSED DUPLICATE of bug 1712280
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2017-01-03 11:02 UTC by Christophe Delaere
Modified: 2019-09-03 06:36 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-09-03 06:36:17 UTC
Type: ---


Attachments (Terms of Use)

Description Christophe Delaere 2017-01-03 11:02:08 UTC
Spec URL: http://cp3.irmp.ucl.ac.be/~delaere/documents/golang-github-hanwen-go-fuse.spec
SRPM URL: http://cp3.irmp.ucl.ac.be/~delaere/documents/golang-github-hanwen-go-fuse.spec
Description: FUSE bindings for Go.
Fedora Account System Username: delaere

Hi, this is my first submission for a package for Fedora, so I am seeking for a sponsor.
This package provides FUSE bindings for the Go language. I contacted the upstream maintainer and he is fine with my proposal.

To prepare the spec file for this package, I started from the one of go-sqlite3.

You can find the compiled package in my copr repository there:
https://copr.fedorainfracloud.org/coprs/delaere/TMSU/

The koji build is there:
https://koji.fedoraproject.org/koji/taskinfo?taskID=16958559


Best,
Christophe.

Comment 1 Michael Schwendt 2017-01-03 14:18:49 UTC
Just some drive-by comments as I won't be able to assist with golang stuff:

"SRPM URL" points at spec file.

One of the first things you may want to do is to point the fedora-review tool at this ticket: fedora-review -b 1409780

It will fetch the latest package files, perform a local build and check the built packages for lots of issues that will be relevant not only during package review. Every package maintainer ought to be aware of that tool and use it, too.

> Version:        0.0.0

If there is no upstream version, this tag contains two superfluous '.0'. A simple

  Version: 0

would achieve the same thing.

Also look up the git snapshot guidelines:
https://fedoraproject.org/wiki/Packaging:Versioning


Less macros in the spec file would be a good thing. Readability of this spec file is unusually low, also due to too many conditional sections that are only for other dists and will require a very careful review (e.g. if you ever turned off subpackage builds, it would be necessary to add proper Obsoletes/Provides tags for the remove packages).

> %global provider        github
> %global provider_tld    com
> %global project         hanwen
> %global repo            go-fuse
> %global provider_prefix %{provider}.%{provider_tld}/%{project}/%{repo}
> URL:            https://%{provider_prefix}
> Source0:        https://%{provider_prefix}/archive/%{commit}/%{repo}-%{shortcommit}.tar.gz

Even the package %name would be affected, if you touched any of these macro definitions,

> Name:           golang-%{provider}-%{project}-%{repo}

but it isn't as easy as that, because there are extra requirements for renaming Fedora packages.


%check section by default didn't test anything. And since your generate %files lists on-the-fly, do you even check for missing files?

Comment 2 Christophe Delaere 2017-01-03 15:01:25 UTC
Thanks for the feedback. I will work on it based on your comments asap.
I can already point you to the srpm... sorry for the wrong cut&paste:

http://cp3.irmp.ucl.ac.be/~delaere/documents/golang-github-hanwen-go-fuse-0.0.0-1.fc25.src.rpm

So far, I followed the wiki:
https://fedoraproject.org/wiki/How_to_create_an_RPM_package

The package passed all tests there (local build, mock build, koji build, rpmlint, and I ran fedora-review with -n). That said, I agree with you about the readability and complexity of the spec file... I was hoping to do the right thing by starting from an already endorsed spec file (I picked randomly  golang-github-mattn-go-sqlite3) but it was maybe not the right choice.

Comment 3 Michael Schwendt 2017-01-03 16:18:00 UTC
fedora-review is not bullet-proof, and human reviewers aren't either. ;)


There seems to be an ongoing effort to get some "Go Packaging Guidelines" through the FPC for several years:

  https://fedorahosted.org/fpc/ticket/382

Skimming over the draft, it seems its makers like the macro overload, and it could be that some of the existing packages apply the same packaging style already. Well, the maintenance burden is on the shoulders of those, who maintain such golang packages.

It could be that other golang packagers love this packaging style.


About the conditional builds, many packagers would prefer %bcond_with/_without over %global constants. It is too much of a risk to switch a global temporarily and not reset it again before committing changes to dist git. You can toggle %bcond_ settings from the outside for test-builds (passing --with/--without foo options to rpmbuild) with no need to modify the spec file.


> $ rpmlint -i golang-github-hanwen-go-fuse-*
> golang-github-hanwen-go-fuse-unit-test.x86_64: E: devel-dependency
> golang-github-hanwen-go-fuse-devel
> Your package has a dependency on a devel package but it's not
> a devel package itself.

Any reason why this -test package is not noarch, too?

Comment 4 Neal Gompa 2017-01-03 16:32:42 UTC
If this package spec was generated by gofed, then it is most likely fully in compliance with how the Golang folks want Go stuff to be packaged.

Comment 5 Ralf Senderek 2017-01-03 16:46:15 UTC
Just a few comments to help you write a more comprehensible spec file:

You have to make yourself familiar with the packaging guidelines:

   https://fedoraproject.org/wiki/Packaging:Guidelines

and the packaging naming guidelines:

   https://fedoraproject.org/wiki/Packaging:Naming?rd=Packaging:NamingGuidelines

I'd recommend to go through you package's spec file and also through the list
of files your package would contain, and ask yourself if everything complies
with these two documents.

Then I'd think that you'd get rid of all unnecessary global variables and 
most of the conditional statements in your spec file. Please start to write
comments above a conditional that clearly states why you need it, and if
you cannot come up with a convincing reason get rid of it. Some - if not most
things - will be done unconditionally in a spec file.

Try to find out what level of complexity is necessary for your packaging project
and start building sub-packages only if there is a reason for it. It's easier to
convince a sponsor to help you get your package approved if you can show that
you understand why you introduced a complexity into your spec file instead of
starting from an unnecessary complex template that does not fit your project.

Keep updating your spec file and I wish you the best of luck to find a sponsor.

Comment 6 Christophe Delaere 2017-01-09 10:01:26 UTC
Thanks a lot for your feedback. I tried to take all that into account.

I prepared an updated version based on a freshly-generated gofed spec file that I skimmed down and documented as much as possible. I believe that the result is readable and sticks to the guidelines.

The only caveat is that the automated tests in the check section cannot run with mock because of the need for a functional fuse system. It is therefore off by default using %bcond_with.

Spec URL: http://cp3.irmp.ucl.ac.be/~delaere/documents/golang-github-hanwen-go-fuse.spec
SRPM URL: http://cp3.irmp.ucl.ac.be/~delaere/documents/golang-github-hanwen-go-fuse-0-1.git0ad840c.fc25.src.rpm
Description: Native bindings for the FUSE kernel module
Fedora Account System Username: delaere

The koji build is there:
https://koji.fedoraproject.org/koji/taskinfo?taskID=17178588

You can also find the compiled package in my copr repository there:
https://copr.fedorainfracloud.org/coprs/delaere/TMSU/

Note that I am still seeking for a sponsor, as this is my first package.

Comment 7 Michael Schwendt 2017-01-12 02:35:13 UTC
> Version:        0
> Release:        1.git%{shortcommit}%{?dist}

Now that you've changed "Version" to something more sane and have added a different git snapshot, you really want to skim over the snapshot versioning guidelines as mentioned before:

  https://fedoraproject.org/wiki/Packaging:Versioning


> Group:          Development/Libraries

https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections


> %description
> %{summary}

> %package devel
> Summary:       %{summary}

> %description devel
> %{summary}

Hmmm? Same %summary in all these places? That's odd, isn't it?

Comment 8 Elliott Sales de Andrade 2019-09-03 06:36:17 UTC

*** This bug has been marked as a duplicate of bug 1712280 ***


Note You need to log in before you can comment on or make changes to this bug.