Bug 1003089 - Review Request: glusterfs-openstack-swift - Gluster for Swift
Review Request: glusterfs-openstack-swift - Gluster for Swift
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Cole Robinson
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 1025059
  Show dependency treegraph
 
Reported: 2013-08-30 14:20 EDT by Luis Pabón
Modified: 2016-11-08 17:25 EST (History)
11 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-04-29 15:43:38 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
besser82: fedora‑review?


Attachments (Terms of Use)

  None (edit)
Description Luis Pabón 2013-08-30 14:20:32 EDT
Spec URL: https://github.com/lpabon/fedora/raw/master/1.8.0/glusterfs-openstack-swift.spec
SRPM URL: https://github.com/lpabon/fedora/raw/master/1.8.0/glusterfs-openstack-swift-1.8.0-7.1.fc19.src.rpm

Description:
Gluster-For-Swift (G4S, pronounced "gee-force") integrates GlusterFS as an
alternative back end for OpenStack Object Storage (Swift) leveraging the
existing front end OpenStack Swift code. Gluster volumes are used to store
objects in files, containers are maintained as top-level directories of volumes,
where accounts are mapped one-to-one to gluster volumes.

Fedora Account System Username:
lpabon
Comment 1 Kaleb KEITHLEY 2013-09-03 09:43:22 EDT
Note that this replaces/obsoletes the glusterfs-ufo subpackage of glusterfs which has broken dependencies in f20 and rawhide.
Comment 2 Matthias Runge 2013-09-03 10:06:38 EDT
Sadly, it doesn't build in mock.

ERROR: Command failed: 
 # ['/usr/bin/yum', '--installroot', '/var/lib/mock/fedora-rawhide-x86_64/root/', 'install', '/home/mrunge/review/1003089-glusterfs-openstack-swift/results/glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch.rpm', '
--setopt=tsflags=nocontexts']
Error: Package: glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch (/glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch)
           Requires: openstack-swift = 1.8.0
           Available: openstack-swift-1.9.0-2.fc20.noarch (fedora)
               openstack-swift = 1.9.0-2.fc20
 You could try using --skip-broken to work around the problem
Error: Package: glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch (/glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch)
           Requires: openstack-swift-container = 1.8.0
           Available: openstack-swift-container-1.9.0-2.fc20.noarch (fedora)
               openstack-swift-container = 1.9.0-2.fc20
Error: Package: glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch (/glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch)
           Requires: openstack-swift-proxy = 1.8.0
           Available: openstack-swift-proxy-1.9.0-2.fc20.noarch (fedora)
               openstack-swift-proxy = 1.9.0-2.fc20
Error: Package: glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch (/glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch)
           Requires: openstack-swift-account = 1.8.0
           Available: openstack-swift-account-1.9.0-2.fc20.noarch (fedora)
               openstack-swift-account = 1.9.0-2.fc20
Error: Package: glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch (/glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch)
           Requires: openstack-swift-object = 1.8.0
           Available: openstack-swift-object-1.9.0-2.fc20.noarch (fedora)
               openstack-swift-object = 1.9.0-2.fc20
 You could try running: rpm -Va --nofiles --nodigest




I did a quick fedora-review -b 1003089
and rpmlint spits out a long list of warnings. 

Checking: glusterfs-openstack-swift-1.8.0-7.1.fc21.noarch.rpm
          glusterfs-openstack-swift-1.8.0-7.1.fc21.src.rpm
glusterfs-openstack-swift.noarch: W: summary-ended-with-dot C GlusterFS Integration with OpenStack Object Storage (Swift).

^^
easy one


glusterfs-openstack-swift.noarch: W: spelling-error %description -l en_US Gluster -> Luster, Cluster, Bluster
glusterfs-openstack-swift.noarch: W: non-standard-group Application/File
glusterfs-openstack-swift.noarch: W: incoherent-version-in-changelog 1.8.0-7 ['1.8.0-7.1.fc21', '1.8.0-7.1']
^^^
please correct the changelog

