Bug 191603 - Review Request: rsnapshot -- rsync-based filesystem snapshots
Review Request: rsnapshot -- rsync-based filesystem snapshots
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
Depends On:
  Show dependency treegraph
Reported: 2006-05-13 15:27 EDT by Chris Petersen
Modified: 2014-10-13 18:54 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2006-05-21 12:21:38 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)
rpmforge specfile (2.24 KB, application/octet-stream)
2006-05-16 14:30 EDT, Jason Tibbitts
no flags Details

  None (edit)
Description Chris Petersen 2006-05-13 15:27:16 EDT
Spec URL: http://forevermore.net/files/packages/rsnapshot/rsnapshot.spec
SRPM URL: http://forevermore.net/files/packages/rsnapshot/rsnapshot-1.2.1-1.src.rpm

This is a remote backup program that uses rsync to take backup snapshots of
filesystems.  It uses hard links to save space on disk.  See http://rsnapshot.org for more info.
Comment 1 Jason Tibbitts 2006-05-16 13:00:53 EDT
Note that version 1.2.3 seems to be current even though this doesn't seem to be
represented at all on rsnapshot.org.

Also, the rpmforge packages which I'm currently using apply a single patch to
the default config file (before configure has processed it).  They uncomment
@CMD_CP@, @CMD_DU@, logfile and lockfile.  Any opinions on that?
Comment 2 Chris Petersen 2006-05-16 13:51:11 EDT
I'd be more than happy to just use the package and spec from rpmforge if it's up
to snuff.

As for the version thing, I know that rsnapshot is currently recovering from a
maintainer switch and users are told to use the cvs version instead of posted
releases because there are a lot of good changes in cvs that the new maintainer
hasn't had enough time to package up yet.  I've seen 1.2.3 floating around, but
it's nowhere to be found when you go to download it.  I'll poke around with the
rpmforge package and see if I can glean any info from it.
Comment 3 Jason Tibbitts 2006-05-16 14:29:32 EDT
Your Source: URL is fine if you just change the version to 1.2.3;
http://rsnapshot.org/downloads/ has all of the released versions.  I checked CVS
and it looks like they're going to release 1.2.4 soon.

Looking at the rpmforge .spec, there's not too much differece once you ignore
whitespace and your section separators.

Rpmforge spec:
Uses macros on the %configure line instead of hardcoding the paths.
Has lots of BuildReqires; I'm not sure if they're necessary.
Neatly calls logger in %post to make sure that errors get logged somewhere.
Patches the default config.
Packages the utils directory as %doc (which should annoy rpmlint; extras doesn't
generally like executable documentation).

Your spec:
Does that hack to avoid configure freaking out on an existing config file. Nice.
Does Require: openssh instead of openssh-clients; I'm not sure which is correct.

I'll attach the rpmforge spec.
Comment 4 Jason Tibbitts 2006-05-16 14:30:31 EDT
Created attachment 129254 [details]
rpmforge specfile
Comment 5 Chris Petersen 2006-05-16 15:34:17 EDT
I'll take a look at this when I get home from work.

The require is probably openssh-clients, I didn't know it was a separate package.

fyi, "my" spec is just a reworking of the upstream spec, so I can't claim
responsibility for the cool configure script.  Looks like Dag's spec has the
same stuff.

I also just noticed that the last changelog date was completely wrong..  I typed
"may 21" instead of "may 13" (which is doubly bad since I probably meant to type 12)
Comment 6 Chris Petersen 2006-05-17 23:59:53 EDT
ok, spec and src.rpm have been updated to take the best of both.  I've also been
offered the job of maintaining the upstream spec (which should be the same thing
as this one).

Spec URL: http://forevermore.net/files/packages/rsnapshot/rsnapshot.spec
SRPM URL: http://forevermore.net/files/packages/rsnapshot/rsnapshot-1.2.3-1.src.rpm
Comment 7 Jeffrey C. Ollie 2006-05-18 12:23:23 EDT
rsync and openssh-clients need to be BuildRequires too.
Comment 8 Chris Petersen 2006-05-18 12:31:26 EDT
Didn't realize that configure actually checked for those.  Fixed, and also
removed the perl requirement, which rpm finds on its own (was told in other
places not to include perl as a build requirement because it's "standard", or
something like that)

