Bug 530473

Summary: Review Request: lessfs - Lessfs is an inline data deduplicating filesystem.
Product: [Fedora] Fedora Reporter: Adam Miller <maxamillion>
Component: Package ReviewAssignee: Pavel Alexeev <pahan>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: chris, duncan, fedora-package-review, i, lemenkov, mattdm, michael.hagmann, michael, notting, pahan, renich, susi.lehtola
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-11-19 09:54:21 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 201449    

Description Adam Miller 2009-10-22 20:10:59 EDT
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.
Comment 1 Peter Lemenkov 2009-10-23 05:52:09 EDT
Missing "Requires: fuse"
Comment 2 Susi Lehtola 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).
Comment 3 Adam Miller 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
Comment 4 Adam Miller 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
Comment 5 Adam Miller 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
Comment 6 Michael Schwendt 2009-11-04 06:41:20 EST
* 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?
Comment 7 Adam Miller 2009-11-09 13:23:28 EST
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
Comment 8 Adam Miller 2009-11-09 15:17:57 EST
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
Comment 9 Pavel Alexeev 2009-11-15 04:06:17 EST
If no one argue I'll review it.
Comment 10 Adam Miller 2009-11-16 10:42:42 EST
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
Comment 11 Pavel Alexeev 2009-11-16 14:17:50 EST
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.
Comment 12 Adam Miller 2009-11-17 11:44:39 EST
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
Comment 13 Susi Lehtola 2009-11-17 11:53:27 EST
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.
Comment 14 Adam Miller 2009-11-17 14:14:51 EST
(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.
Comment 15 Pavel Alexeev 2009-11-18 16:49:00 EST
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
Comment 16 Adam Miller 2009-11-19 09:50:16 EST
[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
Comment 17 Adam Miller 2009-12-10 17:17:56 EST
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
Comment 18 Adam Miller 2009-12-17 15:00:25 EST
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
Comment 19 Pavel Alexeev 2009-12-20 18:59:34 EST
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_an_upstream_bug_link_or_comment
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.
Comment 20 Adam Miller 2010-01-13 10:55:50 EST
Spec URL: http://maxamillion.fedorapeople.org/lessfs.spec
SRPM URL: http://maxamillion.fedorapeople.org/lessfs-1.0.0-2.fc12.src.rpm

Sorry it took me so long, I've been in contact with upstream about some of the issues as well as working with some people in #fedora-devel on how to best approach the QuickLZ issue.

I'm pretty sure I took care of all concerns from the last comment. Looking forward to further review.

-AdamM
Comment 21 Adam Miller 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
Comment 22 Pavel Alexeev 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?
Comment 23 Adam Miller 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
Comment 24 Pavel Alexeev 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_an_upstream_bug_link_or_comment


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.
Comment 25 Pavel Alexeev 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
Comment 26 Matthew Miller 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.)
Comment 27 Adam Miller 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.
Comment 28 Matthew Miller 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".
Comment 29 Matthew Miller 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.
Comment 30 Pavel Alexeev 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.
Comment 31 Adam Miller 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
Comment 32 Matthew Miller 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.
Comment 33 Pavel Alexeev 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?
Comment 34 Matthew Miller 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/
Comment 35 Adam Miller 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
Comment 36 Matthew Miller 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.
Comment 37 Kevin Fenzi 2010-04-17 21:31:19 EDT
*** Bug 583346 has been marked as a duplicate of this bug. ***
Comment 38 Michael Rice 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.
Comment 39 Adam Miller 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
Comment 40 Pavel Alexeev 2010-08-24 06:22:27 EDT
ping?
Comment 41 Duncan Innes 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.
Comment 42 Pavel Alexeev 2010-11-19 06:26:12 EST
Adam Miller, do you plan continue?
Comment 43 Adam Miller 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
Comment 44 Chris Cowley 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/)
Comment 45 Pavel Alexeev 2011-10-16 12:35:50 EDT
Chris, if you willing maintain it in Fedora - feel free to open new review request!
Comment 46 Renich Bon Ciric 2012-05-19 03:36:43 EDT
ping

What's up with this package? Is it coming through?
Comment 47 Chris Cowley 2012-05-21 15:31:30 EDT
Current package is in my Yum repo:
http://yum.chriscowley.me.uk/el/6/SRPMS/repoview/lessfs.html

So far have not put it to use at the day job, but might put it into my lab system over the next few weeks. Unfortunately I had hoped to use it behind GlusterFS, which requires xattrs.
Comment 48 Pavel Alexeev 2012-05-27 11:55:11 EDT
New review: https://bugzilla.redhat.com/show_bug.cgi?id=823661
Comment 49 Christopher Meng 2014-02-09 21:34:05 EST

*** This bug has been marked as a duplicate of bug 823661 ***