Bug 526855 - Review Request: webacula - Web interface of a Bacula backup system
Summary: Review Request: webacula - Web interface of a Bacula backup system
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-10-02 06:11 UTC by Yuri Timofeev
Modified: 2010-02-28 06:03 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-02-28 06:03:08 UTC
atorkhov: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Yuri Timofeev 2009-10-02 06:11:42 UTC
Spec URL: http://webacula.sourceforge.net/files/webacula.spec
SRPM URL: http://webacula.sourceforge.net/files/webacula-3.2.1-1.fc10.src.rpm
Description: Webacula - Web Bacula - web interface of a Bacula backup system.
Currently it can run Job, restore all files or selected files,
restore the most recent backup for a client,
restore backup for a client before a specified time,
mount/umount Storages, show scheduled, running and terminated Jobs and more.
Supported languages: English, French, German, Portuguese Brazil, Russian.

Comment 1 Yuri Timofeev 2009-10-02 06:15:08 UTC
That this is my first package and I need a sponsor.


Project Home Page: http://webacula.sourceforge.net/

mock build - ok.
koji build - ok : https://koji.fedoraproject.org/koji/taskinfo?taskID=1723734

Comment 2 David Nalley 2009-10-06 03:02:42 UTC
First of all, thanks, this looks like a great aid to bacula users.

I can not sponsor you but a few comments to help you clean things up a bit: 

ValidateDateTime.php in source is a bundled library - and needs to use a system lib. Unfortunately a quick search didn't reveal this library in Fedora's package repository. A google search for the function names only reveals a pointer to your software, so this might be something that really is part of webacula, but given it's purpose I doubt it. 

https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

If I were reviewing your packages I'd evaluate that (along with the other libraries) on these factors first: 
1. Does the library have a separate repository 
2. Does or could any other application use this library

A yes answer to either makes me lean towards it being a bundled library and needing to be packaged separately. 



rpmlint:
[ke4qqq@nalleyt43 SRPMS]$ rpmlint webacula-3.2.1-1.fc11.src.rpm 
webacula.src: W: strange-permission webacula_clean_tmp_files 0775

You may want to change that to 755, but don't really see that as a blocker. 


[ke4qqq@nalleyt43 noarch]$ rpmlint webacula-3.2.1-1.fc11.noarch.rpm 
webacula.noarch: E: htaccess-file /usr/share/webacula/html/.htaccess
webacula.noarch: E: htaccess-file /usr/share/webacula/install/.htaccess
webacula.noarch: E: htaccess-file /usr/share/webacula/html/test_mod_rewrite/.htaccess
webacula.noarch: E: htaccess-file /usr/share/webacula/docs/.htaccess
webacula.noarch: E: htaccess-file /usr/share/webacula/application/.htaccess
webacula.noarch: E: htaccess-file /usr/share/webacula/languages/.htaccess
1 packages and 0 specfiles checked; 6 errors, 0 warnings.

content from .htaccess files should ideally be consolidated in your .conf file and the file removed during installation. 


Also consider writing an INSTALL.fedora document much like exists for debian. Essentially you want to take them from the point right after 'yum install webacula' completes, to having a working system.

Comment 3 Yuri Timofeev 2009-10-06 12:25:56 UTC
Thank you for your reply.

1. My class ValidateDateTime.php moved to appropriate directory.
2. Create INSTALL.fedora documentation

But:

$ rpmlint webacula-3.2.2-1.fc10.noarch.rpm
webacula.noarch: E: htaccess-file /usr/share/webacula/html/.htaccess
webacula.noarch: E: htaccess-file /usr/share/webacula/html/test_mod_rewrite/.htaccess
1 packages and 0 specfiles checked; 2 errors, 0 warnings.

Options "RewriteBase" and "RewriteRule" in httpd.conf does not work.
So I left them at the same place, in .htaccess file.

koji build - OK : https://koji.fedoraproject.org/koji/taskinfo?taskID=1730293

Spec URL: http://webacula.sourceforge.net/files/webacula.spec
SRPM URL: http://webacula.sourceforge.net/files/webacula-3.2.2-1.fc11.src.rpm

Comment 4 Peter Lemenkov 2009-10-08 08:06:09 UTC
Yuri, I can sponsor you, but I can't find your FAS name.

Comment 5 Peter Lemenkov 2009-10-08 08:50:41 UTC
Unblocking FE-NEEDSPONSOR - I just sponsored Yuri.

Comment 6 Peter Lemenkov 2009-10-08 09:04:46 UTC
David, you may proceed with actual review now. :)

Comment 7 Alexey Torkhov 2009-10-08 10:14:09 UTC
(In reply to comment #3)
> Options "RewriteBase" and "RewriteRule" in httpd.conf does not work.
> So I left them at the same place, in .htaccess file.
RewriteRule does work, but you need to specify full path in it.

Sources and readme say that license is GPL version 3 or later. If this is correct, license tag should be changed to GPLv3+.

Documentation that is installed in %doc must not be also installed to %{_datadir}/%{name}. Remove those duplicated files after copying.

Use macros consistently - either "rm" or "%{__rm}". I prefer first way :)

And please split changelog entries by empty line.

Comment 8 Yuri Timofeev 2009-10-09 05:03:44 UTC
Now, all errors corrected.
And, at the same time, new version released.

Spec URL: http://webacula.sourceforge.net/files/webacula.spec
SRPM URL: http://webacula.sourceforge.net/files/webacula-3.3.0-1.fc11.src.rpm

Comment 9 Alexey Torkhov 2009-10-09 15:38:51 UTC
(In reply to comment #7)
> Sources and readme say that license is GPL version 3 or later. If this is
> correct, license tag should be changed to GPLv3+.

You didn't address this. Either comment (you're upstream developer, right?) that it is licensed GPL version 3 only, or change the spec.

> And please split changelog entries by empty line.

Well, you removed older entries, but better to save changelog, even during review - you'll have to do it anyway after package is imported.

Comment 10 Yuri Timofeev 2009-10-10 10:54:06 UTC
Thanks and sorry, I'm confused in their spec-files.

I hope that now everything is done properly.

SPEC URL: http://webacula.sourceforge.net/files/webacula.spec
SRPM URL: http://webacula.sourceforge.net/files/webacula-3.3.0-2.fc11.src.rpm

Comment 11 Alexey Torkhov 2009-10-10 16:28:33 UTC
Here is review checklist. A few new problems discovered.

+ rpmlint output:
webacula.src: W: strange-permission webacula_clean_tmp_files 0775
2 packages and 0 specfiles checked; 0 errors, 1 warnings.

+ The package is named according to the Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format
  %{name}.spec.
+ The package meets the Packaging Guidelines.
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The License field in the package spec file matches the actual license.
+ File, containing the text of the license(s) for the package is included in
  %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
- The sources used to build the package matches the upstream source, as
  provided in the spec URL.

http://downloads.sourceforge.net/project/webacula/webacula-3.3.0.tar.gz
Not found. Did you forget to upload release?

+ The package successfully compiles and builds into binary rpms on at least
  one supported architecture.
+ If the package does not successfully compile, build or work on an
  architecture, then those architectures should be listed in the spec in
  ExcludeArch.
+ All build dependencies are listed in BuildRequires.
- Spec file handles locales properly.

It puts locales under /usr/share/webacula/languages while usual place is
/usr/share/locale, so you cannot use %find_lang here. You need to specify
individual locales with %lang(en), like:
%lang(en) %{_datadir}/%{name}/languages/en
...

This will also require to specify directories directly:
%dir %{_datadir}/%{name}
%dir %{_datadir}/%{name}/languages
%{_datadir}/%{name}/application
...

+ Does not store library files.
+ Package does not contain bundle copies of system libraries.
+ The package does not designed to be relocatable.
+ A package owns all directories that it creates.
+ A package does not list a file more than once in the spec file's %files
  listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or
  $RPM_BUILD_ROOT).
+ The package consistently uses macros.

I'm suggesting to replace __command macros (%{__rm}, %{__mv}, ...) with their
usual equivalents as their usage is unnecessary in Fedora. Not a blocker,
though.

+ The package contains code, or permissable content.
+ Does not contain large documentation files.
+ Includes only doc files in %doc.
+ Noarch package - does not contain libraries.
+ Does not contain GUI applications.
+ The package does not own files or directories already owned by other
  packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} or
  $RPM_BUILD_ROOT).
+ All filenames in the package are valid UTF-8.

Comment 12 Alexey Torkhov 2009-10-11 22:09:56 UTC
One more thing:
You have russian translation for summary, but not for description. Why?
You can add it with %description -l ru

Comment 14 Alexey Torkhov 2009-10-12 12:32:03 UTC
You should add following dirs to %files, as I pointed, otherwise you have this dirs unowned:
%dir %{_datadir}/%{name}
%dir %{_datadir}/%{name}/languages

Source http://downloads.sourceforge.net/project/webacula/webacula-3.3.0.tar.gz is still missing. I've found that released file called webacula-v3.3.tar.gz. You can define some var to 3.3 (like "%global file_version 3.3") and use that in Source0 instead of %version.

Next, in released webacula-v3.3.tar.gz there is actually more files than in webacula-3.3.0.tar.gz. You can delete unneeded ones in %prep section.

Comment 15 Alexey Torkhov 2009-10-12 12:49:13 UTC
And also in webacula-v3.3.tar.gz it has simple "webacula" dir instead of "webacula-3.3.0". To work with it you can use -n param for %setup:

%setup -q -n %{name}

Comment 16 Yuri Timofeev 2009-10-13 06:31:50 UTC
"%dir" fixed.

Source name (webacula-3.3.tar.gz) file fixed.

And I removed unneeded files from sources.

SPEC: http://webacula.sourceforge.net/files/webacula.spec
SRPM : http://webacula.sourceforge.net/files/webacula-3.3-4.fc11.src.rpm

Comment 18 Alexey Torkhov 2009-10-15 07:43:42 UTC
Everything is good now.

+ The sources used to build the package matches the upstream source, as
  provided in the spec URL.

d6ffafa3315758fb8d6b313e50c5c78c  webacula-3.3.tar.gz
d6ffafa3315758fb8d6b313e50c5c78c  webacula-3.3.tar.gz

+ Spec file handles locales properly.

APPROVED

Comment 19 Yuri Timofeev 2009-10-15 11:04:48 UTC
Thanks all.

New Package CVS Request
=======================
Package Name: webacula
Short Description: Webacula - Web Bacula - web interface of a Bacula backup system
Owners: tim4dev
Branches: F-11 F-12 EL-5
InitialCC:

Comment 20 Kevin Fenzi 2009-10-15 17:31:22 UTC
cvs done.

Comment 21 Yuri Timofeev 2009-10-16 12:29:05 UTC
koji.fedoraproject.org :
build F-11 F-12 EL-5 completed successfully

Comment 22 Yuri Timofeev 2010-02-16 08:49:10 UTC
Package Change Request
======================
Package Name: webacula
New Branches: F-13
Owners: tim4dev

Comment 23 Kevin Fenzi 2010-02-16 16:07:48 UTC
There is going to be a mass branching for F-13 inn about 8 hours. No need to request F13 branch, you will automatically get one.

Comment 24 Kevin Fenzi 2010-02-28 06:03:08 UTC
This package is built and in, no need to leave this bug open.


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