Bug 829713

Summary: Review Request: grive - An open source Linux client for Google Drive
Product: [Fedora] Fedora Reporter: Vasiliy Glazov <vascom2>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: bugsgentoo, d.g.cameron, estembirek, fdc, gregory.lee.bartholomew, i, ivanfmartinez, jorti, lukast.dev, mail, martin.edlman, mjg, mrunge, notting, package-review, rebus, ron, tadej.j, tcallawa, vascom2, vladimir.rusinov
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-11-11 15:40:22 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Vasiliy Glazov 2012-06-07 11:42:47 UTC
Spec URL: https://github.com/RussianFedora/grive/blob/master/grive.spec
SRPM URL: http://koji.russianfedora.ru/koji/buildinfo?buildID=1608

Description: The purpose of this project is to provide an independent implementation of Google Drive client. It uses the Google Document List API to talk to the servers in Google. The code is written in standard C++.


I need a sponsor for include this package in Fedora.
You can look at my Self introduction here http://lists.fedoraproject.org/pipermail/devel/2012-June/168330.html

Comment 1 Sergio Basto 2012-06-07 15:04:56 UTC
Spec URL: https://github.com/RussianFedora/grive/blob/master/grive.spec
SRPM URL: http://koji.russianfedora.ru/packages/grive/0.1.0/1.fc17.R/src/grive-0.1.0-1.fc17.R.src.rpm

for fix:
$ fedora-review -b 829713 --mock-config fedora-17-x86_64

Comment 3 Vasiliy Glazov 2012-06-14 05:28:43 UTC
Updated to 0.1.1
SRPM URL: http://koji.russianfedora.ru/packages/grive/0.1.1/1.fc17.R/src/grive-0.1.1-1.fc17.R.src.rpm

Comment 4 Vasiliy Glazov 2012-06-21 09:38:07 UTC
Updated to 0.2.0

SRPM URL: http://koji.russianfedora.ru/packages/grive/0.2.0/1.fc17.R/src/grive-0.2.0-1.fc17.R.src.rpm

Comment 5 Matthias Runge 2012-06-21 10:41:14 UTC
Vasiliy, 

the project homepage is gone (http 404)

you should update it to match the new url: https://github.com/Grive/grive

If you're listing both 
SPEC: ....
SRPM: ....

it's easier for a reviewer to take a look onto your work.

- rm -rf in %install is not required anymore

Comment 6 Vasiliy Glazov 2012-06-21 10:55:19 UTC
"rm -rf $RPM_BUILD_ROOT" in %install section created by rpmdev-newspec.
Should I manually remove it from all new specs created by rpmdev-newspec?

Spec URL: https://raw.github.com/RussianFedora/grive/master/grive.spec
SRPM URL: http://koji.russianfedora.ru/packages/grive/0.2.0/1.fc17.R/src/grive-0.2.0-1.fc17.R.src.rpm

I correct spec then new release will be available.

Comment 7 Matthias Runge 2012-06-21 11:10:53 UTC
rpmdev-newspec is ONE way to create specs.

