Bug 241553

+ simple build
+ mock build

- rpmlint safekeep-server-1.0.0-2.noarch.rpm (correct at least the
conffile-without-noreplace-flag warning)

W: safekeep-server spurious-executable-perm
/usr/share/doc/safekeep-server-1.0.0/safekeep-test
E: safekeep-server non-standard-uid /var/lib/safekeep safekeep
E: safekeep-server non-standard-gid /var/lib/safekeep safekeep
E: safekeep-server non-standard-dir-perm /var/lib/safekeep 0750
E: safekeep-server non-standard-uid /var/lib/safekeep/.ssh safekeep
E: safekeep-server non-standard-gid /var/lib/safekeep/.ssh safekeep
W: safekeep-server hidden-file-or-dir /var/lib/safekeep/.ssh
E: safekeep-server non-standard-dir-perm /var/lib/safekeep/.ssh 0700
W: safekeep-server hidden-file-or-dir /var/lib/safekeep/.ssh
W: safekeep-server conffile-without-noreplace-flag /etc/safekeep/safekeep.conf
W: safekeep-server doc-file-dependency
/usr/share/doc/safekeep-server-1.0.0/safekeep-test /usr/bin/python
W: safekeep-server dangerous-command-in-%post mv
W: safekeep-server dangerous-command-in-%preun userdel

