Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: lessfs - Lessfs is an inline data deduplicating filesystem.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Summary: Review Request: lessfs - Lessfs is an inline data deduplicating filesystem. Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: maxamillion@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.7.5-3.src.rpm Description: Lessfs is an inline data deduplicating filesystem.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |lemenkov@gmail.com
--- Comment #1 from Peter Lemenkov lemenkov@gmail.com 2009-10-23 05:52:09 EDT --- Missing "Requires: fuse"
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jussi.lehtola@iki.fi
--- Comment #2 from Jussi Lehtola jussi.lehtola@iki.fi 2009-10-23 06:03:35 EDT --- You're missing the URL. Also, run rpmlint since at least the summary is not fine (it ends with a dot).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #3 from Adam Miller maxamillion@fedoraproject.org 2009-10-23 10:03:02 EDT --- [08:59:26][adam@linOS][SPECS]+ rpmlint lessfs.spec lessfs.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 7, tab: line 1) 0 packages and 1 specfiles checked; 0 errors, 1 warnings.
Fixed both mentions, complete oversight on my part. Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.7.5-4.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #4 from Adam Miller maxamillion@fedoraproject.org 2009-10-28 09:41:18 EDT --- New upstream release and added the %config macro to the config file.
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.8.0-1.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #5 from Adam Miller maxamillion@fedoraproject.org 2009-10-28 11:42:21 EDT --- Made a config change to the spec file, upstream suggested an added configure flag to enable a new feature that offers a 25% performance gain. This has been included in the build.
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.8.0-2.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #6 from Michael Schwendt mschwendt@gmail.com 2009-11-04 06:41:20 EDT --- * The %description could be a bit more verbose than the %summary in explaining what this package supplies.
* In the %summary it is good practise to not repeat %name and reduce the summary to headline/title-style: "Inline data deduplicating filesystem"
export CFLAGS="-ggdb2 -O2"
No comment on why Fedora's global %optflags are not being used?
rm -rf %{buildroot}%{_datadir}/%{name}
Such activity asks for a comment in the .spec file.
%post -p /sbin/ldconfig %postun -p /sbin/ldconfig
No shared library are included in this package.
%{_mandir}/man1/lessfs.1.gz
Typically, the '*' wildcard is used instead of hardcoding the gzip compression suffix ".gz". That way rpmbuild may change the compression technique without requiring spec updates.
/etc/init.d/lessfs
An initscript without corresponding chkconfig/service scriptlets in the .spec file?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #7 from Adam Miller maxamillion@fedoraproject.org 2009-11-09 13:23:28 EDT --- Fixed the mentions listed in the last comment, also a new upstream release.
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.8.1-1.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #8 from Adam Miller maxamillion@fedoraproject.org 2009-11-09 15:17:57 EDT --- New upstream release today.
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.8.2-1.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |pahan@hubbitus.info AssignedTo|nobody@fedoraproject.org |pahan@hubbitus.info Flag| |fedora-review?
--- Comment #9 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2009-11-15 04:06:17 EDT --- If no one argue I'll review it.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #10 from Adam Miller maxamillion@fedoraproject.org 2009-11-16 10:42:42 EDT --- New upstream release yesterday.
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.8.3-1.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #11 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2009-11-16 14:17:50 EDT --- Rpmlint is verbose: $ rpmlint * lessfs.src: W: name-repeated-in-summary Lessfs lessfs.src:97: W: macro-in-%changelog %config lessfs.src: W: mixed-use-of-spaces-and-tabs (spaces: line 7, tab: line 1) lessfs.spec:97: W: macro-in-%changelog %config lessfs.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 7, tab: line 1) lessfs-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/lessfs-0.8.2/lessfsck.c lessfs.i686: W: name-repeated-in-summary Lessfs lessfs.i686: W: dangling-symlink /usr/share/doc/lessfs-0.8.2/COPYING /usr/share/automake-1.10/COPYING lessfs.i686: W: spurious-executable-perm /usr/share/doc/lessfs-0.8.2/ChangeLog lessfs.i686: E: executable-marked-as-config-file /etc/lessfs.cfg lessfs.i686: E: script-without-shebang /etc/lessfs.cfg lessfs.i686: E: no-status-entry /etc/init.d/lessfs lessfs.i686: W: no-reload-entry /etc/init.d/lessfs 3 packages and 1 specfiles checked; 3 errors, 10 warnings.
Please fix it before formal review starts.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #12 from Adam Miller maxamillion@fedoraproject.org 2009-11-17 11:44:39 EDT --- Fixed the rpmlint issues aside from the mixed tabs and spaces one, I can't seem to find what will make that one happy.
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.8.3-2.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #13 from Jussi Lehtola jussi.lehtola@iki.fi 2009-11-17 11:53:27 EDT --- As rpmlint complains, you have tabs in the beginning and spaces starting from line 7. Either change all tab characters into spaces, or vice versa.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #14 from Adam Miller maxamillion@fedoraproject.org 2009-11-17 14:14:51 EDT --- (In reply to comment #13)
As rpmlint complains, you have tabs in the beginning and spaces starting from line 7. Either change all tab characters into spaces, or vice versa.
Yes, but when I edited line 7 then it said some other line was causing the inconsistency and as I recall this is not a blocker for review so I just left it for sake of personal sanity.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #15 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2009-11-18 16:49:00 EDT --- Yes, it is because you really use mesh of tabs and spaces. I don't see any problem convert all spaces to tabs, for example: http://hubbitus.fedorapeople.org/lessfs.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #16 from Adam Miller maxamillion@fedoraproject.org 2009-11-19 09:50:16 EDT --- [08:39:15][adam@turnip][Desktop]+ wget -c http://hubbitus.fedorapeople.org/lessfs.spec --2009-11-19 08:39:18-- http://hubbitus.fedorapeople.org/lessfs.spec Resolving hubbitus.fedorapeople.org... 128.197.185.45 Connecting to hubbitus.fedorapeople.org|128.197.185.45|:80... connected. HTTP request sent, awaiting response... 200 OK Length: 9375 (9.2K) [text/plain] Saving to: `lessfs.spec'
100%[======================================>] 9,375 --.-K/s in 0.04s
2009-11-19 08:39:18 (220 KB/s) - `lessfs.spec' saved [9375/9375]
[08:40:17][adam@turnip][Desktop]+ rpmlint lessfs.spec lessfs.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 63, tab: line 1) 0 packages and 1 specfiles checked; 0 errors, 1 warnings.
---------------------------------------------- As much fun as it is being petty over this, I'd like to move on with the review of the package. I've fixed the spaces/tabs issue. ----------------------------------------------
[08:46:53][adam@turnip][SPECS]+ wget -c http://maxamillion.fedorapeople.org/lessfs.spec --2009-11-19 08:47:09-- http://maxamillion.fedorapeople.org/lessfs.spec Resolving maxamillion.fedorapeople.org... 128.197.185.45 Connecting to maxamillion.fedorapeople.org|128.197.185.45|:80... connected. HTTP request sent, awaiting response... 200 OK Length: 9487 (9.3K) [text/plain] Saving to: `lessfs.spec'
100%[======================================>] 9,487 --.-K/s in 0.03s
2009-11-19 08:47:09 (292 KB/s) - `lessfs.spec' saved [9487/9487]
[08:47:09][adam@turnip][SPECS]+ rpmlint lessfs.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings.
------------------------- For the record though, this has never been a violation during review before. So if the policy changed then I do apologize and would like to politely request a reference to that which I had previously violated. Thank you.
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.8.3-3.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #17 from Adam Miller maxamillion@fedoraproject.org 2009-12-10 17:17:56 EDT --- Updated to 0.9.0 from upstream, the SRPM name is a little different because I built in mock this go around.
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.9.0-1.fc12.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #18 from Adam Miller maxamillion@fedoraproject.org 2009-12-17 15:00:25 EDT --- New release from upstream
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-0.9.4-1.fc12.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #19 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2009-12-20 18:59:34 EDT --- I'm very-very apologize for the big delay. Circumstances beyond my control, but I'm just trying to fight them ...
Long awaited review: Legend: + - Ok. - - Error. +/- - It item acceptable, but I strongly recommend enhancement. = - N/A. MUST Items
[+] MUST: rpmlint must be run on every package. The output should be posted in the review. $ rpmlint * 3 packages and 1 specfiles checked; 0 errors, 0 warnings.
[+] MUST: The package must be named according to the Package Naming Guidelines. [+] MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption. [-] MUST: The package must meet the Packaging Guidelines. https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_... Patch0 has not any comment.
[+] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. [-] MUST: The License field in the package spec file must match the actual license. Comment in lessfs.c say: You can redistribute lessfs and/or modify it under the terms of either (1) the GNU General Public License; either version 3 of the or (at your option) any later version as published by the Free Software Foundation.
So, license of package may be GPLv3+. Why you boundary it by GPLv3?
[-] 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 must be included in %doc.
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text If the source package does not include the text of the license(s), the packager should contact upstream and encourage them to correct this mistake.
Text of license is not includes. Do you try ask maintainer include it?
[+] MUST: The spec file must be written in American English. [+] MUST: The spec file for the package MUST be legible. [+] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. $ md5sum lessfs-0.9.4.tar.gz lessfs-0.9.4.tar.gz.downloaded c4c5dbe234dc026bbba7945dc14f8305 lessfs-0.9.4.tar.gz c4c5dbe234dc026bbba7945dc14f8305 lessfs-0.9.4.tar.gz.downloaded
[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. http://koji.fedoraproject.org/koji/taskinfo?taskID=1882307
[+] MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense. [=] MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden. [=] MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. [+] MUST: Packages must NOT bundle copies of system libraries. [=] MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. Without this, use of Prefix: /usr is considered a blocker. [+] MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory. [+] MUST: A Fedora package must not list a file more than once in the spec file's %files listings. [+] MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. [+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [-] MUST: Each package must consistently use macros. Do not use direct path /etc/lessfs.cfg, /etc/init.d/lessfs . Macros like%{_sysconfdir} must be used.
[-] MUST: The package must contain code, or permissable content. lib_qlz.{c,h}: // QuickLZ data compression library // Copyright (C) 2006-2007 Lasse Mikkel Reinhold // lar@quicklz.com // // QuickLZ can be used for free under the GPL-1 or GPL-2 license (where anything. // released into public must be open source) or under a commercial license if such. // has been acquired (see http://www.quicklz.com/order.html). The commercial license. // does not cover derived or ported versions created by third parties under GPL.
lib_BMW.c, lib_sboxes.c, lib_net.{h,c}, lib_tiger.c, listdb.c, have not any mention of license and require further clarification.
miniacc.h, portab.h, portab_a.h : /* ACC --- Automatic Compiler Configuration This file is part of the LZO real-time data compression library.
There also issue what it is library and must be packaged separately - https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
[=] MUST: Large documentation files must go in a -doc subpackage. (The definition of large is left up to the packager's best judgement, but is not restricted to size. Large can refer to either size or quantity). [+] MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. [+] MUST: Header files must be in a -devel package. [=] MUST: Static libraries must be in a -static package. [=] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). [=] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. [=] MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} [+] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. [=] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. [+] MUST: Packages must not own files or directories already owned by other packages. The rule of thumb here is that the first package to be installed should own the files or directories that other packages may rely upon. This means, for example, that no package in Fedora should ever share ownership with any of the files or directories owned by the filesystem or man package. If you feel that you have a good reason to own a file or directory that another package owns, then please present that at package review time. [+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [+] MUST: All filenames in rpm packages must be valid UTF-8.
SHOULD Items:
[=] SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. [=] SHOULD: The reviewer should test that the package builds in mock. http://koji.fedoraproject.org/koji/taskinfo?taskID=1882307 [+] SHOULD: The package should compile and build into binary rpms on all supported architectures. [+] SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example. Its runs. I do not test anything other.
[+] SHOULD: If scriptlets are used, those scriptlets must be sane. This is vague, and left up to the reviewers judgement to determine sanity. [=] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. [=] SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb. [=] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
Other issues: 1) It fails build on epel5 - http://koji.fedoraproject.org/koji/taskinfo?taskID=1882363 due to old fuse. If you wish maintain lessfs for epel too it need attention. If it designed only for Fedora, BuildRoot tag is ambiguous and deprecated. 2) I strongly recommend include AUTHORS (or authors) file into %doc. And may be README.crypto, README.file_io, README.hashes, README.performance_or_suffer too.
My fault.
P.S. Again sorry for delay.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #21 from Adam Miller maxamillion@fedoraproject.org 2010-01-14 18:19:22 EST --- Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-1.0.0-3.fc12.src.rpm
Sorry, a last minute edit that resulted in a typo which caused the build to fail with a lz error. I had forgotten to build lessfs-1.0.0-2.fc12.src.rpm in mock before uploading but did so with lessfs-1.0.0-3.fc12.src.rpm and it looks good.
-AdamM
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #22 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2010-01-21 13:51:09 EST --- lessfs.src: W: mixed-use-of-spaces-and-tabs (spaces: line 19, tab: line 1) lessfs.i586: W: unstripped-binary-or-object /usr/bin/lessfs 1) lessfs.i586: W: unstripped-binary-or-object /usr/sbin/listdb lessfs.i586: W: unstripped-binary-or-object /usr/sbin/mklessfs lessfs.i586: W: unstripped-binary-or-object /usr/sbin/defrag_lessfs lessfs.i586: W: unstripped-binary-or-object /usr/sbin/lessfsck lessfs.i586: W: spurious-executable-perm /usr/share/doc/lessfs-1.0.0/authors lessfs.i586: W: spurious-executable-perm /usr/share/doc/lessfs-1.0.0/README.crypto lessfs.i586: W: spurious-executable-perm /usr/share/doc/lessfs-1.0.0/copying
2) Still used direct path in several places: # install SYSV init stuff mkdir -p %{buildroot}/etc/rc.d/init.d install -m755 etc/lessfs %{buildroot}/etc/rc.d/init.d/lessfs
install -D -m 644 etc/lessfs.cfg %{buildroot}/etc/lessfs.cfg
3) lib_qlz.{c,h} still in tarball. I saw BR lzo-devel and --with-lzo configure parameter, and if you aree shure it is not used anymore it should be deleted in %prep.
4) lib_BMW.c, lib_sboxes.c, lib_net.{h,c}, lib_tiger.c, listdb.c became part of lessfs and not really libraries? It is clarified by upstream author?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #23 from Adam Miller maxamillion@fedoraproject.org 2010-01-26 16:06:25 EST --- Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-1.0.0-4.fc12.src.rpm
1) I'm pretty sure I got all these (at least I did according to my system): [15:01:17][adam@turnip][SPECS]+ rpmlint ../SRPMS/lessfs-1.0.0-4.fc12.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
2) Fixed
3) Those were actually still being compiled but not linked to, patched out of the Makefile.in as well as removed in %spec now.
4) These libraries were said to be of original work and part of the lessfs application as per upstream author.
-AdamM
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #24 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2010-01-27 14:17:13 EST --- 1) You check only src.rpm package, but binary should be checked too: $ rpmlint * lessfs.i586: W: spurious-executable-perm /usr/share/doc/lessfs-1.0.0/authors lessfs.i586: W: spurious-executable-perm /usr/share/doc/lessfs-1.0.0/copying
2,3,4 Ok.
5) Now in package 2 patches. Did you inform upstream developer about your patches? https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_...
Only for note: mkdir -p %{buildroot}/%{_initddir} install -m755 etc/lessfs %{buildroot}/%{_initddir}/lessfs
May be replaced by just install with -D: install -D -m755 etc/lessfs %{buildroot}/%{_initddir}/lessfs but off course it is not error.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #25 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2010-01-27 15:12:55 EST --- 6) Also it is failed biuld for EPEL due to the old FUSE. http://koji.fedoraproject.org/koji/taskinfo?taskID=1948893
It needs additional attention.
If you do not plan maintain it for EPEL, then BuildRoot tag in spec is ambiguous: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #26 from Matthew Miller mattdm@mattdm.org 2010-02-02 10:08:26 EST --- Note that the 1.0.1 upstream tarball works with the current spec file with no changes. (Patches have not yet made it back upstream.)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #27 from Adam Miller maxamillion@fedoraproject.org 2010-02-02 10:13:36 EST --- Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-1.0.0-5.fc12.src.rpm
1) [09:01:54][adam@turnip][SPECS]+ rpmlint /var/lib/mock/fedora-12-i386/result/lessfs-1.0.0-5.fc12.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Sorry for the oversight, should be good this time.
5) I submitted patch0 to upstream around the 0.8.3 release but it still does not appear to be accepted or applied. I didn't submit Patch1 as upstream has expressed the desire to continue to integrate QuickLZ for performance reasons and I didn't feel the patch was applicable outside of the Fedora package.
6) I don't currently have plans to branch for EPEL but if I do in the future I will work with upstream.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #28 from Matthew Miller mattdm@mattdm.org 2010-02-02 10:41:44 EST --- Looks like qlz is available under the GPL-3, although the upstream packaging is, um, minimal in its license statements. I think a _better_ patch would make it a ./configure option, but I don't think the current state should be a blocker on getting the package in Fedora.
On another note, though: the /etc/lessfs.cfg file is really a _per filesystem_ configuration, not a global one. This is somewhat strange. This is probably a discussion for upstream, but it's important enough that it should get resolved before stuff starts going into production. Instead of /etc/lessfs.cfg, I think there should be /etc/lessfs/sample.cfg, and that should be configured with sensible defaults with the backing storage in (in /var/lib/lessfs/sample, maybe?) instead of the current default "/data".
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #29 from Matthew Miller mattdm@mattdm.org 2010-02-02 21:51:07 EST --- See:
http://www.lessfs.com/wordpress/?p=302&cpage=1#comment-400
Upstream developer gives a thumbs-up to the /etc/lessfs/sample.cfg concept.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #30 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2010-02-13 12:38:32 EST --- 5) Ok, an this note about upstream state is higly appreciated. I saw it in Patch0 (link to public bugtracker is even good, if it mailed - word about it).
Patch1 have not such note at all. If it is completely Fedora-related Patch - no problem, just mention it in two words too.
6) No, then no. Ok.
(In reply to comment #28)
Looks like qlz is available under the GPL-3, although the upstream packaging is, um, minimal in its license statements. I think a _better_ patch would make it a ./configure option, but I don't think the current state should be a blocker on getting the package in Fedora.
In most cases such modifications of course good idea make more common. But in our case it is Fedora-specific short cut-off patch and If such configuration switch is not interesting for upstream developer I think it absolutely have no worth for us in packaging.
On another note, though: the /etc/lessfs.cfg file is really a _per filesystem_ configuration, not a global one. This is somewhat strange. This is probably a discussion for upstream, but it's important enough that it should get resolved before stuff starts going into production. Instead of /etc/lessfs.cfg, I think there should be /etc/lessfs/sample.cfg, and that should be configured with sensible defaults with the backing storage in (in /var/lib/lessfs/sample, maybe?) instead of the current default "/data".
Yes. This is good mention. /data is not fit in any hierarhial standards. Package also do not create and do not own it! If package try use it package it provided must be required. I think there one more patch and defaults tuning according our hier policy required.
Matthew Miller, thanks for note.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #31 from Adam Miller maxamillion@fedoraproject.org 2010-02-16 15:09:15 EST --- Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec SRPM URL: http://maxamillion.fedorapeople.org/lessfs-1.0.0-6.fc12.src.rpm
I took into consideration the suggestion that we move to a /etc/lessfs/sample.cfg and applied that in this version of the spec file.
5) I have added a comment to the effect that the patch is fedora specific, and s far as the note about the patch being sent upstream, I only mailed direct from me to the upstream developer (afaik, its just one person) so I am unable to post a link to the effect.
-AdamM
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #32 from Matthew Miller mattdm@mattdm.org 2010-02-16 19:12:51 EST --- For better or for worse, the main path for development interaction appears to be comments on the developer's blog.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #33 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2010-02-21 17:39:08 EST --- /etc/lessfs/sample.cfg also refer to /data/mta but package does not create and own such directory!
(In reply to comment #32)
For better or for worse, the main path for development interaction appears to be comments on the developer's blog.
It is a good news I think. Can you provide link?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #34 from Matthew Miller mattdm@mattdm.org 2010-02-21 19:27:32 EST --- (In reply to comment #33)
/etc/lessfs/sample.cfg also refer to /data/mta but package does not create and own such directory!
And it probably shouldn't. I think we should change it to /var/lib/lessfs/sample, as mentioned above.
(In reply to comment #32)
For better or for worse, the main path for development interaction appears to be comments on the developer's blog.
It is a good news I think. Can you provide link?
http://www.lessfs.com/wordpress/
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #35 from Adam Miller maxamillion@fedoraproject.org 2010-02-25 17:22:28 EST --- (In reply to comment #33)
/etc/lessfs/sample.cfg also refer to /data/mta but package does not create and own such directory!
If it is just a sample configuration, does that package really need to create and own all mentioned components of the sample configuration?
(In reply to comment #32)
For better or for worse, the main path for development interaction appears to be comments on the developer's blog.
It is a good news I think. Can you provide link?
As mentioned by Matthew, http://www.lessfs.com/wordpress/ <--- do we want this somewhere in the spec file in respect to the sample configuration?
-AdamM
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #36 from Matthew Miller mattdm@mattdm.org 2010-02-25 17:27:54 EST --- If it's _purely_ a sample, it should go in the /usr/share/docs/lessfs-*/sample.cfg.
But I think it's probably reasonable to ship a functional sample and create the /var/lib/lessfs/sample, at the very least because that encourages people to put backing stores for non-sample filesystems in a sensible place (/var/lib/lessfs/myownfilesystem, for example). However, I don't feel particularly strongly about this.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |michael@michaelrice.org
--- Comment #37 from Kevin Fenzi kevin@tummy.com 2010-04-17 21:31:19 EDT --- *** Bug 583346 has been marked as a duplicate of this bug. ***
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #38 from Michael Rice michael@michaelrice.org 2010-04-17 21:50:43 EDT --- Adam, Are you still working on getting this in? Would you like any help? I made the mistake of making a package before checking bugzilla and I am interested in helping if you need it.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #39 from Adam Miller maxamillion@fedoraproject.org 2010-04-28 14:54:25 EDT --- Michael, I would enjoy having help, help is always good! Once this makes it through review I'll happily accept your co-ownership in pkgdb.
-AdamM
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #40 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2010-08-24 06:22:27 EDT --- ping?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Duncan Innes duncan@innes.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |duncan@innes.net
--- Comment #41 from Duncan Innes duncan@innes.net 2010-09-17 03:53:49 EDT --- Is there any movement on this guys? I can offer little but encouragement, but I'll do what I can to help. Functionality like this will provide big gains.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |654686(FE-FEATUREREVIEW)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?
--- Comment #42 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2010-11-19 06:26:12 EST --- Adam Miller, do you plan continue?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Adam Miller maxamillion@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |WONTFIX Flag|needinfo? | Last Closed| |2010-11-19 09:54:21
--- Comment #43 from Adam Miller maxamillion@fedoraproject.org 2010-11-19 09:54:21 EST --- The review somewhat fell off the grid and I have since lost track of upstream changes. I pulled the latest release and it appears its going to require some heavy patching to comply with Fedora Guidelines purely because of the embedded static libraries that are shipped with lessfs (I have spoken with upstream about this and this is their preferred method because of performance of the lib in question). I just started a new job and my free time is not what it once was so I lack the time to look into accomplishing the needed patching and will be closing this review request, if anyone would like to take it on and continue then feel free to either reopen the bug or open a new one and as always the spec and patches are fair game for anyone they might be of some use for.
Good luck and happy hacking.
-AdamM
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|654686(FE-FEATUREREVIEW) |201449(FE-DEADREVIEW) Flag|fedora-review? |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
Chris Cowley chris@chriscowley.me.uk changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@chriscowley.me.uk
--- Comment #44 from Chris Cowley chris@chriscowley.me.uk 2011-10-07 09:26:23 EDT --- I have been looking at LessFS in my day job. As such I have built some packages for my own use.
I have made it available from my personal yum rep (http://yum.chriscowley.me.uk/el/6/SRPMS/repoview/)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=530473
--- Comment #45 from Pavel Alexeev (aka Pahan-Hubbitus) pahan@hubbitus.info 2011-10-16 12:35:50 EDT --- Chris, if you willing maintain it in Fedora - feel free to open new review request!
package-review@lists.fedoraproject.org