Spec URL: http://forevermore.net/files/packages/rsnapshot/rsnapshot.spec
SRPM URL: http://forevermore.net/files/packages/rsnapshot/rsnapshot-1.2.3-2.src.rpm
Comment 9 Jason Tibbitts 2006-05-18 12:42:04 EDT
Perl is in the default set of packages; it is recommended that you not included
it but this is not a blocker.

I'll do a full review after the FESCO meeting.
Comment 10 Jason Tibbitts 2006-05-19 00:28:50 EDT
I got delayed a bit, but here you go:

The only questions I have relate to the %post script.  Are there any other
possible versions other than "unknown" and 1.2?  The comment indicates that the
"latest version is 1.2" which seems to allow for earlier versions.  If there
are, things seem to be busted.

* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* license field matches the actual license.
* license is open source-compatible; license text is included in the package.
* source files match upstream:
   b27d90886b25d0e160b267f98c605aec  rsnapshot-1.2.3.tar.gz
   b27d90886b25d0e160b267f98c605aec  rsnapshot-1.2.3.tar.gz-srpm
* latest version is being packaged.
* BuildRequires are proper.
* package builds in mock (FC5, x86_64).
* rpmlint is silent.
* final provides and requires are sane.
* no shared libraries are present.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
O %check is not present; no test suite upstream.
? scriptlets are present.  They seem sane to me, but see questions above.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.
Comment 11 Chris Petersen 2006-05-19 00:57:39 EDT
I honestly don't know about the version thing.  The script originally came from
upstream, was modified by Dag and then by myself to be a little clener.  My
guess is that there was the "old" format (aka "unknown"), and then the 1.2
format.  If it's a serious concern, I can verify with the current project
maintainer -- just let me know.  I personally haven't seen anything other than
1.2, which has been around for a long time.
Comment 12 Michael Schwendt 2006-05-19 10:02:42 EDT
rsnapshot is in Fedora Extras already. Check owners.list before
reviewing. If binaries are missing, bug the owner.
Comment 13 Jason Tibbitts 2006-05-19 10:15:36 EDT
rsnapshot was checked into owners-list basically when that file was first
imported into CVS (11-Jul-05).  It has never been built for any distro that I
can see.  I assumed it was just an artifact, but it was imported  into CVS on
12-Apr-2005 and has not changed since.

I don't think this package is "really" in extras.  It seems to be left over from
some initial mass import.

Chris, could you ping ghenry@suretecsystems.com and see about taking over this
Comment 14 Gavin Henry 2006-05-19 13:59:05 EDT
Hi all,

I was originally using rsnapshot for a client, but no longer use it anymore.

It was only ever brought in via the devel tree, but doing a cvs up brings in
FC-4 and FC-5.

It must have slipped by me, as I maintain quite a few and I am currently too
busy for this one as well.

I would not mind at all if Chris wishes to take it over.

Please feel free to transfer ownership.

Many thanks,

Comment 15 Jason Tibbitts 2006-05-19 16:39:06 EDT
Well, Chris, it's all yours.  Announce to extras-list that you're taking this
package over (reference this bug), edit owners.list and have at it.
Comment 16 Jason Tibbitts 2006-05-19 16:41:43 EDT
BTW, I'm going to go ahead and approve this, since the submitted package is
fine, even though technically this review wasn't needed.
Comment 17 Chris Petersen 2007-05-28 16:18:28 EDT
Package Change Request
Package Name: rsnapshot
New Branches: EL-5 F-7

Comment 18 Kevin Fenzi 2007-06-22 15:39:55 EDT
cvs done.
Comment 19 Steven Roberts 2014-10-13 00:13:50 EDT
Package Change Request
Package Name: rsnapshot
New Branches: epel7
Owners: strobert
Comment 20 Kevin Fenzi 2014-10-13 18:54:04 EDT
Git done (by process-git-requests).

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