- openssh requirement seems not necessary (is yet required by openssh-clients)
- gecos field for the safekeep user ("Useful comment about the purpose of this
account")
- as this seems to be the a new package for Fedora, i don't understand the
migration of conf files from /etc/safekeep.d to /etc/safekeep/backup.d
- man pages for safekeep.backup and safekeep.conf should be in the
safekeep-server rpm

FYI,
I'm going in to have my wisdom teeth removed on Friday morning. As a result I'm
going to be losing a chunk of discretionary time over the weekend. I don't
expect to be able to get back to working on this for a week or so as I play
catch up from the downtime.

-jef

The 'openssh' requirement is needed since we make use of 'ssh-keygen' and that
exists in 'openssh', not in 'openssh-clients'. That being the case, I think it's
better to be explicit instead of relying on an implicit dependency
(openssh-clients depends on openssh) that may or may not hold true in the future.

Jef: are you still taking care of with this? If you're too busy, I can take this
to the finish.

Folks, just for the record the latest version of safekeep (1.0.1) fixes all the
above mentioned problems. I believe it is ready for inclusion into Fedora
without any need for additional patches.

(In reply to comment #4)
> Jef: are you still taking care of with this? If you're too busy, I can take this
> to the finish.
> 

I'm at a conference this week, with limited net access, so I can't do much this
week. If you want to step in as the lead maintainer that would be okay with me. 
I've been working close with dimi upstream to get everything working, including
using a nologin shell for the default user.  The latest upstream rpms should
address everything that needs to be fixed.  This will never have a clean rpmlint
output because the user that is created uses a non-default location...to hold
generated ssh keys and whatnot. 

-jef"summer conference season, a boon and a curse to productivity"spaleta

No worries -- since you've invested so much time in it, I'll wait until you have
a spare moment. Cheers!

(In reply to comment #1)
> - as this seems to be the a new package for Fedora, i don't understand the
> migration of conf files from /etc/safekeep.d to /etc/safekeep/backup.d


I'll say more concerning the other items later tonight....but the migration
stuff is there primary to help users transition from previous rpm versions as
provided by upstream.  I've been working closely with upstream to develop the
rpm version for a number of minor releases.  The goal with transition scripts is
to make sure that fedora users who were previously using upstream rpms can
easily migrate to the fedora rpms once they hit the repos.

-jef

Jeff,

Thank you for the clarification. However, since we're past the big "1.0" release,
I have decided to remove the migration scripts, since those were meat to help
people migrate from the format used by the very first release. I am quite sure
all affected people have upgraded already, and so to avoid any discussion on the
topic I just removed them in 1.0.1. They were meant as a temporary solution
anyway, and this is a time as good as other to remove them.

New 1.0.1-2 srpm and spec file now available.
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.1-2/safekeep-1.0.1-2.fc7.src.rpm
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.1-2/safekeep.spec
Binaries in:
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.1-2/F-7/
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.1-2/devel/

The new srpm/spec/binaries  remove the migration scripts and the manpages have
been moved to the server subpackage as per comment #1.

Rpmlint runs clean for the common and client subpackages.  The server subpackage
still illicits messages from rpmlint due to the safekeep user using
/var/lib/safekeep/ for its home directory to hold generated ssh keypairs.

E: safekeep-server non-standard-uid /var/lib/safekeep safekeep
E: safekeep-server non-standard-gid /var/lib/safekeep safekeep
bugus errors.  /var/lib/safekeep is being as the home directory for the
generated safekeep user.  There examples of system user home directories in
/var/lib/ already including the rpm user

E: safekeep-server non-standard-dir-perm /var/lib/safekeep 0750
E: safekeep-server non-standard-uid /var/lib/safekeep/.ssh safekeep
E: safekeep-server non-standard-gid /var/lib/safekeep/.ssh safekeep
W: safekeep-server hidden-file-or-dir /var/lib/safekeep/.ssh
E: safekeep-server non-standard-dir-perm /var/lib/safekeep/.ssh 0700
W: safekeep-server hidden-file-or-dir /var/lib/safekeep/.ssh
Bugus errors and warnings, the .ssh directory is configured thusly to work
correctly with the default sshd configuration for fedora.

-jef

Ping?

I'm just waiting for a reviewer comment at this point. This isn't assigned to a
reviewer currently.

I was going to go hunting for a reviewer next week if someone doesn't step up
this week.

-jef

I cannot download the current spec file:

$ curl -I
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.1-2/safekeep.spec
HTTP/1.1 403 Forbidden
[...]

The srpm works. Here are some first observations:
- GPL is not a valid value for the license tag anymore:
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#head-f21ae23bf2f278444e2c385463cfa74a502396b8

- Source0 should be:
http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
not _pr_downloads...

- the client subpackage seems to be empty, so imho you should rename the common
package to client and let the server package depend on the client. Also it seems
to be odd that the client package has a lot of Requires


- Afaik there is no need to package "AUTHORS COPYING LICENSE" multiple times,
packaging it in the client (common) package should be enough.

- does this "Requires:       safekeep-common = %{PACKAGE_VERSION}" really work?
I cannot see where PACKAGE_VERSION is defined and the guidelines mention
%{version}-%{release}" instead.

- every %files section has to start with a %defattr(...) line, but only the
%files section for common has one

Thanks for the review Till!

Regarding the stub client package, I have considered eliminating it,
but I'm afraid this would just complicate matters going forward, for
the following reasons:
  * our plans will require that separate package in the near future,
    so eliminating it just to add it a bit later on will just make
    a mess of deps.
  * the Requires: section is different for the client, and collapsing
    things into a single package will pull in things that are not
    required on the server/client.
Overall, I think we're better off with the additional package.

As for the rest of your comments, they are easy to address, I'll do
so in the next minor release.

I have just released version 1.0.2 that addresses (most of) the issues mentioned
above (except for the -client package, as explained above):
   
http://downloads.sourceforge.net/safekeep/safekeep-1.0.2-1.fc7.src.rpm?use_mirror=osdn

(In reply to comment #15)

Okay I'll take a look at it tonite. 
For those of you playing along. I'm working as closely as possible with Dimi as
the upstream developer to keep the fedora packaging in sync with the upstream.
I'm still expecting to act as the maintainer for the Fedora packages, and the
gate-keeper to the bits in Fedora cvs until Dimi gets Fedora contributor status
and we can share maintainership duties in Fedora space for this package.

-jef

(In reply to comment #16)

> Okay I'll take a look at it tonite. 

Then please assign the bug to you and set the fedora-review flag to "?".

(In reply to comment #17)
> (In reply to comment #16)
> 
> > Okay I'll take a look at it tonite. 
> 
> Then please assign the bug to you and set the fedora-review flag to "?".

Forget this, I was confused and it is too early here. :-)

Okay, updated urls for the spec and srpm being submitted for review. Essentially
the same as Dimi's upstream spec from comment 15.

Spec:
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.2-1/safekeep.spec
SRPM:
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.2-1/safekeep-1.0.2-1.fc7.src.rpm
F-7 noarch RPMS:
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.2-1/F7/

All remaining rpmlint errors are the result of using the /var/lib/safekeep
directory as the home directory for the safekeep user and including a .ssh
sub-directory for ssh keys. As previously discussed in earlier comments, these
errors are erroneous for this atypical but appropriate usage of
/var/lib/safekeep as a home directory. 

The client package is still essentially a metapackage due to the difference in
the server and client dependency chain.  Though as upstream has mentioned, there
 are plans to add additional client package payload so keeping it as a separate
sub-package package now keeps from having to complicate things later as more
client-side development is added.

-jef

Folks, any news on this item? It's getting a bit long in the tooth...

I'd be happy to give this a formal review in the next few days here unless
someone beats me to it. 

Look for a full review in a few here. 

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
See below - License (GPLv2+)
See below- License field in spec matches
See below- License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
d37ae719280f654d5bad27fa23ce79bd  safekeep-1.0.2.tar.gz
d37ae719280f654d5bad27fa23ce79bd  safekeep-1.0.2.tar.gz.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
See below - final provides and requires are sane

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should have sane scriptlets.
OK - Should have subpackages require base package with fully versioned depend.
OK - Should have dist tag
OK - Should package latest version

Issues:

1. What is the license here?
The LICENSE file in the source says GPLv2.1
The spec has GPLv2+
The scripts themselves don't have any mention of a license.
The web site says GPL
http://safekeep.sourceforge.net/license.shtml

Ideally can you get upstream to say if it's GPLv2 only, GPLv2+, or GPL+?
A comment in the scripts themselves would be good.
Also matching up so that all the above say the same thing would be good.

2. This is 1.0.2, but the website seems to have 1.0.1 as latest.
Is this a preview release or they just haven't updated the web site yet?

3. rpmlint says:

safekeep.src: W: strange-permission safekeep.spec 0600
safekeep-client.noarch: W: no-documentation 
safekeep-server.noarch: E: non-standard-uid /var/lib/safekeep safekeep
safekeep-server.noarch: E: non-standard-gid /var/lib/safekeep safekeep
safekeep-server.noarch: E: non-standard-dir-perm /var/lib/safekeep 0750
safekeep-server.noarch: E: non-standard-uid /var/lib/safekeep/.ssh safekeep
safekeep-server.noarch: E: non-standard-gid /var/lib/safekeep/.ssh safekeep
safekeep-server.noarch: W: hidden-file-or-dir /var/lib/safekeep/.ssh
safekeep-server.noarch: E: non-standard-dir-perm /var/lib/safekeep/.ssh 0700
safekeep-server.noarch: W: hidden-file-or-dir /var/lib/safekeep/.ssh

All can be ignored.

4. Perhaps you could include a README.fedora in the client subpackage
explaining why it's currently empty?

5. The client subpackage has:
Requires:       openssh-server
Requires:       coreutils
Requires:       util-linux

Are those really required? Why?

(In reply to comment #23)
> See below - License (GPLv2+)
> See below- License field in spec matches
> See below- License file included in package
> Issues:
> 
> 1. What is the license here?
> The LICENSE file in the source says GPLv2.1
> The spec has GPLv2+
> The scripts themselves don't have any mention of a license.
> The web site says GPL
> http://safekeep.sourceforge.net/license.shtml
> 
> Ideally can you get upstream to say if it's GPLv2 only, GPLv2+, or GPL+?
> A comment in the scripts themselves would be good.
> Also matching up so that all the above say the same thing would be good.
> 
> 2. This is 1.0.2, but the website seems to have 1.0.1 as latest.
> Is this a preview release or they just haven't updated the web site yet?

The website itself needs to be updated. But sf.net project space has 1.0.2
listed as the most current release.
 
http://sourceforge.net/project/showfiles.php?group_id=185128
> 4. Perhaps you could include a README.fedora in the client subpackage
> explaining why it's currently empty?

Easily done. I'll get to it tomorrow.

> 5. The client subpackage has:
> Requires:       openssh-server
> Requires:       coreutils
> Requires:       util-linux
> 
> Are those really required? Why?

okay.. the way this works is that the safekeep server process pulls updates from
clients over ssh.  The server sets up special ssh keys on the client
specifically for this so the rdiff-backup process can be automated using
passphrase-less ssh keys constrained to run only the rdiff-backup commands. So
the client needs a running ssh server. I believe coreutils and util-linux are
needed for helper commands that allow the server to interact with the client to
setup the specialized ssh keys. I can dig into the code tomorrow and confirm the
specifics of that.


-jef

> 1. What is the license here?

GPLv2+. I'll try to add necessary notices.

> 2. This is 1.0.2, but the website seems to have 1.0.1 as latest.
> Is this a preview release or they just haven't updated the web site yet?

Website needs an update.

> 5. The client subpackage has:
> Requires:       openssh-server
> Requires:       coreutils
> Requires:       util-linux
>
> Are those really required? Why?

Yes, they are. As Jeff explained, the client needs to be contacted by the server
via SSH. It also may need su(1) to run the dump commands.

(In reply to comment #25)
> > 1. What is the license here?
> 
> GPLv2+. I'll try to add necessary notices.

Okay can you roll up a 1.0.3 release with the correct copyright notices in the
source code files? 

> Yes, they are. As Jeff explained, the client needs to be contacted by the 
> server via SSH. It also may need su(1) to run the dump commands.


I'll have a short README.fedora ready to roll tonite that touches on this to put
into the docs section of srpm which includes your new tarball.

-jef

I have just released 1.0.3 with a few fixes and the license added to the source
files. I've also updated the website to reflect the new release too.

Please let me know if there are any other outstanding problems.

    

Okay the licensing situation looks clear to me now.

I'm adding a README.fedora document which reads:

Safekeep is designed such that the same script can be used on both the 
backup server (which initiates backups from clients), as well as on 
individual clients to handle client specific tasks such as 
dedicated ssh key generation and client-side backup processes such as dump. 

As a result, on Fedora safekeep is packaged in such a way that Fedora 
clients which want to communicate with a safekeep managed backup server 
can choose to install the safekeep-client subpackage to ensure correct 
safekeep client configuration. This safekeep-client subpackage does not 
itself install any additional files at this time. But it does ensure 
that the correct client side applications are in place for safekeep 
managed backup services.

It is recommended that all computers acting as safekeep clients install 
the safekeep-client subpackage, even though at this time it is 
technically optional if the necessary components such as an ssh server, 
coreutils and util-linux are installed correctly on the client Fedora 
system. In future version of safekeep the safekeep-client package may 
become necessary for correct client operation.


Any objections to the text?
-jef

New spec and srpm with README.Fedora addition:

http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.3/safekeep.spec
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.3/safekeep-1.0.3-2.fc7.src.rpm

No significant differences in the spec since the last 1.0.2 spec other than the
addition of the Fedora README and the Licensing clarification as per the
reviewers comments.

Binaries built against devel located in:
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/safekeep/1.0.3/devel/
no new rpmlint issues.

Balls in the reviewers court now.
-jef

All issues I saw are solved with the package from comment #29. 

Thanks for the great work... this package is APPROVED. 
Don't forget to close this once it's been imported and built. 

Also, since rdiff-backup is available in EPEL, do consider EPEL-4 and EPEL-5
branches if it otherwise works there. ;) 

Good stuff guys, thanks a lot for the prompt response!
Please let me know if there's anything else that I need to do.

New Package CVS Request
=======================
Package Name: safekeep
Short Description: client/server backup system using rdiff-backup
Owners: jspaleta
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes


Note: I'm requesting F-8 to cover my bases, because I'm not sure if F-8
branching completed or not.

cvs done.

Built against devel, F-8 and F-7.
I'm going to leave this in F-7 updates-testing until F8 comes out and the update
tree for F8 is populated to make sure we have a working upgrade path.

EPEL will have to wait till I have a work related reason to maintain EPEL
packages, which should be not so far away.

Closing ticket NextRelease
-jef

Package Change Request
======================
Package Name: safekeep
New Branches: el5 el6
Owners: jspaleta frankcrawford

Git done (by process-git-requests).