[!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf is only needed if supporting EPEL5

(I currently can't find the corresponding policy for that)


Build fails:
[ 94%] Building CXX object libgrive/CMakeFiles/grive.dir/src/bfd/Debug.cc.o
cd /builddir/build/BUILD/Grive-grive-f4b3e48/libgrive && /usr/lib/ccache/c++   -DHAVE_BFD -DVERSION=\"0.2.0\" -DTEST_DATA=\"/builddir/build/BUILD/Grive-grive-f4b3e48/
libgrive/test/data/\" -DSRC_DIR=\"/builddir/build/BUILD/Grive-grive-f4b3e4
8/libgrive/src\" -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4  -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables  -I/builddir/build/BUILD/Grive-grive-f4b3e48/libgrive/src -I/builddir/build/BUILD/Grive-grive-f4b3e48/libgrive/test    -o CMakeFiles/grive.dir/src/bfd/Debug.cc.o -c /builddir/build/BUILD/Grive-grive-f4b3e48/libgrive/src/bfd/Debug.cc
In file included from /builddir/build/BUILD/Grive-grive-f4b3e48/libgrive/src/bfd/SymbolInfo.cc:25:0:
/usr/include/bfd.h:37:2: error: #error config.h must be included before this header
make[2]: *** [libgrive/CMakeFiles/grive.dir/src/bfd/SymbolInfo.cc.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory `/builddir/build/BUILD/Grive-grive-f4b3e48'
make[1]: Leaving directory `/builddir/build/BUILD/Grive-grive-f4b3e48'
make[1]: *** [libgrive/CMakeFiles/grive.dir/all] Error 2
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.NP0BWq (%build)
RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.NP0BWq (%build)
Child return code was: 1
EXCEPTION: Command failed. See logs for output.
 # ['bash', '--login', '-c', 'rpmbuild -bb --target i686 --nodeps builddir/build/SPECS/grive.spec']
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/mockbuild/trace_decorator.py", line 70, in trace
    result = func(*args, **kw)
  File "/usr/lib/python2.7/site-packages/mockbuild/util.py", line 352, in do
    raise mockbuild.exception.Error, ("Command failed. See logs for output.\n # %s" % (command,), child.returncode)
Error: Command failed. See logs for output.
 # ['bash', '--login', '-c', 'rpmbuild -bb --target i686 --nodeps builddir/build/SPECS/grive.spec']
LEAVE do --> EXCEPTION RAISED

Comment 8 Matthias Runge 2012-06-21 11:16:22 UTC
Missing reference to no rm -rf in install:

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

Comment 9 Vasiliy Glazov 2012-06-21 12:27:02 UTC
Can you show full build log? Because my build was succesfull http://koji.russianfedora.ru/packages/grive/0.2.0/1.fc17.R/data/logs/i686/build.log

Comment 10 Vasiliy Glazov 2012-06-21 14:10:02 UTC
It not compile in rawhide. I created bugreport to upstream https://github.com/Grive/grive/issues/72

Comment 11 Vasiliy Glazov 2012-07-17 08:34:34 UTC
Spec URL: https://raw.github.com/RussianFedora/grive/master/grive.spec
SRPM URL: http://koji.russianfedora.ru/packages/grive/0.2.0/2.fc18.R/src/grive-0.2.0-2.fc18.R.src.rpm

Full pass fedora-review and can be compiled for rawhide.

Comment 12 David Cameron 2012-08-08 15:49:57 UTC
Hi,

Here is an informal package review.

I could not compile in mock (rawhide) due to build requirements problems

DEBUG util.py:258:  Getting requirements for grive-0.2.0-2.fc18.src
DEBUG util.py:258:   --> cmake-2.8.9-0.4.rc3.fc18.i686
DEBUG util.py:258:   --> Already installed : libstdc++-devel-4.7.1-5.fc18.i686
DEBUG util.py:258:   --> libcurl-devel-7.27.0-2.fc18.i686
DEBUG util.py:258:   --> json-c-devel-0.9-5.fc18.i686
DEBUG util.py:258:   --> expat-devel-2.1.0-4.fc18.i686
DEBUG util.py:258:   --> 1:openssl-devel-1.0.1c-6.fc18.i686
DEBUG util.py:258:   --> libgcrypt-devel-1.5.0-6.fc18.i686
DEBUG util.py:258:   --> boost-devel-1.48.0-16.fc18.i686
DEBUG util.py:258:   --> binutils-devel-2.22.52.0.4-8.fc18.i686
DEBUG util.py:258:  Error: Package: boost-python3-1.48.0-16.fc18.i686 (fedora)
DEBUG util.py:258:             Requires: libpython3.2mu.so.1.0
DEBUG util.py:258:   You could try using --skip-broken to work around the problem
DEBUG util.py:258:   You could try running: rpm -Va --nofiles --nodigest
DEBUG util.py:348:  Child return code was: 1

There was also an rpmlint warning (not picked up by fedora-review but when I ran manually in RHEL6) because you don't have a Group: tag in the spec:

W: non-standard-group Unspecified

However the review itself shows no problems except the compilation:

Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[!]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[!]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
     Note: The package did not built BR could therefore not be checked or the
     package failed to build because of missing BR
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[x]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[-]: MUST Macros in Summary, %description expandable at SRPM build time.
[-]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: 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 is included in %doc.
[x]: MUST License field in the package spec file matches the actual license.
[x]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
[x]: MUST Package meets the Packaging Guidelines.
[x]: MUST Package is named according to the Package Naming Guidelines.
[?]: MUST Package does not generates any conflict.
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[-]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[?]: MUST Package installs properly.
[-]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint output is silent.
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/mock/829713/grive-0.2.0.tar.gz :
  MD5SUM this package     : 8260b1e6c0369da35ebcfe8c8f840f2b
  MD5SUM upstream package : 8260b1e6c0369da35ebcfe8c8f840f2b

[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains a SysV-style init script if in need of one.
[x]: MUST File names are valid UTF-8.
[!]: SHOULD Reviewer should test that the package builds in mock.
[-]: 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.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[?]: SHOULD Package functions as described.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[?]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[-]: SHOULD %check is present and all tests pass.
[x]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

Issues:
[!]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[!]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
     Note: The package did not built BR could therefore not be checked or the
     package failed to build because of missing BR


Generated by fedora-review 0.1.2

Comment 13 Vasiliy Glazov 2012-08-13 12:45:28 UTC
Spec URL: https://raw.github.com/RussianFedora/grive/master/grive.spec
SRPM URL: http://koji.russianfedora.ru/packages/grive/0.2.0/3.fc18.R/src/grive-0.2.0-3.fc18.R.src.rpm

I added Group.

Your "not compile in rawhide" is temporary problem. And it is rawhide's problem.
Now it compile perfectly.

Comment 14 François Cami 2013-01-13 13:04:07 UTC
Note that this package probably fails there:
https://fedoraproject.org/wiki/Packaging:Guidelines#Packages_which_are_not_useful_without_external_bits

Blocking FE-Legal.

Comment 15 Tom "spot" Callaway 2013-01-13 16:27:52 UTC
What external bits does "grive" need to function? I'm not sure why you think it meets that criteria, aside from the need for Google Drive account (which is not really external bits).

Comment 16 François Cami 2013-01-13 17:30:05 UTC
I understand that grive doesn't need anything else in Fedora to work, but it is not useful without a Google Drive server (and account).

Quoting the wiki: "packages which are not functional or useful without code or packages from third-party sources are not acceptable for inclusion in Fedora."
I think, depending on how it's understood, that this could apply to packages that interface with proprietary SaaS since they are not usable without it. If it's not the case, my apologies :)

Comment 17 Vasiliy Glazov 2013-01-13 17:50:00 UTC
Interface of Google Drive is not proprietary. It has open API.

Comment 18 Tom "spot" Callaway 2013-01-13 18:26:43 UTC
Yeah, that's not the intent of that guideline. Lifting FE-Legal.

Comment 19 Lukas Tvrdy 2013-01-19 18:01:55 UTC
There is package checkgmail.noarch already in repositary. You need Google Mail account for that. Is that the same case as with Google Drive?

Comment 20 黃健毅 2013-04-11 06:08:09 UTC
@Lukas Tvrdy

Assuming this is exactly the same as the other version I've used (on Ubuntu), then if you have a Gmail account, you also have a Gdrive account.

You don't enter the account details, into the application, but the application gives you a URL to go to to grant grive access to your Google Drive so it can sync.

Comment 21 Jason Tibbitts 2013-06-04 18:04:25 UTC
I am triaging old review tickets.  I can't promise a review if you reply, but by closing out the stale tickets we can devote extra attention to the ones which aren't stale.

This fails to build for me; here is a scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5466794

Comment 22 Juan Orti 2013-07-03 10:11:38 UTC
Hello, I'm interested in this package and have created my own spec with the latest version.

Vasiliy, could you take a look at my changes? I can help you co-maintaning this package. Or, if you have dropped your interest in this package, I can submit my own review request.

Here is what I've done:
SPEC: http://jorti.fedorapeople.org/grive/grive.spec
SRPM: http://jorti.fedorapeople.org/grive/grive-0.3.0-0.1.20130702git27817e8.fc19.src.rpm

Comment 23 Vasiliy Glazov 2013-07-03 10:58:25 UTC
I'm still interested in this package.
And I agree be co-maintainer.

I hope it will help me to become a full maintainer.

Comment 24 Martin Edlman 2013-10-17 10:51:46 UTC
Hello Vasiliy and Juan,

I tried to compile SRPM (http://jorti.fedorapeople.org/grive/grive-0.3.0-0.1.20130702git27817e8.fc19.src.rpm) on my F19 build system and compilation failed bacause of missing gcc-c++ compiler.
Build didn't complain of it, so there is missing dependency in the spec file. You should check and fix the spec file.

BuildRequire: gcc-c++

Maybe it requires some other stuff which is already installed on my system, so it doesn't complain.

p.s.: thanks for grive! :-)

Regards,
Martin

Comment 25 Vasiliy Glazov 2013-10-17 10:56:43 UTC
No, gcc-c++ not needed in BR because https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Exceptions_2

Comment 26 Martin Edlman 2013-10-17 12:23:32 UTC
Ohh, I see. Sorry :-/

Comment 27 Eduard Stembirek 2013-10-18 08:03:04 UTC
Thanks ~ really appreciate this ~ good JOB !

Comment 28 Vasiliy Glazov 2013-10-18 08:11:22 UTC
Grive project seems dead.
Now I use this https://github.com/astrada/google-drive-ocamlfuse installed via OPAM (http://opam.ocamlpro.com/).

Comment 29 Christopher Meng 2013-10-18 08:17:17 UTC
(In reply to Vasiliy Glazov from comment #28)
> Grive project seems dead.
> Now I use this https://github.com/astrada/google-drive-ocamlfuse installed
> via OPAM (http://opam.ocamlpro.com/).

I'm packaging OPAM....;)

Comment 30 Vasiliy Glazov 2013-10-18 08:24:58 UTC
I am too :)
In russianfedora repositories.
But I am think OPAM not compatible with Fedora packaging policy.

Comment 31 Michael J Gruber 2013-11-11 14:23:29 UTC
If upstream is dead (as per comment #28) can we close this request?

Comment 32 Vasiliy Glazov 2013-11-11 15:40:22 UTC
Yes. Closed.