Bug 230344 - Review Request: bacula - Cross platform network backup for Linux, Unix, Mac and Windows.
Review Request: bacula - Cross platform network backup for Linux, Unix, Mac a...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dennis Gilmore
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-02-28 09:25 EST by Andreas Thienemann
Modified: 2007-11-30 17:11 EST (History)
13 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-24 22:35:12 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
dennis: fedora‑review+
andreas: fedora‑cvs+


Attachments (Terms of Use)
fix Provides and Requires (4.17 KB, patch)
2007-07-13 07:43 EDT, Dan Horák
no flags Details | Diff
fix for %preun scriptlets (650 bytes, patch)
2007-07-13 12:13 EDT, Dan Horák
no flags Details | Diff

  None (edit)
Description Andreas Thienemann 2007-02-28 09:25:09 EST
Spec URL: http://home.bawue.de/~ixs/bacula/bacula.spec
SRPM URL: http://home.bawue.de/~ixs/bacula/bacula-2.0.2-2.el5.src.rpm
Unpacked SRPM: http://home.bawue.de/~ixs/bacula/
Description: 
Bacula is a set of programs that allow you to manage the backup,
recovery, and verification of computer data across a network of
different computers. It is based on a client/server architecture and is
efficient and relatively easy to use, while offering many advanced
storage management features that make it easy to find and recover lost
or damaged files.

Please do keep in mind, this is just a preliminary submission.
There are still going to besome minor changes in the initscripts and some file locations might be changed a bit. However, this is in general the package I'll finally submit in a few days but put in here already so mmcgrath can take a look at it already.
Comment 1 Dan Horák 2007-02-28 14:39:50 EST
Running "alternatives" in the init-scripts to get the database backend must be
done with LANG=C or the grep in the pipe will find nothing on non-english locales.
Comment 2 Paul Howarth 2007-03-02 10:31:26 EST
I'm glad to see that, unlike upstream, you've split the storage daemon off from
the director package.

I can't look at this in detail at the moment as I'm very busy with $DAYJOB but
I'll keep an eye on this ticket and will hopefully be able to comment more later on.
Comment 3 Dennis Gilmore 2007-03-06 19:09:12 EST
[Tue Mar 06 18:05:00 2007] [error] [client 127.0.0.1] PHP Fatal error:  Smarty 
error: unable to write to $compile_dir '/usr/share/bacula-web/templates_c'. Be 
sure $compile_dir is writable by the web server user. 
in /usr/share/bacula-web/external_packages/smarty/Smarty.class.php on line 
1088


Need to make sure apache can write to that dir
Comment 4 Paul Howarth 2007-03-07 02:41:08 EST
(In reply to comment #3)
> [Tue Mar 06 18:05:00 2007] [error] [client 127.0.0.1] PHP Fatal error:  Smarty 
> error: unable to write to $compile_dir '/usr/share/bacula-web/templates_c'. Be 
> sure $compile_dir is writable by the web server user. 
> in /usr/share/bacula-web/external_packages/smarty/Smarty.class.php on line 
> 1088
> 
> 
> Need to make sure apache can write to that dir

A couple of comments on that:

1. Smarty is availanle in Extras (php-Smarty), so the version in Extras should
be used instead of the bundled version.

2. It shouldn't be writing anything under /usr; I suggest creating
/var/cache/bacula-web and making /usr/share/bacula-web/templates_c a symlink to
it; /var/cache/bacula-web should be writable by apache and will need an SELinux
context type of httpd_cache_t.

Comment 5 Felix Schwarz 2007-03-13 07:07:57 EDT
The changelog dates are wrong (unless you did use a time machine). Before adding
the package to Fedora, please update the version number to 2.0.3 as this release
fixed a dataloss bug when using the new encryption mechanisms.
Comment 6 Andreas Thienemann 2007-03-22 05:33:23 EDT
Thanks for the comments so far. I've been to cebit in germany the last week, so
there was no time to work on the package. Sorry for that.

Thanks for the suggestion handling the web-package. I think putting the data to
/var is actually a good idea but this is best handled not with a symlink but by
changing the location in the code.
Using the system-wide smarty package is certainly a good idea.

And about the dates in the changelog, yeah, they are wrong. :D It's 2007 and not
2006. This has been fixed in the meantime, but I did not update the package on
the web yet.
Comment 7 Dan Horák 2007-03-27 10:49:46 EDT
It looks like that no subpackage creates the directory /var/log/bacula and that
makes logrotate to fail there.
Comment 8 Dan Horák 2007-03-30 04:58:04 EDT
Another issue I see in using generic names for executables as gnome-console and
wxconsole. They should probably use something like bac-foo or bacula-foo and
this should be also moved upstream.
Comment 9 Mike McGrath 2007-04-25 17:11:20 EDT
Any updates available?
Comment 10 Jerry Amundson 2007-04-26 01:43:14 EDT
Mostly research up to mid-April. Lately I started on a new spec file, for 
various reasons...
1. Comment #5 but no update made for 2.0,3,
2. IMO, bacula-docs should be a package on it's own,
3, Trying to understand why the bacula-storage-<database> packages exist, and
4. For my own spec file practice.
Went through a couple rounds with mach build. It still needs work. though I 
hope to have it in decent shape in a few days.
Comment 11 Felix Schwarz 2007-04-26 02:02:39 EDT
What happend to Andreas Thienemanns work? According to #6 he did some internal
improvements. Jerry, do have access to these improvements? Furthermore, Andreas'
spec file looks quite good, so only minor updates (and maybe some extensions)
are needed IMHO. Updating to 2.0.3 should be trivial.

bacula-storage is split up into multiple packages due to different database
backends. The package contains bcopy and btape which are linked against specific
database libraries.
Comment 12 Andreas Thienemann 2007-04-26 12:34:45 EDT
Well, I do not know what happened to my specfile, I also have no idea who Jerry
is and why he is creating a new specfile from scratch.

Anyway, be that as it may, there's an updated bacula package at
<http://home.bawue.de/~ixs/bacula/bacula-2.0.3-2.el5.src.rpm> or the spec at
<http://home.bawue.de/~ixs/bacula/bacula.spec>.

Take a look, it's basically finished and I'd like to see some last comments
before  I'll ask for a review.
Comment 13 Jerry Amundson 2007-04-26 14:37:33 EDT
(In reply to comment #12)
> Well, I do not know what happened to my specfile, I also have no idea who Jerry
> is and why he is creating a new specfile from scratch.

I'm Jerry, and I'm also sorry! I didn't mean to step on toes. But there'd been
little activity lately, and I get nervous when mmcgrath bumps the severity to
medium! :-)
Anyway, thank you for the updated package... I also see the reason for
bacula-storage, so my mumbling in Comment #10 can be ignored.
Comment 14 Andreas Thienemann 2007-04-26 15:09:04 EDT
(In reply to comment #13)

Hi Jerry :-D

Don't worry, you are free to work on the bacula.spec as long as you wish. No
toes to step on. :D I was just trying to answer Felix's question. *g*
Comment 15 Jerry Amundson 2007-04-26 23:03:05 EDT
There's a minor difference with 2.0.3-verify.patch - please refresh from 
sf.net for the next round.
Comment 16 Andreas Thienemann 2007-04-27 03:05:40 EDT
(In reply to comment #15)
> There's a minor difference with 2.0.3-verify.patch - please refresh from 
> sf.net for the next round.

Nope. The patch from sf.net will not apply as the first hunk fails. As it's just
a change in the copyright comments at th start of the file, I removed hunk 1.
Comment 17 Nigel Jones 2007-04-27 03:56:03 EDT
Just a quick note, I've removed Jerry as the bug assignee as he is not sponsored
into cvsextras
(https://admin.fedoraproject.org/accounts/dump-group.cgi?group=cvsextras&format=html)
Comment 18 Mike McGrath 2007-06-05 11:43:42 EDT
Please re-consider not using fedora-usermgmt as it makes the SRPM and RPMs 
incompatible with any other distribution.
Comment 19 Dennis Gilmore 2007-06-05 13:17:20 EDT
i second not using fedora-usermgmt
Comment 20 Mike McGrath 2007-06-05 16:09:12 EDT
Also /var/spool/bacula/log should be moved to /var/log/bacula/ with a logrotate
(if bacula does not already do a log rotation)
Comment 21 Christian Nolte 2007-06-06 10:21:41 EDT
The database backend for the SD is not correctly detected if one uses a locale
other than LANG=C. I had to do a 'export LANG=C' before the alternatives-line in
/etc/init.d/bacula-sd
Comment 22 Dan Horák 2007-06-06 11:02:53 EDT
(In reply to comment #21)
> The database backend for the SD is not correctly detected if one uses a locale
> other than LANG=C. I had to do a 'export LANG=C' before the alternatives-line in
> /etc/init.d/bacula-sd

already found, see #1 :-)
Comment 23 Christian Nolte 2007-06-07 03:33:00 EDT
Ok, next time I read all the comments before posting ;-) Just a side note: the
release announced in comment #12 (bacula-2.0.3-2.el5.src.rpm) still has the
locale problem mentioned in comment #1.

The script /usr/libexec/bacula/make_catalog_backup only works for the
mysql-backend. It should be rewritten to use the alternatives system. I have
installed the following director packages:

rpm -qa | grep bacula-dir
bacula-director-common-2.0.3-2.fc7
bacula-director-postgresql-2.0.3-2.fc7

I had to edit make_catalog_backup to make it work for postgresql.

Comment 24 Felix Schwarz 2007-06-11 10:05:12 EDT
(In reply to comment #18)
> Please re-consider not using fedora-usermgmt as it makes the SRPM and RPMs 
> incompatible with any other distribution.

I like fedora-usermgmt as it provides some additional possibilites for
administrators and this should become a Fedora RPM (even EPEL has fedora-usermgmt).

But there seems to be another problem: I built RPMs from your SRPM on CentOS 5
(i386) but I get these errors when doing a 'yum install ...':
--> Running transaction check
--> Processing Dependency: /usr/afsws/bin/pagsh for package: bacula-common
--> Finished Dependency Resolution
Error: Missing Dependency: /usr/afsws/bin/pagsh is needed by package bacula-common

This seems to be an issue within dependency calculation in rpmbuild as noted in
http://www.bacula.org/dev-manual/Bacula_RPM_Packaging_FAQ.html:
"5. I'm building my own rpms but on all platforms and compiles I get an
unresolved dependency for something called /usr/afsws/bin/pagsh."

Do I need additional rpmbuild defines? Do you experience the same issue?
Comment 25 Felix Schwarz 2007-06-11 10:45:38 EDT
AFS dependency removal:
--------------------------------------------
--- bacula.spec	2007-06-11 16:43:58.000000000 +0200
+++ bacula.modified.spec	2007-06-11 16:12:49.000000000 +0200
@@ -324,6 +324,8 @@
 %patch8 -p0
 popd
 
+rm -f bacula-2.0.3/examples/afs-bacula
+
 # We are building the source several times, each with a different storage backend
 mkdir bacula-mysql bacula-postgresql bacula-sqlite
--------------------------------------------

Furthermore, I think that packaging the examples directory in bacula-common
leads to some perl dependencies which could be omitted: perl-Net-Telnet,
perl-TermReadKey (only available in F7 afaik!), perl-Time-modules and perl-TimeDate.
Comment 26 Jeffrey C. Ollie 2007-06-11 11:33:01 EDT
(In reply to comment #25)
>
> Furthermore, I think that packaging the examples directory in bacula-common
> leads to some perl dependencies which could be omitted: perl-Net-Telnet,
> perl-TermReadKey (only available in F7 afaik!), perl-Time-modules and
perl-TimeDate.

Anything marked as %doc should not be create any additional requirements.  In
most cases, it's easy enough to prevent that from happening by removing the
execure bits from the files.

Comment 27 Christian Nolte 2007-06-14 03:20:20 EDT
logrotate gives an error because /var/log/bacula does not exist as configured in
/etc/logrotate.d/bacula:

error: error accessing /var/log/bacula: No such file or directory
error: bacula:3 glob failed for /var/log/bacula/*.log
error: found error in /var/log/bacula/*.log , skipping
Comment 28 Mike McGrath 2007-06-25 10:32:57 EDT
I'm interested in co-maintaining this package if you'd like some help.
Comment 29 Mike McGrath 2007-07-03 12:31:16 EDT
I got the ok to assist in co-maintaining this package.  Here's the latest:

SPEC: http://mmcgrath.net/~mmcgrath/bacula/bacula.spec
SRPM: http://mmcgrath.net/~mmcgrath/bacula/bacula-2.0.3-3fc6.src.rpm

Changes:
- Chmod -x the example scripts to remove any unwanted requires
- Added a LANG=C above alternatives in the init scripts
Comment 30 Felix Schwarz 2007-07-03 13:55:58 EDT
Thanks for fixing the doc/requires issue. I recompiled Andreas' SRPM on CentOS 5
(with clients for CentOS 4 i386 and FC 6 x86_64) and so far it worked flawlessly
in a small office environment with MySQL (besides the hopefully fixed LANG issues).
Comment 31 Dennis Gilmore 2007-07-09 20:57:28 EDT
i will review this
Comment 32 Andreas Thienemann 2007-07-11 16:25:30 EDT
(In reply to comment #31)
> i will review this

thanks.

final upload is at http://home.bawue.net/~ixs/bacula/bacula-2.0.3-4.src.rpm

Please review that build, it has the known problems basically fixed, the major
rpmlint problems have been fixed, the remaining warnings and errors should be
ignorable.
Comment 33 Andreas Thienemann 2007-07-11 16:34:04 EDT
(In reply to comment #23)

> The script /usr/libexec/bacula/make_catalog_backup only works for the
> mysql-backend. It should be rewritten to use the alternatives system. I have
> installed the following director packages:
Noticed that problem. During building different variables are changed in the
file.  I'm not sure what the right solution is ATM, whether I'll just roll a new
backup-catalog script to select the right database at runtime or if the script
should be packaged for each backend and added to the alternatives system.

This will be fixed before initial import into fedora though and should not be a
blocker for the review.
Comment 34 Andreas Thienemann 2007-07-11 16:36:31 EDT
http://home.bawue.de/~ixs/bacula/bacula-2.0.3-4.src.rpm is the right url.
Comment 35 Dan Horák 2007-07-13 07:43:11 EDT
Created attachment 159155 [details]
fix Provides and Requires

I was not able to update bacula 2.0.2 to 2.0.3 with "rpm -Fvh bacula*.rpm" on
my FC6/x86_64 machine. There were strange dependency problems. This patch
should fix them.
Comment 36 Andreas Thienemann 2007-07-13 10:27:07 EDT
Thx Dan, good catch.

http://home.bawue.de/~ixs/bacula/bacula-2.0.3-5.src.rpm has fixed the problem
Comment 37 Dan Horák 2007-07-13 12:13:07 EDT
Created attachment 159202 [details]
fix for %preun scriptlets

a copy'n'paste bug that makes the %preun scriptlets fail
Comment 38 Andreas Thienemann 2007-07-13 12:29:31 EDT
http://home.bawue.de/~ixs/bacula/bacula-2.0.3-6.src.rpm
Comment 39 John Poelstra 2007-07-17 15:56:18 EDT
I believe there is a missing BuildRequires for sqlite2-devel.

I'm trying to build this package on FC6 PPC and get the following message:
"configure: error: Unable to find sqlite.h in standard locations"
Comment 40 Andreas Thienemann 2007-07-17 19:41:35 EDT
(In reply to comment #39)
> I believe there is a missing BuildRequires for sqlite2-devel.

Mhm, don't think so. For every current fedora release sqlite 3 is used. Only
RHEL4 needs sqlite 2. But luckily the devel packages are are named sqlite-devel.
Requiring sqlite2-devel is not needed on fedora, as sqlite3 is already pulled in
on fedora and sqlite2 get's pulled in by the sqlite-devel dependency on RHEL4.

The difference between sqlite2 and 3 thus lies not in the BuildRequire line but
in the way configure gets called. For sqlite2 it's --with-sqlite and for sqlite3
it has to be --with-sqlite3.

> I'm trying to build this package on FC6 PPC and get the following message:
> "configure: error: Unable to find sqlite.h in standard locations"

This looks very much as if the configure call is incorrect. I just checked the
package in a mock build for fc6-x86_64 and it builds fine.
I guess you did not build the package in mock but simply used rpmbuild --rebuild?
In this case, please pass at least --define 'fedora 6' and maybe --define 'dist
fc6' to your rpmbuild call.

That way the right argument for the %configure macro can be selected.
Comment 41 Dan Horák 2007-07-18 00:59:20 EDT
Andreas is right, look for "# Build sqlite director" in the spec file. Similar
define is needed for RHEL.
Comment 42 Dennis Gilmore 2007-07-18 15:44:33 EDT
ok still alot of rpmlint noise.  permissions are ok 

config files should be noreplace 

initscripts really should have incoherent warnings fixed 

E: bacula-common wrong-script-interpreter
/usr/share/doc/bacula-common-2.0.3/examples/afs-bacula "/usr/afsws/bin/pagsh"

make this non executable


Builds fine in mock on FC-6 

package naming is ok


%if 0%{?fedora}
        %if "%{fedora}" >= "7"
BuildRequires: tcp_wrappers-devel
        %else
BuildRequires: tcp_wrappers
        %endif
%endif

should really be 
%if "%{?fedora}" >= "7"
BuildRequires: tcp_wrappers-devel
%else
BuildRequires: tcp_wrappers
%endif

since im sure we will want tcp_wrappers for epel.

nothing owns /etc/bacula/  or /usr/libexec/bacula/

Comment 43 Paul Howarth 2007-07-18 16:17:13 EDT
(In reply to comment #42)
> %if 0%{?fedora}
>         %if "%{fedora}" >= "7"
> BuildRequires: tcp_wrappers-devel
>         %else
> BuildRequires: tcp_wrappers
>         %endif
> %endif
> 
> should really be 
> %if "%{?fedora}" >= "7"
> BuildRequires: tcp_wrappers-devel
> %else
> BuildRequires: tcp_wrappers
> %endif
> 
> since im sure we will want tcp_wrappers for epel.

This could actually be simplified right down to:
BuildRequires:  /usr/include/tcpd.h

That would work for all distribution releases without needing any of the
conditionals.
Comment 44 Andreas Thienemann 2007-07-18 16:18:47 EDT
(In reply to comment #42)

> ok still alot of rpmlint noise.  permissions are ok 

The noise is basically the following line:
"W: bacula unversioned-explicit-provides bacula-director-%{version}-%{release}"
Looks like an ignorable problem with rpmlint as it does no variable expansion.


> config files should be noreplace 

Most of the config files are marked as noreplace.
I did leave it out for security relevant files such as /etc/pam.d/gnome-console
in order to be able to fix problems with a package update and make sure they are
applied and not put into a .rpmnew file.
Do we have a policy on that?

Same goes for the logwatch files:
W: bacula-director-common conffile-without-noreplace-flag
/etc/logwatch/conf/logfiles/bacula.conf
W: bacula-director-common conffile-without-noreplace-flag
/etc/logwatch/conf/services/bacula.conf

The logformat does change from time to time. That way we can update the definitions.

> initscripts really should have incoherent warnings fixed 

W: bacula-client incoherent-subsys /etc/rc.d/init.d/bacula-fd $prog
The executable is stored in a variable.
rpmlint -i gives "It is also possible that rpmlint gets this wrong, especially
if the init script contains nontrivial shell variables and/or assignments. These
cases usually manifest themselves when rpmlint reports that the subsys name
starts a with '$'; in these cases a warning instead of an error is reported and
you should check the script manually."

If that's important one could change the initscripts.

About the incoherent names: This might not be that easy. We could change the
names from the upstream used bacula-fd, -sd and -dir, to bacula-client,
bacula-storage and bacula-director, which would conform to the bacula package
names. This would however not remove the rpmlint warnings as the real package
names are bacula-{director,storage}-{mysql,sqlite,postgresql} and the init
script is packaged in the -common file getting its runtime information from the
alternatives subsystem.

Either way, the warning won't go away. However, I'm open to renaming the
initscripts to conform to the package names although the current names are what
upstream has been using in the past.
 
> E: bacula-common wrong-script-interpreter
> /usr/share/doc/bacula-common-2.0.3/examples/afs-bacula "/usr/afsws/bin/pagsh"
> 
> make this non executable

It actually is.
-rw-r--r--  /usr/share/doc/bacula-common-2.0.3/examples/afs-bacula
rpmlint issue, ignoring.


> %if 0%{?fedora}
>         %if "%{fedora}" >= "7"
> BuildRequires: tcp_wrappers-devel
>         %else
> BuildRequires: tcp_wrappers
>         %endif
> %endif
> 
> should really be 
> %if "%{?fedora}" >= "7"
> BuildRequires: tcp_wrappers-devel
> %else
> BuildRequires: tcp_wrappers
> %endif
> 
> since im sure we will want tcp_wrappers for epel.

Right. 
 
> nothing owns /etc/bacula/  or /usr/libexec/bacula/

Thx.

Try -7 at http://home.bawue.de/~ixs/bacula/bacula-2.0.3-7.src.rpm
Comment 45 Paul Howarth 2007-07-18 16:35:51 EDT
(In reply to comment #44)
> (In reply to comment #42)
> 
> > ok still alot of rpmlint noise.  permissions are ok 
> 
> The noise is basically the following line:
> "W: bacula unversioned-explicit-provides bacula-director-%{version}-%{release}"
> Looks like an ignorable problem with rpmlint as it does no variable expansion.

Is there some particular reason you're using this style of virtual provide? How
about:

Provides: bacula(director) = %{version}-%{release}

rpmlint might be happier with that.
Comment 46 Andreas Thienemann 2007-07-18 16:50:17 EDT
> Is there some particular reason you're using this style of virtual provide? How
> about:
> 
> Provides: bacula(director) = %{version}-%{release}
> 
> rpmlint might be happier with that.

Actually, that is a very good question.
And taking a closer look, I applied rpmlint to older rpms which had fucked up
require lines.

The current srpms have been fixed by applying the patches from Dan.
Rebuilding them in mock and running rpmlint on them looks much better and this
junk never appeared.

Sorry about that, I guess I should just go to bed for today. :-)
Comment 47 Andreas Thienemann 2007-07-19 09:16:34 EDT
http://home.bawue.de/~ixs/bacula/bacula-2.0.3-8.src.rpm

* Wed Jul 19 2007 Andreas Thienemann <andreas@bawue.net> 2.0.3-8
- Moved some files around in the %%files section and refactored
  spec parts a bit
- Fixed up the catalog-backup scripts by including them in the
  alternatives system
- Applied tls patch fixing some tls disconnection issues.
Comment 48 Dennis Gilmore 2007-07-24 02:04:36 EDT
Looks good now.
sha1sum matches upstream
ef58c91243bd819e0ac278b91aeae16639d6c8ce  bacula-2.0.3.tar.gz
ef58c91243bd819e0ac278b91aeae16639d6c8ce  bacula-2.0.3.tar.gz

Approved
Comment 49 Andreas Thienemann 2007-07-24 15:49:41 EDT
New Package CVS Request
=======================
Package Name: bacula
Short Description: Cross platform network backup for Linux, Unix, Mac and Windows
Owners: andreas@bawue.net,mmcgrath@redhat.com
Branches: FC-6 F-7 EL-5 EL-4
InitialCC:
Comment 50 Andreas Thienemann 2007-07-24 22:35:12 EDT
Package has been built successfully for F7, FC-6 and rawhide.
EL-4 and EL-5 still have unmet dependencies, which should be fixed soon however.

Thx for all the comments and the review.

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