Bug 433398 (Synbak)

Summary: Review Request: synbak - Synbak Universal Backup System (first package, need sponsor)
Product: [Fedora] Fedora Reporter: Ugo Viti <ugo.viti>
Component: Package ReviewAssignee: Jon Ciesla <limburgher>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: fedora, fedora-package-review, itamar, jonathan.underwood, limburgher, notting, pahan
Target Milestone: ---Flags: limburgher: fedora‑review?
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-05-06 09:22:06 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Ugo Viti 2008-02-18 19:42:13 EST
Spec URL: http://www.initzero.it/products/opensource/synbak/download/synbak.spec
SRPM URL: http://www.initzero.it/products/opensource/synbak/download/synbak-1.2.1-1.src.rpm
Description:
Synbak is an application designed to unify several backup methods.
Synbak provides a powerful reporting system and a very simple interface
for configuration files.
Synbak is a wrapper for several existing backup programs suppling the
end user with common method for configuration that will manage the
execution logic for every single backup and will give detailed reports
of backups result.

Synbak can make backups using:
- RSync over ssh, rsync daemon, smb and cifs protocols (automount functions)
- Tar archives (tar, tar.gz and tar.bz2)
- Tape devices (using multi loader changer tapes too)
- LDAP databases
- MySQL databases
- Oracle databases
- CD-RW/DVD-RW
- Wget to mirror HTTP/FTP servers

Synbak can make reports using:
- EMail
- HTML pages
- RSS feeds

Moreover, if you are a developer and want to contribute,
the modular nature of synbak will allow you to easly write
new backup methods, reports, and translations.
Comment 1 Guidolin Francesco 2008-02-19 03:05:31 EST
(this is an unofficial review)

[+] spec file seems correct
[+] rpmlint is silent
[+] package build in mock
[+] program works proprely
[-] you should use %{?dist} in Release tag:
Release:        1%{?dist}
Comment 2 Ugo Viti 2008-02-19 06:56:17 EST
Hi,

thanks for the reply...

Fixed the spec file and released build 2:

Spec URL: http://www.initzero.it/products/opensource/synbak/download/synbak.spec
SRPM URL:
http://www.initzero.it/products/opensource/synbak/download/synbak-1.2.1-2.fc8.src.rpm

Best Regards
Comment 3 Jon Ciesla 2008-09-19 10:31:23 EDT
Official review.  I am also willing to sponsor you, once this package is approved.  To that end, have you done any unoffical reviews of other's packages?  If so, please post links.  If not, do a few, and post links.

- MUST: rpmlint must be run on every package. The output should be posted in the review.

Clean.

- MUST: The package must be named according to the Package Naming Guidelines .

Good.

- MUST: The spec file name must match the base package %{name}, in the format %{name}.spec unless your package has an exemption on Package Naming Guidelines .

Good.

- MUST: The package must meet the Packaging Guidelines .

Good.

- MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines .

Good.

- MUST: The License field in the package spec file must match the actual license.

License tag should be GPLv2+ and CC-BY-SA.

- 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.

Good.

- MUST: The spec file must be written in American English.

Good.

- MUST: The spec file for the package MUST be legible. If the reviewer is unable to read the spec file, it will be impossible to perform a review. Fedora is not the place for entries into the Obfuscated Code Contest (http://www.ioccc.org/).

Good.  Might want to limit the description to the first 7 lines.  Your call.

- 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. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.

Good.

- MUST: The package must successfully compile and build into binary rpms on at least one supported architecture.

Good.

- MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch needs to have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number should then be placed in a comment, next to the corresponding ExcludeArch line. New packages will not have bugzilla entries during the review process, so they should put this description in the comment until the package is approved, then file the bugzilla entry, and replace the long explanation with the bug number. The bug should be marked as blocking one (or more) of the following bugs to simplify tracking such issues: FE-ExcludeArch-x86 , FE-ExcludeArch-x64 , FE-ExcludeArch-ppc , FE-ExcludeArch-ppc64

NA, noarch.

- 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.

Good.

- MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.

Good.

- MUST: Every binary RPM package which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. If the package has multiple subpackages with libraries, each subpackage should also have a %post/%postun section that calls /sbin/ldconfig. An example of the correct syntax for this is:

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

NA.

- 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.

NA.

- 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. Refer to the Guidelines for examples.

Good.

- MUST: A package must not contain any duplicate files in the %files listing.

Good.

- 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.

Good.

- MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ).

Good.

- MUST: Each package must consistently use macros, as described in the macros section of Packaging Guidelines .

Good.

- MUST: The package must contain code, or permissable content. This is described in detail in the code vs. content section of Packaging Guidelines .

Good.

- MUST: Large documentation files should 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)

NA.

- 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.

Good.

- MUST: Header files must be in a -devel package.

NA.

- MUST: Static libraries must be in a -static package.

NA.

- MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability).

NA.

- 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.

NA.

- MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}

NA.

- MUST: Packages must NOT contain any .la libtool archives, these should be removed in the spec.

Good.

- 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. This is described in detail in the desktop files section of the Packaging Guidelines . 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.

NA.

- 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.

NA.

- MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} ( or $RPM_BUILD_ROOT ). See Prepping BuildRoot For %install for details.

Good.

- MUST: All filenames in rpm packages must be valid UTF-8.

Good.


So, of that, just fix the license tag.

A few other things.  The INSTALL mentioned some mandatory programs.  

A: You should probably determine the rpms that provide these, and Require them.

B: Since you obviously cannot Require Oracle, you should either:

i. Create synback-fedora-README.txt explaining that Oracle support is present but won't work unless Oracle is installed.
ii. Create a patch to remove Oracle support.

My preference is ii., but either should be fine, and i. is simpler.
Comment 4 Ugo Viti 2008-09-22 15:53:11 EDT
> So, of that, just fix the license tag.
> A few other things.  The INSTALL mentioned some mandatory programs.  
> A: You should probably determine the rpms that provide these, and Require them.
> B: Since you obviously cannot Require Oracle, you should either:
> i. Create synback-fedora-README.txt explaining that Oracle support is present
> but won't work unless Oracle is installed.

Hi Jon,

I just released 1.2.2 version of synbak.

- Fixed the license tag (moved synbak from GPLv2 + GPLv3+)
- Included the README.Fedora (about oracle support licensing) file into doc dir
- Added all required extra rpm packages to the spec file

let's me know if it's ok now.

SRPM URL: http://www.initzero.it/products/opensource/synbak/download/synbak-1.2.2-1.fc9.src.rpm

the spec file is included into srpm of course, I must provide an external version?

Best Regards
Comment 5 Jon Ciesla 2008-09-22 16:19:40 EDT
(In reply to comment #4)
> > So, of that, just fix the license tag.
> > A few other things.  The INSTALL mentioned some mandatory programs.  
> > A: You should probably determine the rpms that provide these, and Require them.
> > B: Since you obviously cannot Require Oracle, you should either:
> > i. Create synback-fedora-README.txt explaining that Oracle support is present
> > but won't work unless Oracle is installed.
> 
> Hi Jon,
> 
> I just released 1.2.2 version of synbak.
> 
> - Fixed the license tag (moved synbak from GPLv2 + GPLv3+)
> - Included the README.Fedora (about oracle support licensing) file into doc dir
> - Added all required extra rpm packages to the spec file
> 
> let's me know if it's ok now.

Looks good, but rename README.Fedora to synbak-README.fedora, so multiple SRPMs on dev systems won't clobber each other.

> SRPM URL:
> http://www.initzero.it/products/opensource/synbak/download/synbak-1.2.2-1.fc9.src.rpm
> 
> the spec file is included into srpm of course, I must provide an external
> version?

It's preferrred.  I won't make a huge deal out of it, some reviewers will, so I usually do, since I use scripts to put my rpms on my webserver anyway.
 
> Best Regards

Rename the readme, and the package itself is fine.  Just need to see some review work, and I can sponsor and approve.
Comment 6 Jon Ciesla 2008-09-22 16:20:56 EDT
To clarify the review work, find several review requests, and complete the review process, but be sure to mention that you're not yet sponsored and cannot actually approve the package.
Comment 7 Ugo Viti 2008-09-22 16:55:12 EDT
> Looks good, but rename README.Fedora to synbak-README.fedora, so multiple SRPMs
> on dev systems won't clobber each other.

Ok, I renamed to "synbak-README.fedora" and uploaded a new copy to 
SRPM URL: http://www.initzero.it/products/opensource/synbak/download/synbak-1.2.2-1.fc9.src.rpm

and uploaded 

SPEC URL: http://www.initzero.it/products/opensource/synbak/download/synbak.spec

> To clarify the review work, find several review requests, and complete the
> review process, but be sure to mention that you're not yet sponsored and cannot
> actually approve the package.

it's right, I'll make some reviews and post links here in the next days.

Thank you,

Best Regards
Comment 8 Jon Ciesla 2008-10-08 09:10:22 EDT
Update?
Comment 9 Jonathan Underwood 2009-01-01 08:26:19 EST
Hi Jon - there doesn't seem to be any requirement in the sponsorship guidelines that a new contributor must perform a few unofficial reviews before being sponsored. I agree that it is desirable, but it doesn't seem to be mandatory? I don't think performing unofficial package reviews should block this package from being approved and imported, therefore. OTOH, perhaps the submitter has vanished.
Comment 10 Mamoru TASAKA 2009-01-01 09:25:48 EST
(In reply to comment #9)
> Hi Jon - there doesn't seem to be any requirement in the sponsorship guidelines
> that a new contributor must perform a few unofficial reviews before being
> sponsored. I agree that it is desirable, but it doesn't seem to be mandatory?

See:
https://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored#Sponsorship_model
Actually it depends on the judgment of the people who is going to
sponsor the new contributors. However for my case I always request
new contributors to either submit anothere review request or do a pre-review
of other person's review request, especially because
- after getting sponsored he/she now can formally approve other person's
  review request
- and the new contributor has to _maintain_ his/her packages on Fedora
  (of course the sponsor must support him/her, however basically
   it should be done by him/herself).
Comment 11 Jon Ciesla 2009-01-02 08:33:22 EST
It shouldn't block the package, but it can block sponsorship if the sponsor wishes it to, which I do.  I like to see 1-3 practice reviews.  If this package is hugely critical, an existing maintainer can take it over if the submitter is unable to become sponsored for whatever reason.  I don't see anything glaring that would stop Ugo being sponsored, I just like to have some indication of understanding of the guidelines, beyond the construction of one package.
Comment 12 Jonathan Underwood 2009-01-02 09:16:41 EST
Jon, Mamoru  - thanks for your responses, I can certainly understand that point of view.
Comment 13 Jon Ciesla 2009-03-31 09:20:58 EDT
Ping?
Comment 14 Jon Ciesla 2010-04-29 15:56:28 EDT
This review is 2 years old and inactive for 1.  Ugo, if I don't hear back in 1 week, I'm closing it.
Comment 15 Jon Ciesla 2010-05-06 09:22:06 EDT
Closing.