Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1379432 (odoo) - Review Request: odoo - Suite of web based open source business apps
Summary: Review Request: odoo - Suite of web based open source business apps
Keywords:
Status: CLOSED NOTABUG
Alias: odoo
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Alec Leamas
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: python-psycogreen python-jcconv
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2016-09-26 17:54 UTC by Björn 'besser82' Esser
Modified: 2017-02-08 13:10 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-02-08 13:10:29 UTC


Attachments (Terms of Use)

Description Björn 'besser82' Esser 2016-09-26 17:54:24 UTC
Description:

  Odoo is a complete ERP and CRM.  The main features are accounting
  (analytic and financial), stock management, sales and purchases
  management, tasks automation, marketing campaigns, help desk,
  POS, etc.  Technical features include a distributed server,
  flexible workflows, an object database, a dynamic GUI,
  customizable reports, and XML-RPC interfaces.


Koji Build:

  No build, because of unmet BuildRequires, currently under review.


Issues:

  No known packaging issues, but some complaints from rpmlint:

  odoo.noarch: E: non-readable /etc/odoo/odoo.conf 640
  odoo.noarch: E: non-standard-dir-perm /etc/odoo 750
  odoo.noarch: E: non-standard-dir-perm /var/log/odoo 750

  ---> Those dirs / files are not world-readable, because they may
       contain sensitive information, like db-password and such.

  odoo.noarch: W: empty-%postun

  ---> caused by %systemd_postun, which is an empty macro, but
       this error is consistent with packaging guidelines.

  odoo.noarch: W: non-standard-gid /etc/odoo odoo
  odoo.noarch: W: non-standard-gid /etc/odoo/odoo.conf odoo
  odoo.noarch: W: non-standard-gid /var/lib/odoo odoo
  odoo.noarch: W: non-standard-gid /var/log/odoo odoo
  odoo.noarch: W: non-standard-uid /etc/odoo odoo
  odoo.noarch: W: non-standard-uid /etc/odoo/odoo.conf odoo
  odoo.noarch: W: non-standard-uid /var/lib/odoo odoo
  odoo.noarch: W: non-standard-uid /var/log/odoo odoo

  ---> Those files and dirs are owned by the system-user for Odoo,
       which is created during %pre-scriptlet.  This makes sure
       the Odoo-server (but no regular user) has write access to
       those locations.

  odoo.noarch: W: spelling-error %description -l en_US customizable
                  -> customization
  odoo.noarch: W: spelling-error %description -l en_US workflows
                  -> work flows, work-flows, workloads

  ---> False positive spelling-errors.


FAS-User:

  besser82


Urls:

  Spec URL:  https://besser82.fedorapeople.org/review/jekyll/odoo.spec
  SRPM URL:  https://besser82.fedorapeople.org/review/odoo-9.0c.20160923-0.1.fc26.src.rpm


Thanks for review in advance!

Comment 2 Björn 'besser82' Esser 2016-10-03 08:21:27 UTC
Updated package:

Urls:

  Spec URL:  https://besser82.fedorapeople.org/review/odoo.spec
  SRPM URL:  https://besser82.fedorapeople.org/review/odoo-9.0c.20161003-0.1.fc26.src.rpm

Comment 3 Björn 'besser82' Esser 2016-10-26 18:16:49 UTC
Updated package:

  Dependencies have fully landed in fc25 and Rawhide.


Koji Build:

  http://koji.fedoraproject.org/koji/taskinfo?taskID=16211714


Urls:

  Spec URL:  https://besser82.fedorapeople.org/review/odoo.spec
  SRPM URL:  https://besser82.fedorapeople.org/review/odoo-10.0.20161026-0.1.fc26.src.rpm

Comment 4 Alec Leamas 2016-10-26 19:12:36 UTC
Being the responsible for the predecessor (which should have been orphaned long time ago) this is my duty.  WIll look into into it on a few days.

Comment 5 Alec Leamas 2016-10-26 22:07:17 UTC
Some initial remarks, before going into the review:

   - I see all sorts of licenses in the code: Apache, BSD, GPLv3, MIT... can you review this? The LGPLv3 for everything doesn't seem OK.
  - Here seems to be some bundled, minified javascript libraries such as react. Please refer to https://fedoraproject.org/wiki/Packaging:JavaScript. 

Here is also things like (./odoo/addons/l10n_cr/__init__.py:)

At top;
# Part of Odoo. See LICENSE file for full copyright and licensing details.

line 8-9
#    First author: Carlos Vásquez <carlos.vasquez@clearcorp.co.cr> (ClearCorp S.A.)
#    Copyright (c) 2010-TODAY ClearCorp S.A. (http://clearcorp.co.cr). All rights reserved

To me, this doesn't make sense. It looks like the author has claimed all rights, and published under a MIT license. And then odoo has pasted there own license on top, more or less hijacking the code. I am not a lawyer, but this looks fishy. Thoughts?

Comment 6 Björn 'besser82' Esser 2016-10-26 23:23:35 UTC
(In reply to Alec Leamas from comment #5)
> Some initial remarks, before going into the review:
> 
>    - I see all sorts of licenses in the code: Apache, BSD, GPLv3, MIT... can
> you review this? The LGPLv3 for everything doesn't seem OK.
>   - Here seems to be some bundled, minified javascript libraries such as
> react. Please refer to https://fedoraproject.org/wiki/Packaging:JavaScript. 
> 
> Here is also things like (./odoo/addons/l10n_cr/__init__.py:)

Yeah, I found a lot of stuff to unbundle: fonts, js, python-code  :/


> At top;
> # Part of Odoo. See LICENSE file for full copyright and licensing details.
> 
> line 8-9
> #    First author: Carlos Vásquez <carlos.vasquez@clearcorp.co.cr>
> (ClearCorp S.A.)
> #    Copyright (c) 2010-TODAY ClearCorp S.A. (http://clearcorp.co.cr). All
> rights reserved
> 
> To me, this doesn't make sense. It looks like the author has claimed all
> rights, and published under a MIT license. And then odoo has pasted there
> own license on top, more or less hijacking the code. I am not a lawyer, but
> this looks fishy. Thoughts?

For the MIT-License it is fine this way, since this license explicitly allows relicensing / sublicensing.  See fulltext: https://opensource.org/licenses/MIT

Comment 7 Alec Leamas 2016-10-27 08:17:43 UTC
Hm... note that the bundling guidelines have changed [1], bottom line is that you are not required to unbundle if the bundled code isn't packaged already.   For example, I'd think twice before unbundling some of this python code. The alternative is to bundle it properly with a virtual Provides: etc.

From a review standpoint I'm mostly concerned with the fonts and large, standard javascript libraries such as jquery and react; I think these should unbundled. I'm also surprised that the installation code doesn't minify the javascript libs. Here are examples of react (again) which is 650k, this is  a lot to download in a webpage. It should be minified in production, preferably in a patch which could be upstreamed. The proper solution would be to use some bundling tool like webpack or so, but this is probably beyond the scope of packaging.

I'm also concerned with the licenses. The list produced by fedora-review is a good starter for a working license break-down.


[1] https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:Bundled_Libraries

Comment 8 Alec Leamas 2016-12-07 18:06:53 UTC
Ping?!

Comment 9 Alec Leamas 2017-01-27 12:10:06 UTC
This review has been stalled for too long. Could you please provide som feedback here?

Just to be clear: this is the first step in the stalled reviews policy handling

Comment 10 Alec Leamas 2017-02-08 13:10:29 UTC
Closing as per policy for stalled reviews. I'm open to resurrect it.


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