Bug 1072054 - Review Request: gnome-code-assistance - Common code assistance services for code editors
Summary: Review Request: gnome-code-assistance - Common code assistance services for c...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kalev Lember
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-03-03 19:26 UTC by Elad Alfassa
Modified: 2014-05-17 06:33 UTC (History)
6 users (show)

Fixed In Version: gnome-code-assistance-0.3.1-5.fc20
Clone Of:
Environment:
Last Closed: 2014-05-17 06:33:09 UTC
Type: ---
Embargoed:
kalevlember: fedora-review+


Attachments (Terms of Use)

Description Elad Alfassa 2014-03-03 19:26:21 UTC
Spec URL: http://elad.fedorapeople.org/reviews/gnome-code-assistance/gnome-code-assistance.spec
SRPM URL: http://elad.fedorapeople.org/reviews/gnome-code-assistance/gnome-code-assistance-0.3.20140303git-1.fc21.src.rpm
Description: gnome-code-assistance is a project which aims to provide common code assistance
services for code editors (simple editors as well as IDEs). It is an effort to
provide a centralized code-assistance as a service for the GNOME platform
instead of having every editor implement their own solution.

Fedora Account System Username: elad

This package is required to update gedit-code-assistance to the latest version (which hasn't been updated in Fedora for AGES). Packaging a git snapshot is not optimal, upstream said they might do a release soon just for us. 
To make things easier, I did not include the commit hash in the package version name, and that is okay by the package version-and-naming guidelines.

Comment 1 paolo borelli 2014-03-06 18:31:46 UTC
(In reply to Elad Alfassa from comment #0)
>upstream said they might do a release soon just for us.


Release is now done

Comment 2 Kalev Lember 2014-03-21 11:37:51 UTC
(In reply to paolo borelli from comment #1)
> Release is now done

I can only see a release from last November.

Comment 3 Kalev Lember 2014-03-25 11:36:29 UTC
gnome-code-assistance 0.3.1 is out now; can you update the packaging please?

Comment 5 Kalev Lember 2014-04-15 12:20:41 UTC
A hard packaging question is how to deal with multiple backends. People who are working on C code might not appreciate getting a ruby interpreter, for example.

The way the packaging is set up currently, is that the gnome-code-assistance package has hard deps on all the supported interpreters / language runtimes. Right now this includes python, ruby, vala, clang, gjs, but I am sure this list only gets bigger in the future when gnome-code-assistance gets more backends.

I don't have any good ideas how to improve this though :)

Comment 6 Elad Alfassa 2014-04-15 12:42:26 UTC
We could split each backend to a subpackage, but it will keep growing and growing. Also, the ruby interpreter is needed for the CSS and HTML plugins, and many people work on these too.

We need to find a proper solution for this, especially because I think this should be default in the Workstation product as it will improve the "developer experience" of Fedora.

Comment 7 Kalev Lember 2014-04-15 14:23:47 UTC
> License:        GPLv2+

I think this should be GPLv3+ instead. Some code files are GPLv2+, some are GPLv3+, and some are MIT.

Also, something for upstream: a bunch of code files are missing license headers altogether; would be great if upstream could add these. And the COPYING that's shipped is for LGPL, even though the code files (those that have headers) seem to be GPL.

> URL:            http://gnome.org

http://wiki.gnome.org/Projects/CodeAssistance

> Source0:        http://ftp.acc.umu.se/pub/GNOME/sources/gnome-code-assistance/0.3/%{name}-%{version}.tar.xz

Would be better to use download.gnome.org instead of the acc.umu.se mirror. https://download.gnome.org/sources/gnome-code-assistance/0.3/gnome-code-assistance-0.3.1.tar.xz

> %files
> %{_datadir}/dbus-1/services/*.service

Should have Requires: dbus for both the directory ownership and because the service just doesn't work without it. Not sure how the kdbus plans are going to affect this in the future.

Comment 8 Ignacio Casal Quinteiro (nacho) 2014-04-22 08:15:27 UTC
Any news on this?

Comment 10 Kalev Lember 2014-04-24 11:28:58 UTC
Looks nice, but I think the licensing stuff needs a bit more work. The license tag now reads "GPLv3+, MIT" but it's still a bit unclear. There are more licenses used -- in particular I've found files with LGPLv2+ and GPLv2+. Not sure how to best write it in the license tag. I guess one option would be to only specify the strictest license (''License: GPLv3+' in this case), or alternatively list all of them ('License: GPLv3+ and GPLv2+ and LGPLv2+ and MIT'). Note that I haven't done a full license audit so there might be more.

https://fedoraproject.org/wiki/Licensing:FAQ#Multiple_licensing_situations

Another licensing issue is that the rpm should ship all the LICENSE files. A few are in the subdirectories:

backends/css/gems/sass-3.2.12/MIT-LICENSE
backends/css/gems/sass-3.2.12/vendor/listen/LICENSE
backends/go/deps/src/github.com/jessevdk/go-flags/LICENSE
backends/go/deps/src/github.com/guelfey/go.dbus/LICENSE
backends/go/deps/src/code.google.com/p/go.tools/LICENSE

The webkitgtk3 package has a add_to_doc_files() macro to deal with multiple license files, might be worth copying it from there: http://pkgs.fedoraproject.org/cgit/webkitgtk3.git/tree/webkitgtk3.spec

Comment 11 Elad Alfassa 2014-04-24 12:08:27 UTC
The go files are irrelevant because I'm not builidng the go backend atm

Also, what we are seeing here is worse, actually. That's bundling of system libs and that's not good. I need to investigate exactly what is bundled here apart from what we see here (rubygem-sass), and figure out how to strip it out without harming functionality.

It might also make licensing a bit simpler.
I'll take a poke at it later today.

Comment 12 Jesse van den Kieboom 2014-04-24 13:24:40 UTC
The reason for distributing things like sass in the gnome-code-assistance sources is because distro's do not always handle these kind of dependencies in the same way. Maybe fedora packages sass separately, but other distro's might not. It's therefore provided in-tree as a convenience. However, if sass is available on the system at configure time, then it gnome-code-assistance will use that, instead of the in-tree sass.

See for example https://git.gnome.org/browse/gnome-code-assistance/tree/backends/css/deps.mf which conditionally installs sass depending on RUBY_SASS (which is defined when there was a system sass available during configure).

Comment 14 Kalev Lember 2014-05-05 19:50:50 UTC
Cool, great work!

While you are at this, is /usr/libexec/gnome-code-assistance/backends/py/gnome/codeassistance/json/deps/simplejson/ also something that could be used from distro packages? Is that python3-simplejson, perhaps?

Comment 16 Kalev Lember 2014-05-05 21:34:04 UTC
And here's the review checklist:

Fedora review gnome-code-assistance-0.3.1-4.fc21.src.rpm 2014-05-05

$ rpmlint gnome-code-assistance \
          gnome-code-assistance-debuginfo \
          gnome-code-assistance-0.3.1-4.fc21.src.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings.

+ OK
! needs attention

+ rpmlint is quiet
+ The package is named according to Fedora packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The package contains the license file (COPYING)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match the sources in the srpm
  c0fddeea5dedab1d962883d7b7ed51b3  gnome-code-assistance-0.3.1.tar.xz
  c0fddeea5dedab1d962883d7b7ed51b3  Download/gnome-code-assistance-0.3.1.tar.xz
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a locale handling
n/a ldconfig in %post and %postun
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
! Package owns all the directories it creates

  /usr/libexec/gnome-code-assistance/ is unowned

+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The package must contain code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package
n/a Header files should be in -devel
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
n/a -devel must require the fully versioned base
+ Packages should not contain libtool .la files
n/a .desktop file handling
+ Doesn't own files or directories already owned by other packages
+ Filenames are valid UTF-8

I've found two more issues while going over the checklist:

  1) Unowned /usr/libexec/gnome-code-assistance/ directory
  2) Shipping both Python 2 and Python 3 bytecode

Since this package uses Python 3, would be nice to remove the Python 2 bytecompiled files, or somehow prevent rpmbuild from creating them in the first place.

e.g. for one of the files, types.py:

$ rpm -ql gnome-code-assistance | grep types.*py
# Those are Python 3 bytecompiled files
/usr/libexec/gnome-code-assistance/backends/py/gnome/codeassistance/__pycache__/types.cpython-33.pyc
/usr/libexec/gnome-code-assistance/backends/py/gnome/codeassistance/__pycache__/types.cpython-33.pyo
#
/usr/libexec/gnome-code-assistance/backends/py/gnome/codeassistance/types.py
# ... and those are Python 2 bytecompiled files
/usr/libexec/gnome-code-assistance/backends/py/gnome/codeassistance/types.pyc
/usr/libexec/gnome-code-assistance/backends/py/gnome/codeassistance/types.pyo

Anyway, looks fine to me to go in, especially if you fix the unowned directory before importing the package.

APPROVED

Comment 17 Elad Alfassa 2014-05-06 09:41:46 UTC
New Package SCM Request
=======================
Package Name: gnome-code-assistance
Short Description: Common code assistance services for code editors
Owners: nacho
Branches: f20
InitialCC:

Comment 18 Gwyn Ciesla 2014-05-06 11:53:25 UTC
Git done (by process-git-requests).

Comment 19 Elad Alfassa 2014-05-06 12:35:43 UTC
Package Change Request
======================
Package Name: gnome-code-assistance
Owners: elad,nacho

I made a mistake in my SCM request and only listed nacho instead of listing nacho and myself. Can you please change that?

Comment 20 Gwyn Ciesla 2014-05-06 13:09:22 UTC
NOTE: Misformatted request; using 'Branches' instead.
WARNING: No new branches requested.

Comment 21 Fedora Update System 2014-05-06 13:37:23 UTC
gnome-code-assistance-0.3.1-5.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/gnome-code-assistance-0.3.1-5.fc20

Comment 22 Fedora Update System 2014-05-06 21:31:24 UTC
gnome-code-assistance-0.3.1-5.fc20 has been pushed to the Fedora 20 testing repository.

Comment 23 Fedora Update System 2014-05-17 06:33:09 UTC
gnome-code-assistance-0.3.1-5.fc20 has been pushed to the Fedora 20 stable repository.


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