glusterfs-openstack-swift.noarch: W: invalid-license Apache
^^

!!! The license must be correct.

glusterfs-openstack-swift.noarch: W: no-url-tag



glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-plugin
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-ufo
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-container
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-object
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-proxy
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-account


glusterfs-openstack-swift.noarch: W: conffile-without-noreplace-flag /etc/swift/container-server.conf-gluster
glusterfs-openstack-swift.noarch: W: conffile-without-noreplace-flag /etc/swift/account-server.conf-gluster
glusterfs-openstack-swift.noarch: W: conffile-without-noreplace-flag /etc/swift/object-server.conf-gluster
glusterfs-openstack-swift.noarch: W: conffile-without-noreplace-flag /etc/swift/fs.conf-gluster
glusterfs-openstack-swift.noarch: W: conffile-without-noreplace-flag /etc/swift/swift.conf-gluster
glusterfs-openstack-swift.noarch: W: conffile-without-noreplace-flag /etc/swift/proxy-server.conf-gluster


Probably you forgot to mark them as config file?


glusterfs-openstack-swift.noarch: W: no-documentation
glusterfs-openstack-swift.noarch: W: no-manual-page-for-binary gluster-swift-gen-builders
glusterfs-openstack-swift.noarch: W: no-manual-page-for-binary gluster-swift-print-metadata
glusterfs-openstack-swift.src: W: summary-ended-with-dot C GlusterFS Integration with OpenStack Object Storage (Swift).
glusterfs-openstack-swift.src: W: spelling-error %description -l en_US Gluster -> Luster, Cluster, Bluster
glusterfs-openstack-swift.src: W: spelling-error %description -l en_US gluster -> fluster, glister, luster
glusterfs-openstack-swift.src: W: non-standard-group Application/File
glusterfs-openstack-swift.src: W: invalid-license Apache
glusterfs-openstack-swift.src: W: no-url-tag
glusterfs-openstack-swift.src:3: W: macro-in-comment %{name}
glusterfs-openstack-swift.src:3: W: macro-in-comment %{version}
glusterfs-openstack-swift.src:3: W: macro-in-comment %{release}
glusterfs-openstack-swift.src:6: W: macro-in-comment %{name}
glusterfs-openstack-swift.src:6: W: macro-in-comment %{version}
glusterfs-openstack-swift.src:7: W: macro-in-comment %{name}
glusterfs-openstack-swift.src:7: W: macro-in-comment %{version}
glusterfs-openstack-swift.src:8: W: macro-in-comment %{name}
glusterfs-openstack-swift.src:8: W: macro-in-comment %{version}
glusterfs-openstack-swift.src:8: W: macro-in-comment %{release}
glusterfs-openstack-swift.src:8: W: macro-in-comment %{name}
glusterfs-openstack-swift.src:8: W: macro-in-comment %{version}
glusterfs-openstack-swift.src:8: W: macro-in-comment %{name}
glusterfs-openstack-swift.src:34: W: hardcoded-packager-tag gluster-users@gluster.org


glusterfs-openstack-swift.src:47: W: unversioned-explicit-obsoletes glusterfs-swift-plugin
glusterfs-openstack-swift.src:48: W: unversioned-explicit-obsoletes glusterfs-swift
glusterfs-openstack-swift.src:49: W: unversioned-explicit-obsoletes glusterfs-ufo
glusterfs-openstack-swift.src:50: W: unversioned-explicit-obsoletes glusterfs-swift-container
glusterfs-openstack-swift.src:51: W: unversioned-explicit-obsoletes glusterfs-swift-object

Please do only versioned obsoletes. Although others would work and this package replaces the other: just be explicit.
Comment 3 Kaleb KEITHLEY 2013-09-03 11:07:09 EDT
Right off the bat...

Vendor: should not be Red Hat. In the glusterfs packages I/we use either Fedora Project or glusterfs.org depending on who builds the packages.

Version and Release: 1.8.0 for rawhide and Fedora 20 — really? and Release should be 1.

Various Requires of openstack-swift-* = 1.8.0. Both Fedora 20 and rawhide have openstack-swift-* 1.9.0 and 1.8.0 is not available. 

You might wish to consider starting with replacing glusterfs-ufo with this in Fedora 19 (this is my personal hotlist item) and then expand this later to include Fedora 20 and rawhide when the port/rebase to openstack-swift-* 1.9.x is ready.
Comment 4 Luis Pabón 2013-09-03 13:31:32 EDT
Thanks for the quick turn-around.  I will take a look at your review and comments. 

On the subject of versions, I will resubmit as 1.9.0 for Fedora 20
Comment 5 Luis Pabón 2013-09-04 14:25:21 EDT
Please review the following files.  These are now v1.9.1 and have successfully been built with koji on f19, f20, and f21.

Spec URL: https://github.com/lpabon/fedora/raw/master/1.9.1/glusterfs-openstack-swift.spec

SRPM URL: https://github.com/lpabon/fedora/raw/master/1.9.1/glusterfs-openstack-swift-1.9.1-0.fc19.src.rpm
Comment 6 Kaleb KEITHLEY 2013-09-04 15:27:41 EDT
Is this really the bits that work with openstack-swift-1.9.x? Fedora 19 still has openstack-swift-1.8.0.

And See my comments above about Fedora 20 having openstack-swift-1.9.0 still, not 1.9.1. And openstack-swift hasn't been built for rawhide yet, so let's not get ahead of ourselves (While I could, technically, kick off a build 1.9.0 for rawhide, I'm not sure I see a need for it quite yet, and it might upset an apple cart or two.)

And finally, as above, Release should be 1, not 0. 0 is reserved for packages that cannot or must not be updated, e.g. foo-1.2.3-0.x86_64.rpm, or for prereleases like I did for glusterfs in fedora 19 to make the oVirt/vdsm people happy. E.g. glusterfs-3.4.0-0.1.beta.x86_64.rpm which will be updated by glusterfs-3.4.0-0.2.beta.x86_64.rpm and glusterfs-3.4.0-1.x86_64.rpm.

You'll have branches in your Fedora SCM git repo. Master is used to build for rawhide, and then you'd have — today anyway — f20, f19, and perhaps f18 and el6 branches. In the master and f20 branches you'd have a spec and sources that work with openstack-swift-1.9.x. In the f19 branch (and perhaps el6 branch) the spec and sources need to work with openstack-swift-1.8.0. (And for f18 you could use the glusterfs-swift-1.8.0 RPMs if you were so inclined.)

So pick one of f19 or f20, get it reviewed, accepted, Fedora SCM repo created, branched, etc., and then do the others. While I personally might like it if you did F19 first, the fact that F20 is close to alpha might mean it makes more sense to do that first.
Comment 7 Björn 'besser82' Esser 2013-09-05 03:54:07 EDT
I'll take this.
Comment 8 Luis Pabón 2013-09-05 10:37:41 EDT
Ok, I have updated both v1.8.0 and v1.9.1 of the SRPMs.  I am not sure which version of Fedora should receive this change, but I will let you guys decide that.

For Fedora 19, here is v1.8.0:
Spec URL: https://github.com/lpabon/fedora/raw/master/1.8.0/glusterfs-openstack-swift.spec

SRPM URL: https://github.com/lpabon/fedora/raw/master/1.8.0/glusterfs-openstack-swift-1.8.0-1.fc19.src.rpm

For Fedora 20+, here is v1.9.1 which is compatible only with openstack-swift-1.9.1 (For more information on Swift 1.9.1 please see https://launchpad.net/swift ):
Spec URL: https://github.com/lpabon/fedora/raw/master/1.9.1/glusterfs-openstack-swift.spec

SRPM URL: https://github.com/lpabon/fedora/raw/master/1.9.1/glusterfs-openstack-swift-1.9.1-1.fc19.src.rpm
Comment 9 Kaleb KEITHLEY 2013-09-09 07:43:53 EDT
You can't specify openstack-swift-1.9.1 for F20 because, at least as of right now, F20 only has 1.9.0.
Comment 10 Kaleb KEITHLEY 2013-09-09 13:29:26 EDT
Looking better: F19 w/ 1.8.0 builds on my f19, koji scratch builds is good. (http://koji.fedoraproject.org/koji/taskinfo?taskID=5914676)

rpmlint points out a few issues that should probably be fixed:
 1) get rid of how to build, it seems unnecessary.
 2) line 35 get rid of the Packager line
 3) lines 48-54, fix Obsoletes w/o versions, e.g. glusterfs-ufo <= 3.4.0
 4) line 32, mixed spaces and tabs
 5) line 36, fix License, s/b one of "ASL 1.0", "ASL 1.1", or "ASL 2.0" (see https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses)

