Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.7-1.fc17.src.rpm Description: ownCloud Server provides you a private file sync and share cloud. Host this server to easily sync business or private documents across all your devices, and share those documents with other users of your ownCloud server on their devices. Fedora Account System Username: brummbq Gotchas: owncloud is bundling jquery, but I suppose since basically every webapp in fedora is doing the same we can ignore this. the web server integration is non-existent, I'm planning to create separate subpackages for apache and nginx (though no idea how to configure nginx) currently selinux is preventing write access on /etc/owncloud, need to fill a bug against selinux-policy some php deps are also missing in Fedora probably a lot more...
Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.7-2.fc17.src.rpm bump...feedback welcome :)
Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.7-3.fc17.src.rpm third iteration, I've added also database subpackages....so you don't have to install all db drivers anymore.
just a few drive-by comments: - is that main package functional without any of those database sub-packages? - you also need to require rsyslog (for dir /etc/rsyslog.d), note: there's also syslog-ng, you're fine to uninstall rsyslog from your system - same for that logrotate-snippet - the svg-editor is also LGPLv3 and MIT https://github.com/owncloud/apps/blob/master/files_svgedit/README http://fedoraproject.org/wiki/Licensing#Good_Licenses lists, LGPLv3 is only sort of compatible with AGPL.
(In reply to comment #3) > just a few drive-by comments: > - is that main package functional without any of those database > sub-packages? No, not really. Though you can't install owncloud without at least one database sub-package because the main pkg requires: %{name}-database = %{version}-%{release} > - you also need to require rsyslog (for dir /etc/rsyslog.d), > note: there's also syslog-ng, you're fine to uninstall rsyslog from your > system I think I will switch back to the internal logging mechanism of owncloud. It doesn't require any kind of syslog, so we don't have to bother with the correct dependencies :) > - the svg-editor is also LGPLv3 and MIT oc 4.0 does not yet include svg-edit, but I've added a note thanks for the comment
Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.7-4.fc17.src.rpm small update
Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.7-4.fc17.src.rpm -unbundle php-getid3 -fixes selinux error in combination with latest selinux-policy
correct links: Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.7-5.fc17.src.rpm
I'll review this this weekend during FUDCon. If somebody else is quicker, go ahead.
owncloud-4.0.8 Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.8-1.fc17.src.rpm (In reply to comment #8) > I'll review this this weekend during FUDCon. maybe *this* weekend? ;)
Maybe you want to concentrate on the 4.0.x series, however 4.5.0 has been released on October, 11th.
I'm planning to stick to the 4.0.x series for now and update to 4.5 post-review, provided that this review doesn't take forever... btw still in the need of a reviewer for https://bugzilla.redhat.com/show_bug.cgi?id=859713
Created attachment 637034 [details] copyright file extracted from debian owncloud package Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.8-2.fc17.src.rpm this is for the legal folks: I've updated the license field in the spec. Is it ok? Am I missing something important? I've attached also the copyright file from debian. They have searched through the source code and listed all licenses, I suppose. Note that most of the 3rdparty libs are stripped out at build time with exception of phpass and the js stuff.
One big showstopper: - jslint rears its ugly head again. apps/files_texteditor/js/aceeditor/worker-javascript.js contains the jslint code (well, the jshint derived code) and that license is non-free because of the infamous "The Software shall be used for Good, not Evil." No code under that license is Free, and is not acceptable for Fedora. I'm not sure if there is a way to remove this portion of the code (or maybe just the aceeditor code altogether), but it cannot go into Fedora as is. (And we know from previous attempts that Douglas will not waive the problematic clause.) A few minor notes: - There are other aceeditor js files which are (MPLv1.1 or GPLv2+ or LGPLv2+). - some of the jquery plugins are (MIT or GPL+) - others (timepicker) are (MIT or GPLv2) - The ODF icons are ASL 2.0 [see: core/img/filetypes/readme-2.txt] - The Silk icons are CC-BY - The kde-look icons are CC-BY-SA All of those licenses need to be reflected in the License tag (the ones in parenthesis need to be listed with their parenthesis). ****** I realize that it is a moot point if the jslint issue is not resolved, but I figured I'd go ahead and finish looking for licenses while I was in there. Leaving blocked on FE-Legal.
I think the simplest fix would be to rip out the JavaScript mode from aceeditor, or at least the parts that use jshint (in lib/ace/mode/javascript-worker.js, and of course lib/ace/mode/javascript/jshint.js needs to be zapped entirely).
(The file names in comment #14 are from current upstream, it looks like the bundled code is older and uses different file names.) FWIW, the ace editor is arguably in violation of the JSLint/JSHint license, considering that: * according to the license, "The Software shall be used for Good, not Evil." * according to http://www.jslint.com/lint.html , "eval is evil" * https://github.com/ajaxorg/ace/blob/master/lib/ace/mode/javascript_worker.js contains both a call to JSHint and the following: eval("throw 0;" + str); :-)
spec file with longest license tag ever: Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.8-3.fc17.src.rpm (In reply to comment #14) > I think the simplest fix would be to rip out the JavaScript mode from > aceeditor Well, some other modes are also using that jslint-javascript-worker. I've just deleted those files, which means that the syntax highlighting doesn't work any more for these file types. But you are still able to edit them. If you can come up with a better patch, feel free to attach it.
A patch is not suitable to delete unacceptably-licensed code, you should repackage the tarball to get rid of it entirely.
So, one issue is that the bundled old version of the ace editor bundles JSHint into the same file as the rest of the JavaScript worker. In the current upstream code, it is a separate file, which would be much easier to get rid of without removing other functionality (rm the file from the tarball, then apply a patch to remove its uses). With this code, the repackaging script will need to use some sed or awk to remove the offending lines by number (because it cannot mention the code to be removed if it is to be shipped in the SRPM), then a patch can be applied. (Or you have to delete the whole worker_javascript.js as you did now with the patch and then deal with the fallout, but that breaks a lot more functionality than needed. :-( )
(In reply to comment #17) > A patch is not suitable to delete unacceptably-licensed code, you should > repackage the tarball to get rid of it entirely. Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.8-4.fc17.src.rpm (In reply to comment #18) > So, one issue is that the bundled old version of the ace editor bundles > JSHint into the same file as the rest of the JavaScript worker. In the > current upstream code, it is a separate file, which would be much easier to > get rid of without removing other functionality (rm the file from the > tarball, then apply a patch to remove its uses). Are you suggesting to replace the bundled aceeditor with the current upstream one? Another issue is that I can't patch the compressed js files without recompressing them from the uncompressed files. Unless there is an js compression tool in fedora that I don't know of.
(In reply to comment #19) > Another issue is that I can't patch the compressed js files without > recompressing them from the uncompressed files. Unless there is an js > compression tool in fedora that I don't know of. That seems bad in a practical sense from an open source point of view. If we can't reasonably patch the software, it's a big problem. I hate to add more blockers, but....
(In reply to comment #20) > That seems bad in a practical sense from an open source point of view. If we > can't reasonably patch the software, it's a big problem. I hate to add more > blockers, but.... Of course we could skip that minify step and serve uncompressed js files, though that would increase the net traffic. This would be as open source as it can be :) So I don't see a blocker here.
If the minify step consists of running jsMin, then I can only strongly recommend skipping that step, because jsMin is infected with the same silly "no Evil" license clause as jsLint/jsHint.
(In reply to comment #21) > Of course we could skip that minify step and serve uncompressed js files, > though that would increase the net traffic. This would be as open source as > it can be :) Let's do that, and then maybe eventually use yuicompressor?
(In reply to comment #23) > Let's do that, and then maybe eventually use yuicompressor? Thats the way debian does it. But as long as we don't need to modify js files and don't have a yuicompressor package this discussion is rather academic? ;)
Where is php-pear-MDB2-Driver-sqlite coming from? I don't see a review request....
php-pear-MDB2-Driver-sqlite requires php sqlite extension (v2) which is deprecated for years (pdo_sqlite and sqlite3 exists), moved from php 5.4 sources to pecl (but unmaintained), and removed from fedora (for years). So the sqlite sub package could probably be dropped.
Guys, did you spotted /owncloud/lib/MDB2/Driver/... ? They have their own sqlite driver included. I'm not sure where did that came from. Is it some sort of fork? If yes, I could move it to the owncloud-sqlite subpackage. So for the moment, I can just drop the req to php-pear-MDB2-Driver-sqlite without disabling sqlite (In reply to comment #26) > So the sqlite sub package could probably be dropped. We still want a sqlite subpackage in order to not pull in mysql or postgresql
(In reply to comment #27) > Guys, did you spotted /owncloud/lib/MDB2/Driver/... ? > They have their own sqlite driver included. I'm not sure where did that came > from. Is it some sort of fork? If yes, I could move it to the > owncloud-sqlite subpackage. I dunno. I'm just looking at the dependencies in the spec file. But if it's some sort of fork, we should look at whether it should be unbundled. > (In reply to comment #26) > > So the sqlite sub package could probably be dropped. > We still want a sqlite subpackage in order to not pull in mysql or postgresql +1
As I understand, this is not a fork, but a new driver (MDB2_Driver_sqlite3). Of course, it will be great to have this one propose as a new PEAR extension (we probably should encourage upstream to do this). So, for now, I don't see any need to unbundle something which is not bundled.
> I could move it to the owncloud-sqlite subpackage Yes. And please requires php-sqlite3
Created attachment 640645 [details] phpci-fullreport.txt Output result of phpci print --report full --recursive owncloud Note: This eport only detects "known" extensions (and there is some it doesn't know yet), and can have false positive. So according to it, requires should be (at least) php-curl php-date php-dom php-exif php-fileinfo php-filter (Warning: not yet available on RHEL) php-gd php-gmp php-iconv php-json php-ldap php-mbstring php-openssl php-pcre php-pdo php-session php-simplexml php-spl php-zip # optional php-bcmath Ignored : php-pgsql, php-mysql, php-sqlite3 moved to sub-package Ignored : php-xml (this extension is provided by php-common and is also a package which provides others extension... yes this is ugly) From the code, I don't see any need for php-posix. Note: it is better to request "extension" rather than "package", as this could change in the future.
So, for "sqlite" sub package (probably a better name would be sqlite3), requires should be. php-pcre php-sqlite3 php-pear(MDB2) Note : it is also preferable to use php-pear(xxx) rather than php-pear-xxx.
(In reply to comment #30) > > I could move it to the owncloud-sqlite subpackage > Yes. > > And please requires php-sqlite3 done, do you excuse, if I don't upload a new package for every little change another open point is 3rdparty/phpass. I'm hesitating to unbundle it, because it looks like an abandoned and unmaintained lib. The last release was over 2 years ago.
Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.8-5.fc17.src.rpm - reworked the php and pear dependencies - note also the new dist conditionals
Should config.sample.php go in %doc instead of the config directory? I don't have a strong feeling here; I think we're inconsistent as a distro and I don't see a guideline offhand. Shouldn't PEAR5.php and PEAR-LICENSE be removed along with the other pear files? And, I'm sorry for not saying this sooner, but I was re-reading the bundled libraries rules and I think we need to either split out or go through the bundled-library-exception process for the remaining stuff under 3rdparty. (A lot of it may fall under "copylibs" https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Copylibs) (I know you're working on jquery at bug #857992). This may have the benefit of making the License field less insane. :) On a less difficult note, maybe some of the text from http://owncloud.org/about/ could go in the description? I think it'd be nice for the description of the database subpackages to make clear that it's intentional for them to be empty. Alternately, they could contain doc files for configuration with that particular database? Not a review blocker.
(In reply to comment #35) > Should config.sample.php go in %doc instead of the config directory? I don't > have a strong feeling here; I think we're inconsistent as a distro and I > don't see a guideline offhand. agreed, moved to doc > > Shouldn't PEAR5.php and PEAR-LICENSE be removed along with the other pear > files? thanks, I've overlooked those > > And, I'm sorry for not saying this sooner, but I was re-reading the bundled > libraries rules and I think we need to either split out or go through the > bundled-library-exception process for the remaining stuff under 3rdparty. (A > lot of it may fall under "copylibs" > https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Copylibs) (I > know you're working on jquery at bug #857992). This may have the benefit of > making the License field less insane. :) okay let's see what we have left: 3 php libs: smb4php (single php class, newest release, 5 years old) php-when (single php class, 2 years old) phpass (2 files, probably dead upstream) How should I cope with them? The copylib rule doesn't apply here I suppose? Also there are 3 js-libs (chosen, fullcalender and timepicker) that I don't want to touch. I will try to get an exception... > > > > On a less difficult note, maybe some of the text from > http://owncloud.org/about/ could go in the description? aye > > > I think it'd be nice for the description of the database subpackages to make > clear that it's intentional for them to be empty. Alternately, they could > contain doc files for configuration with that particular database? Not a > review blocker. good idea, I'll write some database configuration instructions (actually you just need to create an user and a database)
here we go: Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.8-6.fc17.src.rpm - php-when and phpass (pronounced ph-pass ;)) are out FPC-ticket: https://fedorahosted.org/fpc/ticket/236 -> we don't need to remove the js-libs, there are no guidelines -> the php class smb4php is adjusted to owncloud, I wouldn't dare to replace it with a genuine
small update, properly handling of selinux context on rhel Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.0.8-7.fc17.src.rpm
Mathew, are you able/willing to continue on this pkg review? I heard rumors you might be busy, so I can help out if that's the case.
(In reply to comment #39) > Mathew, are you able/willing to continue on this pkg review? I heard rumors > you might be busy, so I can help out if that's the case. I think the rumors are kind of correct. If you've got spare cycles to take over, it's all yours!
jump to owncloud 4.5.x Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.5.7-1.fc17.src.rpm @rdieter: You still wanna help out?
OK, will carve out some time today. would appreciate any recipricol/swap reviews for anything blocking, https://bugzilla.redhat.com/show_bug.cgi?id=kde-reviews , qaccessibilityclient/simon or qt5-* stuff in particular.
naming: ok 1. Sources: ok but, # removed any occurences of jslint Source0: %{name}-%{version}-repack.tar.bz2 SHOULD: so tarball creation is readily reproducible and verifiable, please include more detail on what what excluded exactly or (ideally) include a script to do it. 2. scriptlets: mostly ok, but SHOULD: for f18+ consider following macro'ized systemd scriptlets per https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd builds/installs: ok 3. runtime: (apparently?) not OK. followed owncloud-README.fedora, installed, pointed browser at http://localhost/owncloud/, and get 403 Forbidden Error /var/log/httpd/error_log contained, [Sun Feb 24 16:21:25.387065 2013] [authz_core:error] [pid 32560] [client 127.0.0.1:49753] AH01630: client denied by server configuration: /usr/share/owncloud/ but, won't consider this a blocker for now (won't rule out user-error on my part), hopefully can tweak/polish things in the meantime to get this to work reliably. licensing: oh boy, looks accurate according to my quick verification with licensecheck tool, will leave this to FE-Legal blocker to be authoritative. That's my first pass looking this over. So far, no MUST blockers found. Feel free to humor me and address mentioned SHOULD items in the meantime.
Spec URL: http://brummbq.fedorapeople.org/owncloud.spec SRPM URL: http://brummbq.fedorapeople.org/owncloud-4.5.7-2.fc17.src.rpm (In reply to comment #43) > # removed any occurences of jslint > Source0: %{name}-%{version}-repack.tar.bz2 > > SHOULD: so tarball creation is readily reproducible and verifiable, please > include more detail on what what excluded exactly or (ideally) include a > script to do it. see new bash script in the srpm, am I supposed to ship that? > > > 2. scriptlets: mostly ok, but > > SHOULD: for f18+ consider following macro'ized systemd scriptlets per > https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd disagreed, I'm just reloading httpd in the sub-package. Those macros seem to do all kind of fancy stuff > > > 3. runtime: (apparently?) not OK. followed owncloud-README.fedora, > installed, pointed browser at http://localhost/owncloud/, and get 403 > Forbidden Error bad rdieter, you're using httpd 2.4 ;) have not tested on F18 yet, but should work now
1. as long as it's included in src.rpm and clearly documented what it's purpose is, fine with me. ps. don't mess with the bz review flag, that's for the reviewer only. :) ? = currently under review - = review failed + = review accepted Spot, any eta on clearing the FE-Legal blocker?
Sorry, jslint is gone now, so lifting FE-Legal.
Great! things look swell from my perspective, APPROVED. we can haggle any remaining runtime wrinkles post-review
New Package SCM Request ======================= Package Name: owncloud Short Description: Private file sync and share server Owners: brummbq Branches: f18 el6 InitialCC: thanks to everyone involved in this review
Git done (by process-git-requests).
owncloud-csync-0.70.4-1.fc18, mirall-1.2.1-2.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/owncloud-csync-0.70.4-1.fc18,mirall-1.2.1-2.fc18
imported into rawhide
Un-setting flag.
Package Change Request ====================== Package Name: owncloud New Branches: epel7 Owners: adamwill