Bug 1072054
Summary: | Review Request: gnome-code-assistance - Common code assistance services for code editors | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Elad Alfassa <elad> |
Component: | Package Review | Assignee: | Kalev Lember <kalevlember> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | icq, jessevdk, kalevlember, package-review, panemade, pborelli |
Target Milestone: | --- | Flags: | kalevlember:
fedora-review+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | gnome-code-assistance-0.3.1-5.fc20 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2014-05-17 06:33:09 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: |
Description
Elad Alfassa
2014-03-03 19:26:21 UTC
(In reply to Elad Alfassa from comment #0) >upstream said they might do a release soon just for us. Release is now done (In reply to paolo borelli from comment #1) > Release is now done I can only see a release from last November. gnome-code-assistance 0.3.1 is out now; can you update the packaging please? Updated. 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.1-1.fc21.src.rpm 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 :) 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. > 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. Any news on this? Updated. 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.1-2.fc21.src.rpm 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 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. 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). Updated. 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.1-3.fc21.src.rpm 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? Indeed, good catch. 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.1-4.fc21.src.rpm 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 New Package SCM Request ======================= Package Name: gnome-code-assistance Short Description: Common code assistance services for code editors Owners: nacho Branches: f20 InitialCC: Git done (by process-git-requests). 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? NOTE: Misformatted request; using 'Branches' instead. WARNING: No new branches requested. 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 gnome-code-assistance-0.3.1-5.fc20 has been pushed to the Fedora 20 testing repository. gnome-code-assistance-0.3.1-5.fc20 has been pushed to the Fedora 20 stable repository. |