If we could get through the review processess and get the git repo using F19 and 1.8.0 I think that would be good and we can just make minor tweaks for F20 and 1.9.0.
Comment 11 Luis Pabón 2013-09-11 19:27:15 EDT
I have updated v1.8.0 with the comments above.

For Fedora 19, here is v1.8.0:
Spec URL: https://github.com/lpabon/fedora/raw/master/1.8.0/glusterfs-openstack-swift.spec

SRPM URL: https://github.com/lpabon/fedora/raw/master/1.8.0/glusterfs-openstack-swift-1.8.0-1.fc19.src.rpm
Comment 12 Kaleb KEITHLEY 2013-09-20 07:53:26 EDT
Pete Zaitcev just built 1.9.1 for rawhide/f21.

It doesn't seem like anyone is planning to update to 1.9.1 for f20.

Can we push this forward and wrap it up?

And I'm going to drop the obsolete glusterfs-ufo RPM in the packaging of glusterfs-3.4.1,l which is due out in a couple of days, and F20 Alpha is also coming out in a couple of days.
Comment 13 Luis Pabón 2013-09-22 20:31:12 EDT
I think Pete Zaitcev has just been able to also update fedora 20 with 1.9.1.  In preparation for that version of OpenStack Swift, I have also updated the gluster-swift version 1.9.1 SRPM:

Spec URL: https://github.com/lpabon/fedora/raw/master/1.9.1/glusterfs-openstack-swift.spec

SRPM URL: https://github.com/lpabon/fedora/raw/master/1.9.1/glusterfs-openstack-swift-1.9.1-1.fc19.src.rpm
Comment 14 Luis Pabón 2013-09-30 13:43:05 EDT
Is there anything I still need to do to continue the process?
Comment 15 Kaleb KEITHLEY 2013-10-07 08:29:22 EDT
Hi Bjorn,

Can we please get this reviewed and finished up?

I pushed GlusterFS-3.4.1 to stable today for f18, f19, and f20, which removed the old UFO package and the (broken) dependency on openstack-swift. We need this package in sooner, rather than later as we now have no package in Fedora or EPEL to replace what we used to have.

If there is some hold-up or you can't find the time time to do it please let us know so that we can try to find someone else to review it.

Thanks,
Comment 16 Björn 'besser82' Esser 2013-10-21 07:21:17 EDT
(In reply to Kaleb KEITHLEY from comment #15)
> Hi Bjorn,
> 
> Can we please get this reviewed and finished up?
> 
> If there is some hold-up or you can't find the time time to do it please let
> us know so that we can try to find someone else to review it.
> 
> Thanks,

Hi Kaleb!

Sorry for the delay on this.  I'm currently reviewing the different versions for the corresponding dists.  This just took some time to go through.  You can expect this to be finished until tomorrow ~ 11:00 UTC +0200.

Cheers,
  Björn
Comment 17 Matthias Runge 2013-10-21 12:02:54 EDT
I had the chance to give this a quick review.

* Source0 needs to be a URL, or we need a comment, how to produce the tarball. Currently, this comes "out of nothing". Even worse: try to get the tarball from the gluster page.
* remove python from buildrequires and from requires.
* versioned requirements are discouraged: Every time when swift is updated, your requirements will break, you need to adjust your spec and to rebuild, even if swift changed in a minimal way or released a bugfix, even without changing the api.
At least, I'd change that to Requires : openstack-swift >= 1.9.0 (the same applies for the other versioned requirements)
* remove clean section from the spec, as well as the defattr line from files section
* I see, the tarball contains unittests, and functional tests as well. I'd expect those tests to be executed at build time; you can safely remove them from the final packages. Still it's a good check, that your package is not utterly broken:
%check
%{__python} setup.py test
Note: You'd need to add the requirements from test-requires to the build requirements as well.
* are you planning to submit this package for RHEL5 as well? If not, remove the first three lines.
Comment 18 Kaleb KEITHLEY 2013-11-05 07:32:11 EST
Matthias Runge wrote to me:

> There were quite a few issues noted in the bug report, and they're not
> fixed until now (if I'm not wrong here).
> 
> For example, there is no description, how to produce the used tarball.
> I'd prefer to tag a release in git repo, or to provide tarballs from the
> gluster.org website to download. Is it prossible to provide them?

Luis, can you please address Björn's and Matthias's issues and push this along. Among other things I know openstack-swift does not build on RHEL 5.

I've uploaded the tarball to http://download.gluster.org/pub/gluster/glusterfs-openstack-swift/1.9.1/glusterfs-openstack-swift-1.9.1-1.tar.gz and I presume the git repo is tagged accordingly.

There is also the review checklist from https://fedoraproject.org/wiki/PackagingDrafts/ReviewTemplate and copied here

 +:ok, =:needs attention, -:needs fixing

MUST Items:
[] MUST: rpmlint must be run on every package.
<<output if not already posted>>
[] MUST: The package must be named according to the Package Naming Guidelines.
[] MUST: The spec file name must match the base package %{name}
[] MUST: The package must meet the Packaging Guidelines. [FIXME?: covers this list and more]
[] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines.
[] MUST: The License field in the package spec file must match the actual license.
[] MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.
[] MUST: The spec file must be written in American English.
[] MUST: The spec file for the package MUST be legible.
[] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL.
<<md5sum checksum>>
[] MUST: The package must successfully compile and build into binary rpms on at least one supported architecture.
[] MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch.
[] MUST: All build dependencies must be listed in BuildRequires
[] MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro.
[] MUST: Every binary RPM package which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
[] MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review
[] MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory.
[] MUST: A package must not contain any duplicate files in the %files listing.
[] MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line.
[] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[] MUST: Each package must consistently use macros, as described in the macros section of Packaging Guidelines.
[] MUST: The package must contain code, or permissible content. This is described in detail in the code vs. content section of Packaging Guidelines.
[] MUST: Large documentation files should go in a doc subpackage.
[] MUST: If a package includes something as %doc, it must not affect the runtime of the application.
[] MUST: Header files must be in a -devel package.
[] MUST: Static libraries must be in a -static package.
[] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability).
[] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package.
[] MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} 
[] MUST: Packages must NOT contain any .la libtool archives, these should be removed in the spec.
[] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section.
[] MUST: Packages must not own files or directories already owned by other packages.
[] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[] MUST: All filenames in rpm packages must be valid UTF-8.

SHOULD Items:
[] SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
[] SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
[] SHOULD: The reviewer should test that the package builds in mock.
[] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[] SHOULD: The reviewer should test that the package functions as described.
[] SHOULD: If scriptlets are used, those scriptlets must be sane.
[] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[] SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb.
[] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
[] SHOULD: Packages should try to preserve timestamps of original installed files.
Comment 19 Luis Pabón 2013-11-10 23:21:47 EST
Hi all, thank you for the reviews.  I have a new 1.10.0-1 version available for F19+:

SPEC URL:
https://raw.github.com/lpabon/fedora/master/1.10.0/glusterfs-openstack-swift.spec

SRPM URL:
https://github.com/lpabon/fedora/raw/master/1.10.0/glusterfs-openstack-swift-1.10.0-1.fc19.src.rpm
Comment 20 Luis Pabón 2013-12-04 14:09:29 EST
Any update?
Comment 21 Matthias Runge 2013-12-06 02:39:38 EST
Oh yes:

why do you use these: 
%{!?_version:%define _version 1.10.0}
%{!?_name:%define _name glusterfs-openstack-swift}
%{!?_release:%define _release 1}
Please move the definitions to the corresponding place. it makes the whole thing more readable.

version, name and release are already variables.

- remove rm -rf ... from the clean section
https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean

- using vendor is unusual
- if you have tests, I'd love to see them executed during build. Of course, there is no need to package them. %exclude is the corresponding tag for files section.
Comment 22 Matthias Runge 2013-12-06 02:49:06 EST
more: 
please adapt to the following:
- https://fedoraproject.org/wiki/Packaging:Python#BuildRequires

and
- https://fedoraproject.org/wiki/Packaging:Python#Macros

%{__python} deprecated
The unversioned macro, %{__python} is deprecated. You should use %{__python2} to explicitly reference the python2 interpreter instead. This is future proofing for the time when things will be switched over to python3 by default instead of python2.

Is
URL      : http://launchpad.net/gluster-swift

really the home page of this project? If not, please change accordingly.
Comment 23 Björn 'besser82' Esser 2013-12-06 03:09:34 EST
I can't see any senseful movement here, unfortunately.  :(

Now there is a tarball on Canonical's launchpad, but I still can't determine where it is from, nor how was produced.  Still no releases recognizable inside git-repo's commits; no git-tags marking a release or anything...

