[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFS: Folding@home



First off, I'd like to thank you for spending the time to construct this useful analysis!

Frank Küster wrote:
Nick Lewycky <nicholas@mxc.ca> schrieb:

Package name : folding
Version      : 4.00
Upstream     : http://folding.stanford.edu
URL          : http://wagon.dhs.org/folding/
Description  : Folding@home Client (install package)
WNPP bug     : http://bugs.debian.org/261257


Some further remarks:

- Why is the version 4.00? If this is the version of the upstream
  software, I would rather name the package folding@home4-installer, or
  - if it doesn't matter - omit this version completely, and just give
it its own versioning.

The upstream version is 4.00. Moving the 4 to the package name is wrong and will break upgrades to version 5 some day. Removing the version from the package version is sub-optimal (but permissible) by Debian Policy, section 3.2.1. Is there a compelling reason to remove it?

I'd also like to avoid long package names which don't fit in the first column of "dpkg --list". I also want the package name to match the username it creates. How about "fahclient"?

- there's a superfluous templates.pot in the top-level directory

Thanks. I'll fix that RSN.

- I'm not sure, but is it common practice to put installer packages for
non-free software into contrib?

The "distributed-net" package is in non-free while the "setiathome" package is in contrib. A concensus would be a good thing here. From my reading, contrib is correct.

- debian/copyright: You HAVE TO have a copyright statement for the code
  that you wrote in the installer package, and it has to be have
  DFSG-free license to be able to go into contrib or main. It's probably
  a good idea to also include upstreams license, but NOT in the
  copyright file of your package.

Thanks. I'll fix that too.

- you should remove commented lines and unnecessary targets from
debian/rules.

Ok.

- your postinst seems to write into files "client.cfg" in the current
  directory. This is bad - there might be other files there with the
  same name. Later you repeat it in /var/lib/folding - why the
duplication?

It's not supposed to be in the current directory, the error is that I was missing "cd /var/lib/folding" inside of the if.

The reason for the duplication is simple. That test is for whether this package is currently being reconfigured. Not to be confused with configuration, which happens on first install only. We don't want to rerun adduser on reconfigure.

- the stale links in /var/log seem odd.

That's because they are!

It's a wart. FAH4Console-Linux.exe outputs FAHlog.txt in its working directory. It also manages its own log rotation into FAHlog-Prev.txt. I decided that links to these files in /var/log was the best way to comply with Debian Policy section 10.8.

- It seems that the main work of the package is done in the postinst. I
  would suggest that you put this into a separate script, with the
  option to call it from postinst, or to delay that to a later
  timepoint. If this is included in Debian, people will chose it in
  their first big "Ah, now let's look what cool packages are on those
  disks" install run, and then it's annoying to see the modem dialling
  in postinst to get that file, while in fact you are eager to test all
  the cool stuff you just selected. And there may be no network
connection at all during installation.

(Refer to "I see a theme here..." below.)

- the postinst calls /etc/init.d/folding restart before the user is
  created (reconfigure may be called after the user account has been
  accidently deleted!). That will either not work, or have unexpected
  consequences.

No it doesn't. Notice that there's an "exit 0" right afterwards? That's the reconfiguration-only branch of the code. (By which I mean a manual run of dpkg-reconfigure which implies that configuration (the installation step) has already occured.)

I'll grant that the logic throughout postinst is highly convoluted. Does POSIX sh support functions? I avoided them in case they are a bash-ism, but if not, they would allow me to greatly clarify the workings of postinst.

- I doubt that "folding" is a good name for the user. The name may be in
  use for a local user who owns group-public protein folding data.

That's a general problem which can occur with any name I choose, or with other packages. I'd be willing to go for "fahclient" as a username, but not "foldingathome", "fahlinux", "folding@home" or most variants. I like "folding" since it fits within the username column as displayed by top.

- please see the recent thread here in the group about
  name-group-separators for chown.

Thanks. Fixed.

- the #DEBHELPER# entry inside an if-statement seems to be wrong. Oh, I
  see, you repeat it later. Hm. still it seems odd.

Right after #DEBHELPER# is "exit 0" so what I'm doing here is making certain that #DEBHELPER# happens before I exit. The inside of that if is the standard upgrade case where the client on disk matches the md5sum on the package and there's no reason to download a new one.

- You REALLY shouldn't let the postinst fail if the download
  fails. Nobody will be able to fix and finish the installation (of your
  and other packages) if the internet connection, or maybe just the
proxy is down.

I see a theme here: you want to seperate the installer package installation from the downloading step. I find that a bit awkward myself, but then I'm always online.

Would it be ok if it tried to download, failed gracefully (allowing the package to install) by offering a script that would perform the installation manually? And online users would see it work automatically with no intervention?

- there's no documentation. At least you should try to (get permission
  to) include the command line options in
http://folding.stanford.edu/console-userguide.html.

Hm, that's because you're not supposed to run it directly. You're supposed to let it add itself into your runlevels and work away. Keep those filthy hands off! :)

I would advice not to make a Debian-native package of this. It's quite
simple, and it can as well be used on an rpm-based distribution, without
modification of the main script (postinst) - therefore other
distributions might re-use it.

Ok, I'll change that.

Furthermore, I think that an installer package is only really useful if
it creates a Debian package out of the downloaded stuff - that's the
whole point of a package managment. Although, I admit, I cannot
currently imagine a program that would want to depend on folding@home.

There are 3rd party utilities for Folding@home:

fpd & qd:
http://boston.quik.com/rph/fah.html

qdcsv:
http://homepage.mac.com/iam_kranki2/qdcsv.html

KFoldingApplet:
http://home.houston.rr.com/epasveer/

kfolding:
http://members.shaw.ca/khessels/kfolding/

KFold monitor:
http://www.kdfold.com/

... and others.

As for creating a package out of the downloaded content, there's no benefit to doing that. The installer package is a proxy for the real thing. If you install it, you get Folding@home. If you remove it, you remove Folding@home.

(Note that your suggestion that I move the install out of postinst breaks this illusion slightly. I'll have to wait until this time tommorow before I decide how I feel about that.)

One other question: The following text on the website makes me think
that your md5sum check will always fail:

,----
| Each different running copy of F@H has to have its own Machine ID
| number. If you download each copy of F@H from the web site and install
| fresh, there will be no problems.
`----

No, the client creates "machinedependent.dat" in the current working directory the first time it is run. The downloaded file is always the same.

Have you checked (without a proxy)?

Actually I have. What I haven't checked is *with* a proxy since it would take me a while to set up a proxy just to test my package on. I'd really appreciate if someone else would test that for me.

I'm going to make the remaining changes and upload new packages soon. Thanks a lot for your comments!

Nick Lewycky



Reply to: