Bug 225243 - Merge Review: amanda
Merge Review: amanda
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Orion Poplawski
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-29 16:00 EST by Nobody's working on this, feel free to take it
Modified: 2010-07-12 18:06 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-04-09 18:40:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
orion: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-29 16:00:11 EST
Fedora Merge Review: amanda

http://cvs.fedora.redhat.com/viewcvs/devel/amanda/
Comment 1 Orion Poplawski 2007-08-21 12:08:58 EDT
More thorough review to come shortly, but:

Requires(pre): useradd 

should be

Requires(pre): shadow-utils

or you get:

Error: Missing Dependency: useradd is needed by package amanda
Comment 2 Orion Poplawski 2008-02-20 13:30:26 EST
I'd like to pick this up, but I think it makes sense to jump to 2.6.0 which has
just had a second beta release and will have a full release shortly.  I've
worked up a new package here:

http://www.cora.nwra.com/~orion/fedora/amanda.spec
http://www.cora.nwra.com/~orion/fedora/amanda-2.6.0-0.b2.fc8.src.rpm

Changes are:

* Wed Feb 20 2008 Orion Poplawski <orion@cora.nwra.com> 2.6.0-0.b2
- Update to 2.6.0b2, drop upstreamed patches
- Update xattrs patch
- Add patches to fix install locations
- Add -fPIE/-pie to CFLAGS/LDFLAGS, drop pie patch
- Drop autotools BR
- Drop /usr/bin/Mail BR and specify mailer as /bin/mail
- Add %%check section
- Move /etc/amanda/amandates to /var/lib/amanda/amandates
- Remove ending . from summaries


Also, I've commented out changing the user name in .amandahosts in the client
%post script.  I can go either way, but this is the name of the user on the
server, not the client.

TODO:

- Fixup perl install location/requires
- Update xinetd and default .amandahosts files
- More rpmlint cleanups

Comment 3 Radek Brich 2008-03-11 12:12:14 EDT
Hi.

I'm slightly missing what are our roles here, but thanks :-)

About your changes:

The PIE patch is apparently still needed, as the build fails with your spec:

/usr/bin/ld: .libs/alloc.o: relocation R_X86_64_PC32 against `debug_alloc' can
not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: Bad value

That means, -PIE option can only be used for executables, not libraries, so
adding -fPIE/-pie to common *FLAGS cannot work. And thus the autotools will be
still needed.

As for the username changing, it's needed when upgrading from older packages and
the %post is not for server subpackage, but for main amanda package (or say,
common), where the .amandahosts file is contained.

Rest seems ok, but I'll rather wait with the updgrade at least for final release
of Amanda 2.6.0, they are still doing changes...
Comment 4 Orion Poplawski 2008-03-12 00:47:02 EDT
(In reply to comment #3)
> I'm slightly missing what are our roles here, but thanks :-)

Basically, with the merge of core and extras, all core packages were set to go
through a package review to make sure they conformed to modern Fedora packaging
guidelines.  As a Fedora packager, I'm here to help with that, and I assume you
are the current amanda maintainer. :-)

> About your changes:
> 
> The PIE patch is apparently still needed, as the build fails with your spec:
> 
> /usr/bin/ld: .libs/alloc.o: relocation R_X86_64_PC32 against `debug_alloc' can
> not be used when making a shared object; recompile with -fPIC
> /usr/bin/ld: final link failed: Bad value
> 
> That means, -PIE option can only be used for executables, not libraries, so
> adding -fPIE/-pie to common *FLAGS cannot work. And thus the autotools will be
> still needed.

Ah well.  I've updated the pie patch then.  What does -pie buy us these days? 
Doesn't seem like many programs use it.
 
> As for the username changing, it's needed when upgrading from older packages and
> the %post is not for server subpackage, but for main amanda package (or say,
> common), where the .amandahosts file is contained.

The problem is that the username listed in .amandahosts is the username on the
remote machine, not the named on the machine .amandahosts resides on.  We don't
know that the remote machine has been upgraded or not.  Either way can cause
problems though, so it's up to you.

> Rest seems ok, but I'll rather wait with the updgrade at least for final release
> of Amanda 2.6.0, they are still doing changes...

That's fine.  Too bad we missed the freeze, but ah well.

http://www.cora.nwra.com/~orion/fedora/amanda-2.6.0-0.b3_20080310.fc8.src.rpm

* Tue Mar 11 2008 Orion Poplawski <orion@cora.nwra.com> 2.6.0-0.b3_20080310.1
- Update to 2.6.0b3_20080310
- Re-add updated pie patch, re-add autogen
- Update example patch to modify Makefile.am, leave template.d in
 /var/lib/amanda for now

TODO:

- Update xinetd and default .amandahosts files
- More rpmlint cleanups
- learn more about template.d
Comment 5 Radek Brich 2008-03-19 09:11:18 EDT
(In reply to comment #4)

Ok, few more points.

> Ah well.  I've updated the pie patch then.  What does -pie buy us these days? 
> Doesn't seem like many programs use it.

The -pie option has some security benefits, I think all Fedora daemons should be
compiled with this. But isn't your new -pie patch missing some configure.in
stuff? I'd rather let it there...

> The problem is that the username listed in .amandahosts is the username on the
> remote machine, not the named on the machine .amandahosts resides on.  We don't
> know that the remote machine has been upgraded or not.  Either way can cause
> problems though, so it's up to you.

Alright. I didn't notice that. But anyway, shouldn't we suppose users will
upgrade all machines at once, rather than having mixed version environment
(which sounds like bad idea)? Moreover, it's already there, why remove it now...

I have also noticed these things:

* The -lib patch, is it really needed? The package compiles just fine without
it. And after patching configure.in with -pie (see above), this gets overriden
by autogen.

* Why did the examples disappear from /usr/share/doc? I though it was good place
for such thing, people may be looking for it there...
Comment 6 Orion Poplawski 2008-03-27 17:00:40 EDT
(In reply to comment #5)
> (In reply to comment #4)
> 
> But isn't your new -pie patch missing some configure.in
> stuff? I'd rather let it there...
> 

The old pie patch had some stuff for configure.in, but it didn't do anything. 
It checked whether the compiler had pie support and set PIE_LDFLAGS (and some
others) if so, but none of those variables were used anywhere.  Now if upstream
were to ever support this, they would require a working check in configure.in
and proper modifying of the Makefile.am files.

> I have also noticed these things:
> 
> * The -lib patch, is it really needed? The package compiles just fine without
> it. And after patching configure.in with -pie (see above), this gets overriden
> by autogen.
 
It fixes a problem that occurs when you try to build the amanda package on a
machine with an older amanda-devel installed (because it ended up putting
-L/usr/lib before the locally built lib path).  Not going to happen in the
fedora build system, but nice to not have others trip up on it.  I've updated it
to patch the sources.

> * Why did the examples disappear from /usr/share/doc? I though it was good place
> for such thing, people may be looking for it there...
> 

Seemed like duplication to have in /etc/amanda/DailySet1 and there, but may make
sense to keep.

* Thu Mar 27 2008 Orion Poplawski <orion@cora.nwra.com> 2.6.0-0.b3_20080314.1
- Update to 2.6.0b3_20080314
- New -lib patch that patches the autotool source files

http://www.cora.nwra.com/~orion/fedora/amanda-2.6.0-0.b3_20080314.1.fc8.src.rpm
Comment 7 Orion Poplawski 2008-05-21 16:56:36 EDT
Still need to look into examples locations...

http://www.cora.nwra.com/~orion/fedora/amanda-2.6.0p1-1.fc8.src.rpm

* Wed May 21 2008 Orion Poplawski <orion@cora.nwra.com> 2.6.0p1-1
- Update to 2.6.0p1
- Move xinetd to main package - used by both client and server
- Make calcsize setuid root
- New -lib patch that patches the autotool source files
- Update pie patch
- Update example patch to modify Makefile.am, leave template.d in
 /var/lib/amanda for now
- Drop libdir patch, use --with-libdir instead
- Move perl modules to %%{perl_vendorarch} and add perl Requires
- Update xattrs patch
- Add patches to fix install locations
- Drop /usr/bin/Mail BR and specify mailer as /bin/mail
- Add %%check section
- Move /etc/amanda/amandates to /var/lib/amanda/amandates
- Remove ending . from summaries
Comment 8 Jason Tibbitts 2008-08-14 09:20:47 EDT
Two interesting things:

There is nobody CC'd on this ticket.
Amanda has just changed ownership.

I've CC'd the new maintainer; maybe this can get cleaned up and closed out.
Comment 9 Daniel Novotny 2008-10-01 09:41:17 EDT
hello,

now built the srpm on F9, updated to 2.6.0p2
new things discovered:

 * you need another BuildRequires dependency "perl-ExtUtils-Embed"
   without it you'll get compilation errors about missing header files
 * the perl patch seems to be already upstreamed (complains about duplicit
   patching) so I removed it
 * in the %files section one library name is "libamglue.so" and not "libamglue.so.*" (dot and number not used this time, I wonder why...)

all probably new stuff in 2.6.0p2 or F9...

http://people.fedoraproject.org/~dnovotny/amanda.spec
 
Orion, what do you think about it?
Comment 10 Orion Poplawski 2008-10-10 12:53:37 EDT
I've merged in your changes with mine that I've had to make while testing/running 2.6.0p1.

http://www.cora.nwra.com/~orion/fedora/amanda.spec
http://www.cora.nwra.com/~orion/fedora/amanda-2.6.0p2-2.fc9.src.rpm

* Fri Oct 10 2008 Orion Poplawski <orion@cora.nwra.com> 2.6.0p2-2
- Drop duplicated libamglue.so from -devel
- Update -pie patch
- Fix Source typo
- Move xinetd to main package - used by both client and server
- Move %{_libexecdir}/amanda/amanda-sh-lib.sh to main package
- LIBEXECDIR is used in xinetd template
- Make calcsize setuid root

I'm starting to work with upstream to get a workable version of the pie patch merged upstream.
Comment 11 Orion Poplawski 2008-10-13 18:39:10 EDT
I'm missing a trailing || : in the %pre client script:

%pre client
# when upgrading, move /etc/amandates to /var/lib/amanda/amandates
[ -f /etc/amandates ] &&
        mv /etc/amandates /var/lib/amanda/amandates || :
Comment 12 Daniel Novotny 2008-10-17 06:09:05 EDT
(In reply to comment #10)
> I've merged in your changes with mine that I've had to make while
> testing/running 2.6.0p1.
> 
> http://www.cora.nwra.com/~orion/fedora/amanda.spec
> http://www.cora.nwra.com/~orion/fedora/amanda-2.6.0p2-2.fc9.src.rpm

hello Orion, your new .src.rpm does not contain all the patches you made, for example amanda-2.6.0b3-amdiskflush.patch is not there...
Comment 13 Orion Poplawski 2008-10-17 10:36:53 EDT
(In reply to comment #12)
> (In reply to comment #10)
> > I've merged in your changes with mine that I've had to make while
> > testing/running 2.6.0p1.
> > 
> > http://www.cora.nwra.com/~orion/fedora/amanda.spec
> > http://www.cora.nwra.com/~orion/fedora/amanda-2.6.0p2-2.fc9.src.rpm
> 
> hello Orion, your new .src.rpm does not contain all the patches you made, for
> example amanda-2.6.0b3-amdiskflush.patch is not there...

Ignore the separate .spec file, and just use the src.rpm.  I must have accidentally posted the version that we use locally.  I have some private patches that we use for doing disk based backups, but aren't suitable for general use.  Sorry about that.
Comment 14 Daniel Novotny 2008-10-20 07:41:59 EDT
okay, I pushed the new amanda 2.6.0p2 into rawhide
Comment 15 Orion Poplawski 2008-10-22 18:03:43 EDT
(In reply to comment #11)
> I'm missing a trailing || : in the %pre client script:
> 
> %pre client
> # when upgrading, move /etc/amandates to /var/lib/amanda/amandates
> [ -f /etc/amandates ] &&
>         mv /etc/amandates /var/lib/amanda/amandates || :

This still needs to get fixed.  I would, but ACLs prevent me.
Comment 16 Daniel Novotny 2008-10-23 04:55:45 EDT
(In reply to comment #15)
> (In reply to comment #11)
> > I'm missing a trailing || : in the %pre client script:
> > 
> > %pre client
> > # when upgrading, move /etc/amandates to /var/lib/amanda/amandates
> > [ -f /etc/amandates ] &&
> >         mv /etc/amandates /var/lib/amanda/amandates || :
> 
> This still needs to get fixed.  I would, but ACLs prevent me.
OK, fixed.
Comment 17 C.M. Connelly 2009-02-09 15:04:40 EST
(In reply to comment #5)

> Alright. I didn't notice that. But anyway, shouldn't we suppose users will
> upgrade all machines at once, rather than having mixed version environment
> (which sounds like bad idea)? Moreover, it's already there, why remove it now...

Just for information, Amanda is supposed to be capable of working with various versions.  In our environment, we have machines running CentOS 3 (including the Amanda server), 4, and 5.  Updating Amanda on the older machines isn't likely to happen until the machines are replaced or reinstalled unless there's a really good reason to do so.  I suspect that other users may have even more heterogenous libraries.

On the other hand, part of my setup script is to modify or replace the .amandahosts file, so I'm not bothered either way.
Comment 18 Daniel Novotny 2009-11-30 10:14:59 EST
hello Orion,

All changes for the package review are made.
Do you think it's enough (and you can do "fedora-review+")
or do I need to change something more?
Comment 19 Orion Poplawski 2010-07-12 18:06:42 EDT
We should get this into EPEL for EL-6.  I'm willing to maintain there if needed.

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