I still would intend to review this, but be the meaning of reproducible release-tarballs, there is less than nothing...
Comment 24 Luis Pabón 2013-12-12 16:39:08 EST
(In reply to Björn "besser82" Esser from comment #23)
> I can't see any senseful movement here, unfortunately.  :(
> 
> Now there is a tarball on Canonical's launchpad, but I still can't determine
> where it is from, nor how was produced.  Still no releases recognizable
> inside git-repo's commits; no git-tags marking a release or anything...
> 
> I still would intend to review this, but be the meaning of reproducible
> release-tarballs, there is less than nothing...

Hi Bjorn, I think you may be looking at the github repo tracking this spec file.  The project stores package downloads on launchpad.net/gluster-swift as well as directing the viewer to the Code: https://github.com/gluster/gluster-swift .  The code is tagged and released accordingly.  Please let me know if this helps or if you have any more questions.
Comment 25 Luis Pabón 2013-12-12 16:41:05 EST
(In reply to Matthias Runge from comment #22)
> more: 
> please adapt to the following:
> - https://fedoraproject.org/wiki/Packaging:Python#BuildRequires
> 
> and
> - https://fedoraproject.org/wiki/Packaging:Python#Macros
> 
> %{__python} deprecated
> The unversioned macro, %{__python} is deprecated. You should use
> %{__python2} to explicitly reference the python2 interpreter instead. This
> is future proofing for the time when things will be switched over to python3
> by default instead of python2.
> 
> Is
> URL      : http://launchpad.net/gluster-swift
> 
> really the home page of this project? If not, please change accordingly.

Thank you Matthias for your review.  Yes, we host our project details and bugs on launchpad.net just as OpenStack Swift does.
Comment 26 Luis Pabón 2013-12-12 16:57:34 EST
Thank you all for your comments.  I didn't even know about the %exclude macro, that makes things a _lot_ easier.

Please take a look at the new updated spec file:

SPEC URL:
https://raw.github.com/lpabon/fedora/master/1.10.0/glusterfs-openstack-swift.spec

SRPM URL:
https://github.com/lpabon/fedora/raw/master/1.10.0/glusterfs-openstack-swift-1.10.0-1.fc19.src.rpm
Comment 27 Cole Robinson 2014-02-14 15:40:22 EST
I'll help finish the review
Comment 28 Cole Robinson 2014-02-14 16:29:29 EST
Group    : Applications/File

RPM groups are basically worthless, but maybe change this to Applications/System like openstack-nova uses (though openstack-swift uses Development/Languages ??)

[X]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed

I assume defattr is there because this package will be built for el6, similar to openstack and gluster, so that makes sense.

[ ]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /etc/swift(openstack-swift),
     /usr/lib/python2.7/site-packages/gluster(glusterfs-api)

Package wants to own /etc/swift, which is already owned by openstack-swift. So drop. So drop %dir %{_confdir} . Same with the gluster path, I think you can just change  %{python_sitelib}/gluster to %{python_sitelib}/gluster/swift

Interesting rpmlint output:

glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-plugin
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-ufo
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-container
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-object
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-proxy
glusterfs-openstack-swift.noarch: W: obsolete-not-provided glusterfs-swift-account

Since this doesn't appear to just be a package rename for the gluster bits, plain obsolete is fine here

glusterfs-openstack-swift.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gluster/swift/common/middleware/gswauth/setup.py 0644L /usr/bin/python

Ah, a forked package buried way down in the module hierarchy :) My recommendation is to re-organize this in some less crazy way upstream, but in the short term, just %exclude those nested setup.py* and test_swauth since they aren't important.

Also, seconding mrunge's suggestion of adding a %check section that runs your packages unit tests, but it's not a hard requirement.
Comment 29 Cole Robinson 2014-02-14 16:30:56 EST
Also, I'm not a sponsor, and it looks like you'll need one, so blocking FE-NEEDSPONSOR. Maybe mrunge can help when we get to it
Comment 30 Michael Schwendt 2014-02-14 17:20:30 EST
Latest EL6 and EL5 contain RPM >= 4.4, so %defattr is not needed for them.
Comment 31 Cole Robinson 2014-02-14 18:14:49 EST
Michael, does that mean this package would need to add an explicit dependency on newer rpm? Or does some automatic dependency handle it?
Comment 32 Michael Schwendt 2014-02-15 04:58:37 EST
It means %defattr can simply be dropped unless you want to build the package for some much older target. In koji, there is RPM 4.4.x for EL5 and RPM 4.8.0 for EL6.
Comment 33 Matthias Runge 2014-02-17 05:46:49 EST
(In reply to Cole Robinson from comment #29)
> Also, I'm not a sponsor, and it looks like you'll need one, so blocking
> FE-NEEDSPONSOR. Maybe mrunge can help when we get to it

Yeah, I could. Then I should even do the review; sadly, my time is currently very limited. 

In order to get sponsored, it's a good policy, to look at other's peoples review requests and to comment on them:
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

To run fedora-review against this bug here, would have catched many issues mentioned earlier. *hint*

fedora-review -b 1003089
Comment 34 Cole Robinson 2014-02-19 11:03:26 EST
FWIW I'm offline till Monday, but if there's any change I'll respond ASAP when I'm back
Comment 35 Matthias Runge 2014-03-13 05:16:30 EDT
Luis,

could you please address the issues, Cole noted in c28?

Also please fix this: 

glusterfs-openstack-swift.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/gluster/swift/common/middleware/gswauth/setup.py 0644L /usr/bin/python
Comment 36 Niels de Vos 2014-03-13 10:46:53 EDT
Just to prevent any confusion, I'll be working with Luis and have taken care of the sponsoring.

Anyone can proceed with this review, or let me know and I'll pick it up from here.

Thanks to all who contributed so far!
Comment 37 Cole Robinson 2014-03-13 10:56:48 EDT
I'm happy to finish up the review. I'll comment here when I start the next review round, once Luis respins the package.
Comment 38 Matthias Runge 2014-04-29 05:16:31 EDT
What's the status here? There are a few issues noted above to be fixed, but in general not a big deal.
Comment 39 Luis Pabón 2014-04-29 15:43:38 EDT
Hi, sorry for the delay.  We are currently in the process or renaming the project from gluster-swift (http://launchpad.net/gluster-swift) to SwiftOnFile (https://github.com/swiftonfile/swiftonfile).  This change will clear a lot of confusion in the community.  We will be resubmitting our spec file for swiftonfile by mid May which will upgrade glusterfs-ufo* rpms.  We will be basing the swiftonfile rpm spec file from the knowledge gathered here.  Thank you very much for the help.

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