Merge lp:~mvo/launchpad/support-timeframe-fix-660433 into lp:launchpad/db-devel

[Michael Vogt]

Update support-timeframe info for NEW packages.

Hello,

this branch fixes the soyuz bug #660433. It will re-generate the support-timeframe information (Supported tag in more-extras.overrides) for all supported distros to update it for packages like new kernels that get added as NEW packages after the final release.

I added a simple test for the functionality. I'm not sure it follows the guidelines, there was no tests/ directory under cronscripts/publishing (and the test is done with sh).

Its also prepares mock-data (./mock-data/ubuntu-archive/ubuntu) for testing germinate integration. This is not used (yet?), so that part of the tree could be removed (this would make the diff less cluttered).

Someone with soyuz experience (and knowledge about e.g. the content of "crontab -l") needs to review this carefully as it will affect the extra-overrides file from stable distros and therefore modify stuff in $distro-updates.

Cheers,
 Michael

[Michael Vogt (mvo)]

It also modifies maintenance-check.py to put information only for lucid and later. While the script will work for hardy as well it does not make sense to generate "Supported" info in hardy-updates for it when there is none for the main archive.

[Robert Collins (lifeless)]

That sample data will add to the tree size and isn't necessary or
desirable - could you please uncommit back to before you added it and
resubmit? Thanks.

Binary files in bzr is a bad idea, and anytime it seems like a good
one... its not :)

-Rob

[Michael Vogt (mvo)]

Hey Robert, thanks for your review. I'm happy to remove the data. Please note that its empty gz files currently, so the data added to bzr should be small. But I'm fine with removing it as its currently not required (something small binray (one or two stanzas will be required when we test the germinate output though, so advise is welcome how to handle this).

[Robert Collins (lifeless)]

On Fri, Oct 15, 2010 at 8:57 PM, Michael Vogt <email address hidden> wrote:
> Hey Robert, thanks for your review. I'm happy to remove the data. Please note that its empty gz files currently, so the data added to bzr should be small. But I'm fine with removing it as its currently not required (something small binray (one or two stanzas will be required when we test the germinate output though, so advise is welcome how to handle this).

We already have such data elsewhere in the tree, or we can generate it
on demand.

We're currently working on parallel testing too, which means that
tests *cannot* depend on mutating data in the tree.

(and yes, you'll need to have it be a python unittest).

-Rob

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9888. By Michael Vogt on 2010-10-15

remove all binary data from the tree and generate it on the fly

9889. By Michael Vogt on 2010-10-15

cronscripts/publishing/tests/test_cron.germinate.py: convert test to python

9890. By Michael Vogt on 2010-10-15

cronscripts/publishing/tests/test_cron.germinate: remove shell script test

[Michael Vogt (mvo)]

I generate the data on the fly now and converted the shell to proper unittest. Please have another look.

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9891. By Michael Vogt on 2010-10-15

clear mock ubuntu/dists data too

[Jeroen T. Vermeulen (jtv)]

Hi Michael,

Thanks for the branch. I'm ecstatic to see someone work on these scripts, especially with tests attached; but a bit uncomfortable reviewing the changes. Most of the existing code seems absolutely ancient and ill-matched to today's source tree. I'll resist the temptation to say "while you're here, could you also clean up this bit of other code" a lot. :-)

Julian is still looking at the testing and design sides, but I'll review the code on the assumption that that will come out okay. Lots of notes though. I hope you won't be put off by them!

=== modified file 'cronscripts/publishing/cron.germinate'
--- cronscripts/publishing/cron.germinate 2010-09-06 09:46:01 +0000
+++ cronscripts/publishing/cron.germinate 2010-10-15 12:12:43 +0000

@@ -126,10 +125,19 @@
 done
 echo " done."

-# now generate the Supported extra overrides
-$MAINTAINCE_CHECK $suite > "$MISCROOT/more-extra.override.$suite.main.supported" 2> _maintenance-check.stderr
-if [ $? -eq 0 ]; then
- cat "$MISCROOT/more-extra.override.$suite.main.supported" >> "$MISCROOT/more-extra.override.$suite.main.new"
-fi
-
 mv -f "$MISCROOT/more-extra.override.$suite.main.new" "$MISCROOT/more-extra.override.$suite.main"
+
+# now generate the Supported extra overrides for all supported distros
+for supported_suite in `$LAUNCHPADROOT/scripts/ftpmaster-tools/lp-query-distro.py supported`; do

The lines in this script get pretty long. Could you place the "do" on a separate line?

for supported_suite in `$LAUNCHPADROOT/scripts/ftpmaster-tools/lp-query-distro.py supported`
do
    # ...
done

+ echo -n "Running maintenance-check for $supported_suite... "
+ if $MAINTAINCE_CHECK $supported_suite > "$MISCROOT/more-extra.override.$supported_suite.main.supported" 2> _maintenance-check.$supported_suite.stderr; then

And this one is now definitely too long! It'd help if these enormous filenames were in sensibly-named variables. Makes it easier to get an overview of what's happening.

+ # make sure the more-extra.override file exists
+ touch "$MISCROOT/more-extra.override.$supported_suite.main"

This comment just re-words the code. Instead, could you make it say very briefly _why_ it does this?

(Also, contrary to what you see in these scripts, please start your sentences with capital letters and punctuate them!).

+ # remove old "Supporte" info from extra overrides

Typo: missing "d." Also, please capitalize & punctuate.

+ sed /"^.* Supported"/d "$MISCROOT/more-extra.override.$supported_suite.main" > "$MISCROOT/more-extra.override.$supported_suite.main.new"
+ cat "$MISCROOT/more-extra.override.$supported_suite.main.supported" >> "$MISCROOT/more-extra.override.$supported_suite.main.new"
+ mv "$MISCROOT/more-extra.override.$supported_suite.main.new" "$MISCROOT/more-extra.override.$supported_suite.main"
+ fi
+ echo " done"
+done

Does this piece of the script want to live in its own function?

=== modified file 'cronscripts/publishing/maintenance-check.py'
--- cronscripts/publishing/maintenance-check.py 2010-05-28 16:17:51 +0000
+++ cronscripts/publishing/maintenance-check.py 2010-10-15 12:12:43 +0000

@@ -297,7 +297,11 @@
    ...

review: Needs Fixing

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9892. By Michael Vogt on 2010-10-20

cronscripts/publishing/cron.germinate: address merge review issues raised by jtv

9893. By Michael Vogt on 2010-10-20

cronscripts/publishing/maintenance-check.py: fix missing capitalization and punctation and kill off whitespace

9894. By Michael Vogt on 2010-10-20

cronscripts/publishing/tests/mock-data/mock-bin/germinate: add comment in the script header

9895. By Michael Vogt on 2010-10-20

cronscripts/publishing/tests/mock-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py: update to include comment and error checking and a else claue

9896. By Michael Vogt on 2010-10-20

cronscripts/publishing/tests/test_cron.germinate.py: address a lot of the review points, punctation, test template, variable naming, comments and docstrings (more to come)

[Martin Pool (mbp)]

Thanks for reviewing it, Jeroen. I want to contribute a meta-review.

First off, thanks for reviewing this quickly and expressing enthusiasm for what mvo did. Those two things make a big difference to the speed at which anyone can contribute and they're especially important for new contributors.

> + gz = gzip.GzipFile(os.path.join(d, "Sources.gz"), "w")
> + gz.close()
>
> Here too it would be nice to have a helper: "create gzip file."
>
>
> + # binaries
>
> Cap & Punct.
>
>
> + for arch in self.ARCHES:
> + for p in ["", "debian-installer"]:
>
> Please replace "p" with a full variable name.
>
>
> + d = os.path.join(
>
> Same for "d."

This kind of review comment I think is not so good, for a couple of reasons.

It's bad scaling behaviour to manually comment on all N instances of a problem, when a single comment would cover all of them. It seems to me that, like in my recent contribution, the problem is not that mvo made some particular mistakes, but that he just didn't know of some Launchpad conventions. I think it would be better to have a shorter review with the coding standard URL and pointing out the particular bits that are relevant. It will be quicker for you too.

I don't know how much mvo has participated in lp development before but I'm guessing not that much. So he probably won't know he ought to run 'make lint', or the quirk that it only covers currently uncommitted code.
However when somebody is putting up an early patch that violates these pretty minor rules, I wonder how much it is worth getting them to go back and eg remove whitespace. istm it is better to build up more knowledge of conventions in their head, and more enthusiasm for the project. Again, probably better scaling behaviour.

Saying "I don't understand this, please add a comment" or "you don't need this" is of course fine.

my 2c.

[Jeroen T. Vermeulen (jtv)]

Martin, you make a good point. I was actually just trying to be helpful: since I'm "proof-reading" the entire diff anyway, there's no need for Michael to go over the whole thing to find each individual instance that needs changing. I just noted them consistently, as is common practice in Launchpad reviews.

I'm used to this practice and don't feel in any way accused when a reviewer does it for me (or accusatory when I do it for someone else). But now that you mention it, I suppose it does need an explanation for new contributors.

[Jeroen T. Vermeulen (jtv)]

The starting point for the ins and outs of writing Launchpad branches, by the way, is here: https://dev.launchpad.net/LaunchpadHackingFAQ

Unfortunately there's rather a lot of it. The actual coding rules is in pages linked from there, notably the Python style guide.

[Michael Vogt (mvo)]

Thanks for the review Jeroen and thanks for the meta-review from Martin as well!

I update the code to adress a lot of the points, but there is more to be done. I'm happy
about the detailed review as it means I can just go over it point by point. The result
is that the code is now better and more readable. Martin is right, I don't usually do LP
development and I'm not familiar with the processes. I just want to fix this bug as it
affects the distro (badly) :)

I do not nessecarily agree with all of the points in the review (e.g. I think short variable
like "d" are fine if they are just used in a two lines or a short function). But that is fine, its more important that its consistent with the rest of the codebase.

The only "grief" I have is the testing template. To test a small part of LP like this
that does not need the database or other fancy part using the template drags in a huge
set of python dependencies, including psycopg2, storm, transactions, windmill
(that appears to be not packaged in ubuntu maverick) and more. It would be great to be
able to use the test template without all this if its not actually needed for the test
at hand.

[Martin Pool (mbp)]

On 21 October 2010 15:28, Jeroen T. Vermeulen <email address hidden> wrote:
> Martin, you make a good point.  I was actually just trying to be helpful: since I'm "proof-reading" the entire diff anyway, there's no need for Michael to go over the whole thing to find each individual instance that needs changing.  I just noted them consistently, as is common practice in Launchpad reviews.
>
> I'm used to this practice and don't feel in any way accused when a reviewer does it for me (or accusatory when I do it for someone else).  But now that you mention it, I suppose it does need an explanation for new contributors.

I don't take it as accusatory at all. There are probably some things
we could do in lp reviews to help with this - it would be neat if your
editor could jump through a list of review comments with the lines
they correspond to, or perhaps if a reviewer could more easily get in
and edit the branch...

--
Martin

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9897. By Michael Vogt on 2010-10-21

cronscripts/publishing/tests/test_cron.germinate.py: refactor to create fake ubuntu archive root as a tempdir and add new helper create_directory_if_missing

9898. By Michael Vogt on 2010-10-21

cronscripts/publishing/tests/test_cron.germinate.py: move creating the germinate process environment into its own function, add gzip helper, add directory lists helper

9899. By Michael Vogt on 2010-10-21

cronscripts/publishing/tests/test_cron.germinate.py: add helper create_file()

[Michael Vogt (mvo)]

Beside the following two open issues I think I fixed all of the things mentioned.
Open is:

I don't use from lp.testing import TestCase because I could not make it work on my machine.
Adding the launchpad PPA and installing the launchpad-dependencies did not help.

I either get:
 ImportError: No module named windmill.bin.admin_lib (when importing canonical.testing.layers)
or
 ImportError: No module named fixtures (when importing from lp.testing import TestCase only)
I guess I do something wrong but I don't know what.

The other one is that I did not change the directory layout yet (where the tests lives).

[Colin Watson (cjwatson)]

I don't expect problems here from a deployment point of view; obviously (say) dists/lucid/ won't be changed, as opposed to dists/lucid-updates/, but that's OK. I trust you're aware that this means that it's only possible to generate updated Supported fields for packages that have been changed or added in lucid-updates.

One thing that jumped out at me, although I realise it was in the pre-existing code too:

- if distro[0] < 'h':
- print "ERROR: only hardy or later is supported"
+ if distro[0] < 'l':
+ logging.error("only lucid or later is supported")

We're only six years or so away from wrapping around the end of the alphabet. Perhaps this needs to be more explicit somehow? Of course, having to list all historical release names here isn't particularly great either ...

[Michael Vogt (mvo)]

Hello Colin, thanks for this feedback. The generated Suported fields will indeed be only be updated for changed or added Packages. But that is ok, before lucid-final we reviewed the data to made sure that it is sane. The open issue is that newly published kernels to not get the right Supported field currently and that makes ubuntu-support-status print out wrong data for those.

The wrapping is a good point, I just added the relevant historic names to make it explicit.

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9900. By Michael Vogt on 2010-10-21

cronscripts/publishing/tests/mock-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py: return more supported distros to test filter in maintenance-check.py

9901. By Michael Vogt on 2010-10-21

cronscripts/publishing/maintenance-check.py: make unsupported release names explicit to avoid future problems when the codenames wrap around (thanks to Colin!)

[Julian Edwards (julian-edwards)]

167 +# mvo: I would love to use this, but it appears doing this import
168 +# requires python-psycopg2, python-storm, python-transaction,
169 +# python-lazr.restful and now "windmill" that is not packaged
170 +# at this point I give up and let someone else with a proper
171 +# LP environment fix this
172 +#from canonical.testing.layers import DatabaseFunctionalLayer

In an ideal world every test would not use any layers because it's massively quicker to run the tests. However, I can't see any reason that your tests need layers so I think it's fine to remove this commented block entirely.

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9902. By Michael Vogt on 2010-10-21

cronscripts/publishing/tests/test_cron.germinate.py: remove (commented out) import for canonical.testing.layers, use abspath() for BASEPATH to ensure it works correctly when run from different dirs

[Michael Vogt (mvo)]

Thanks Julian, I removed that part of the diff now. I still can not use from lp.testing import TestCase for some reason.

[Julian Edwards (julian-edwards)]

I'd personally be happy with you using the unittest one if it means you can run your own tests.

[Jeroen T. Vermeulen (jtv)]

With apologies for the delays—holiday and crisis, yin and yang—I'm just coming back to this branch. I hope I can help with this problem:

> Beside the following two open issues I think I fixed all of the things
> mentioned.
> Open is:
>
> I don't use from lp.testing import TestCase because I could not make it work
> on my machine.
> Adding the launchpad PPA and installing the launchpad-dependencies did not
> help.
>
> I either get:
> ImportError: No module named windmill.bin.admin_lib (when importing
> canonical.testing.layers)
> or
> ImportError: No module named fixtures (when importing from lp.testing import
> TestCase only)
> I guess I do something wrong but I don't know what.

These are external modules that have recently been updated. Try running rocketfuel-get (or if that doesn't do the trick, rocketfuel-setup). Then, in your branch, run
{{{
. ~/.rocketfuel-env.sh
utilities/link-external-sourcecode "$LP_TRUNK_NAME"
make
}}}

That should update your branch to use the latest external modules. You need to re-run rocketfuel-get from time to time to keep them updated. Sometimes you also need to merge in newer versions of your parent branch.

[Jeroen T. Vermeulen (jtv)]

Thanks for all the work you did on this! Is it just me or does the diff look a lot shorter now, despite having the new bits code? It certainly seems easier to read.

=== modified file 'cronscripts/publishing/cron.germinate'
--- cronscripts/publishing/cron.germinate 2010-09-06 09:46:01 +0000
+++ cronscripts/publishing/cron.germinate 2010-10-21 19:06:53 +0000

+# Now generate the Supported extra overrides for all supported distros.
+SUITES=`$LAUNCHPADROOT/scripts/ftpmaster-tools/lp-query-distro.py supported`
+for supported_suite in $SUITES; do
+ echo -n "Running maintenance-check for $supported_suite... "
+ # The support timeframe information is stored here
+ SUPPORTED="$MISCROOT/more-extra.override.$supported_suite.main.supported"
+ # This is the target override file that contains germinate plus support info
+ TARGET="$MISCROOT/more-extra.override.$supported_suite.main"
+ # Debug/Log information
+ LOG="_maintenance-check.$supported_suite.stderr"
+ if $MAINTAINCE_CHECK $supported_suite > $SUPPORTED 2> $LOG; then
+ # The target file may be missing on the server and the script should
+ # not fail if that is the case.
+ touch $TARGET
+ # Remove old "Supported" info from extra-overrides as it may be
+ # stale now and we replace it with fresh content below.
+ sed /"^.* Supported"/d $TARGET > ${TARGET}.new
+ cat $SUPPORTED >> ${TARGET}.new
+ mv ${TARGET}.new $TARGET
+ fi
+ echo " done"
+done

This looks nice and clear.

=== modified file 'cronscripts/publishing/maintenance-check.py'
--- cronscripts/publishing/maintenance-check.py 2010-05-28 16:17:51 +0000
+++ cronscripts/publishing/maintenance-check.py 2010-10-21 19:06:53 +0000

+# Names of the distribution releases that are not supported by this
+# tool. All later versions are supported.
+UNSUPPORTED_DISTRO_RELEASED = ["dapper", "edgy", "feisty", "gutsy", "hardy",
+ "intrepid", "jaunty", "karmic"]

A small thing: our coding standards say this should be formatted as

UNSUPPORTED_DISTRO_RELEASED = [
    "dapper",
    "edgy",
    "feisty",
    "gutsy",
    "hardy",
    "intrepid",
    "jaunty",
    "karmic",
    ]

(Notice the trailing comma even on the last line.)

@@ -275,8 +282,8 @@

- if distro[0] < 'h':
- print "ERROR: only hardy or later is supported"
+ if distro in UNSUPPORTED_DISTRO_RELEASED:
+ logging.error("only lucid or later is supported")

Much nicer, yes!

=== added directory 'cronscripts/publishing/tests'
=== added directory 'cronscripts/publishing/tests/mock-data'
=== added directory 'cronscripts/publishing/tests/mock-data/mock-bin'
=== added file 'cronscripts/publishing/tests/mock-data/mock-bin/germinate'
--- cronscripts/publishing/tests/mock-data/mock-bin/germinate 1970-01-01 00:00:00 +0000
+++ cronscripts/publishing/tests/mock-data/mock-bin/germinate 2010-10-21 19:06:53 +0000
@@ -0,0 +1,5 @@
+#!/bin/sh
+#
+# This is a mock germinate script that just produces enough (empty)
+# files so that the cron.germinate shell script can run.
+touch structure all all.sources minimal standard

Perfect. This tells me exactly what I'm looki...

review: Approve

[Jeroen T. Vermeulen (jtv)]

(Setting review type to "code," because the ec2 landing script requires an Approved vote of that type.)

review: Approve (code)

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9903. By Michael Vogt on 2010-11-04

cronscripts/publishing/tests/mock-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py: fix pocketlint errors (indent, spacing)

9904. By Michael Vogt on 2010-11-04

cronscripts/publishing/maintenance-check.py: fix pocketlint errors (spacing, indent, linewrap)

9905. By Michael Vogt on 2010-11-04

cronscripts/publishing/tests/test_cron.germinate.py: fix pocketlint errors (spacing, indent, linewrap)

9906. By Michael Vogt on 2010-11-04

cronscripts/publishing/tests/test_cron.germinate.py: updated to follow LP coding standards

9907. By Michael Vogt on 2010-11-04

cronscripts/publishing/tests/test_cron.germinate.py, cronscripts/publishing/maintenance-check.py: add SPACE around "=" to follow LP coding standard

9908. By Michael Vogt on 2010-11-04

cronscripts/publishing/tests/test_cron.germinate.py: adress formating issues and assert messages

[Michael Vogt (mvo)]

On Wed, Oct 27, 2010 at 10:46:02AM -0000, Jeroen T. Vermeulen wrote:

> Thanks for all the work you did on this! Is it just me or does the
> diff look a lot shorter now, despite having the new bits code? It
> certainly seems easier to read.

Thanks for the detailed review and sorry for the slow reply, the UDS
was taking all my attention. Its indeed much nicer now.

[..]
> === modified file 'cronscripts/publishing/maintenance-check.py'
> --- cronscripts/publishing/maintenance-check.py 2010-05-28 16:17:51 +0000
> +++ cronscripts/publishing/maintenance-check.py 2010-10-21 19:06:53 +0000
>
> +# Names of the distribution releases that are not supported by this
> +# tool. All later versions are supported.
> +UNSUPPORTED_DISTRO_RELEASED = ["dapper", "edgy", "feisty", "gutsy", "hardy",
> + "intrepid", "jaunty", "karmic"]
>
> A small thing: our coding standards say this should be formatted as
>
> UNSUPPORTED_DISTRO_RELEASED = [
> "dapper",
> "edgy",
> "feisty",
> "gutsy",
> "hardy",
> "intrepid",
> "jaunty",
> "karmic",
> ]

> (Notice the trailing comma even on the last line.)

Thanks, updated in my branch.

[..]
> --- cronscripts/publishing/tests/mock-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py 1970-01-01 00:00:00 +0000
> +++ cronscripts/publishing/tests/mock-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py 2010-10-21 19:06:53 +0000
> @@ -0,0 +1,24 @@
> +#!/usr/bin/python
> +#
> +# This is a mock version of the lp-query-distro.py script that
> +# returns static data so that the cron.germinate shell script
> +# can be tested.
> +
> +import sys
> +
> +def error_and_exit():
>
> Another small thing from our coding standards: each top-level (i.e. unindented) class or function definition in a file should have two blank lines above it.
>
> Running "make lint" should tell you about these things. With such old code though, it's possible that your changes drown among warnings for code you didn't even touch.
[..]

Thanks, I fixed that in my branch now as well. I can not run make lint
apparently without setting up a proper LP dev environment. It
complains about missing external dependencies, if I run the suggested
script it will fails with a non-obvious error. But checking lint.sh.in
shows that pocketlint is used and that makes it trivial to run it.

I went over the pocketlint errors and fixed all of them (except for
one in line 88 that I'm not sure what the correct style to use is).

[..]
> +# mvo: I would love to use this, but it complains about a missing
> +# import for "fixtures" and I can not find a package that
> +# provides this module
> +#from lp.testing import TestCase # or TestCaseWithFactory
>
> The disadvantage of having a very active hands-on architect is that things change so fast. :-) When in doubt, I generally go through these steps:
[..]

Thanks for these tips. For this particular piece of code I do not need
a full launchpad environment so I haven't setup one. But I will keep
this in my tomboy notes for when I need it :)

> + self.ubuntu_misc_dir = os.path.join(self.archive_dir, "ubuntu-misc")
> + self.ubuntu_ger...

[Jeroen T. Vermeulen (jtv)]

Hi Michael,

This sure isn't an easy review. My compliments on your tenacity—it'll be really good to have this landed. The code as such is good as far as I'm concerned.

To be honest, developing a branch without a full LP environment set up is probably not a good idea. It messes with your system setup and drops any postgres clusters you may have, but on the bright side, it is fully automated (including easy updates and faster bzr interaction with future branches) and gives you access to "make lint" and the test runner. It may be worth setting up a virtual machine for.

I set up a copy of your branch locally. It turns out that on the one hand, the test suite does run your script test. On the other, it's not run in the right way somehow and it breaks. As it turns out, moving the test and the mock-data directory into lib/lp/soyuz/scripts/tests/ fixes that. You can see my changes at lp:~jtv/launchpad/mvo

Unfortunately it's still not quite working. The cron.germinate script seems to run a command "lockfile" that it can't find. If this is from a package that needs to be installed on the system, it should be included in either the dependencies for Launchpad or the zc.buildout packages. Otherwise the test suite will not pass on the EC2 and buildbot machines, which start out with clean base systems.

Jeroen

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9909. By Michael Vogt on 2010-11-11

merged with db-devel

9910. By Michael Vogt on 2010-11-11

merged from jtv (many thanks)

9911. By Michael Vogt on 2010-11-11

lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/lockfile: add mock lockfile command to ensure cron.germinate finds this command when it needs it

9912. By Michael Vogt on 2010-11-11

lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/lockfile: return 1 instead of 0 (this is what cron.germinate expects)

[Michael Vogt (mvo)]

Thanks for your branch. I merged it into my branch and I have a LP setup now (that took a while to setup). I created a mock lockfile command, however the test is still failing in my lucid VM. Probably some problem with the path shuffling. I look into this tomorrow.

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9913. By Michael Vogt on 2010-11-11

lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/cronscripts/publishing/maintenance-check.py: fix symlink target now that the tests moved to a different directory

[Michael Vogt (mvo)]

The branch should be ready now, the tests pass in a LP dev environment for me now.

review: Needs Resubmitting

[Jeroen T. Vermeulen (jtv)]

Argh! The test gets stuck when I run it:

Running germinate... ........................ done.
Running maintenance-check for hardy... done
Running maintenance-check for jaunty... done
Running maintenance-check for karmic... done
Running maintenance-check for lucid...

At that point maintenance-check.py sits effectively idle forever.

What I suspect happens is that the script tries to access the URLs that it has hard-coded into it:

{{{
# germinate output base directory
BASE_URL = "http://people.canonical.com/~ubuntu-archive/germinate-output/"

# hints dir url, hints file is "$distro.hints" by default
# (e.g. lucid.hints)
HINTS_DIR_URL = "http://people.canonical.com/~ubuntu-archive/seeds/platform.%s/SUPPORTED_HINTS"

# we need the archive root to parse the Sources file to support
# by-source hints
ARCHIVE_ROOT = "http://archive.ubuntu.com/ubuntu"
}}}

The test suite should not make any network connections outside of the test system. I'm afraid that to get this working, we may have to inject test doubles for these URLs as well.

(Also, on a very minor note, the current branch still adds cronscripts/publishing/tests although of course it's now empty.)

[Michael Vogt (mvo)]

Thanks Jeroen for running this inside the test evnironment. I look into adding test doubles next, that is going to be a bit of work. And I removed the empty dir now :)

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9914. By Michael Vogt on 2010-11-25

cronscripts/publishing/tests: removed, empty now

9915. By Michael Vogt on 2010-11-25

cronscripts/publishing/maintenance-check.py: support url overrides in the environment for {MAINTENANCE_CHECK_BASE_URL, MAINTENANCE_CHECK_HINTS_DIR_URL, MAINTENANCE_CHECK_ARCHIVE_ROOT}

[Gavin Panella (allenap)]

Michael, this is a long-lived branch into which you and the reviewers have put a lot of work. Is there something I can do to help you get this landed?

[Michael Vogt (mvo)]

Thanks Gavin! I did look into this one again yesterday. I need to create the test dummies, that is tedious work. But the bugfix is important, so I will give it another go.

[lp:~mvo/launchpad/support-timeframe-fix-660433]

9916. By Michael Vogt on 2011-01-07

lib/lp/soyuz/scripts/tests/test_cron_germinate.py: create mock environment for get-maintenance-check.py because it can not use the network to fetch the real data when the testsuite runs

[Michael Vogt (mvo)]

This update should create all the needed files locally now without the need to touch the network. Please note that I added a "class PoorMvosTestCase(unittest.TestCase)" that is used as a fallback if the full lp.testing import TestCase can not be imported. Feel free to remove it in the final merge (or I can remove it too once the rest looks ok). I added it because I only have a full LP environment in a VM and that is a bit of a hassle to work with. But this script really only needs the makeTemporaryFile feature. The test passes now for me without network and the data looks sane too.

[Gavin Panella (allenap)]

Michael, I've merged devel into your branch, cleaned up your last revision and put it in lp:~allenap/launchpad/support-timeframe-fix-660433. Please can you merge my branch into yours and push back to Launchpad?

[Gavin Panella (allenap)]

Don't worry, I'll land this directly from my branch.

[Gavin Panella (allenap)]

I've merged this into *devel* via lp:~allenap/launchpad/support-timeframe-fix-660433.

[Michael Vogt (mvo)]

On Thu, Jan 20, 2011 at 07:01:55PM -0000, Gavin Panella wrote:
> Michael, I've merged devel into your branch, cleaned up your last revision and put it in lp:~allenap/launchpad/support-timeframe-fix-660433. Please can you merge my branch into yours and push back to Launchpad?

Thanks a lot for this! And sorry for my late reply, I was away for a
cross distro meeting a couple of days and just returned. Merged and
pushed.

I just read that you will land it from your branch, thats great. I
hope you did not had too much trouble with merge conflicts and all?

Cheers,
 Michael

[Gavin Panella (allenap)]

Michael, it landed without a hitch, perfect. Thank you for all the work you've done on this :)

=== modified file 'cronscripts/publishing/cron.germinate'
--- cronscripts/publishing/cron.germinate	2010-11-02 15:54:52 +0000
+++ cronscripts/publishing/cron.germinate	2011-01-07 13:50:46 +0000
@@ -6,12 +6,12 @@
 set -e
 set -u
 
-ARCHIVEROOT=/srv/launchpad.net/ubuntu-archive/ubuntu
+ARCHIVEROOT=${TEST_ARCHIVEROOT:-/srv/launchpad.net/ubuntu-archive/ubuntu}
 MISCROOT=$ARCHIVEROOT/../ubuntu-misc
 LOCKROOT=$ARCHIVEROOT/..
 GERMINATEROOT=$ARCHIVEROOT/../ubuntu-germinate
 
-LAUNCHPADROOT=/srv/launchpad.net/codelines/current
+LAUNCHPADROOT=${TEST_LAUNCHPADROOT:-/srv/launchpad.net/codelines/current}
 MAINTAINCE_CHECK=$LAUNCHPADROOT/cronscripts/publishing/maintenance-check.py
 
 ## Check to see if another germinate run is in progress
@@ -28,7 +28,6 @@
 
 trap cleanup EXIT
 
-LAUNCHPADROOT=/srv/launchpad.net/codelines/current
 suite=`$LAUNCHPADROOT/scripts/ftpmaster-tools/lp-query-distro.py development`
 
 echo -n "Running germinate... "
@@ -131,10 +130,27 @@
 done
 echo " done."
 
-# now generate the Supported extra overrides
-$MAINTAINCE_CHECK $suite > "$MISCROOT/more-extra.override.$suite.main.supported" 2> _maintenance-check.stderr
-if [ $? -eq 0 ]; then
-    cat "$MISCROOT/more-extra.override.$suite.main.supported" >> "$MISCROOT/more-extra.override.$suite.main.new"
-fi
-
 mv -f "$MISCROOT/more-extra.override.$suite.main.new" "$MISCROOT/more-extra.override.$suite.main"
+
+# Now generate the Supported extra overrides for all supported distros.
+SUITES=`$LAUNCHPADROOT/scripts/ftpmaster-tools/lp-query-distro.py supported`
+for supported_suite in $SUITES; do
+    echo -n "Running maintenance-check for $supported_suite... "
+    # The support timeframe information is stored here
+    SUPPORTED="$MISCROOT/more-extra.override.$supported_suite.main.supported"
+    # This is the target override file that contains germinate plus support info
+    TARGET="$MISCROOT/more-extra.override.$supported_suite.main"
+    # Debug/Log information 
+    LOG="_maintenance-check.$supported_suite.stderr"
+    if $MAINTAINCE_CHECK $supported_suite > $SUPPORTED 2> $LOG; then
+        # The target file may be missing on the server and the script should
+        # not fail if that is the case.
+        touch $TARGET
+        # Remove old "Supported" info from extra-overrides as it may be 
+        # stale now and we replace it with fresh content below.
+        sed /"^.* Supported"/d $TARGET > ${TARGET}.new
+        cat $SUPPORTED >> ${TARGET}.new
+        mv ${TARGET}.new $TARGET
+    fi
+    echo " done"
+done
 
=== modified file 'cronscripts/publishing/maintenance-check.py'
--- cronscripts/publishing/maintenance-check.py	2010-05-28 16:17:51 +0000
+++ cronscripts/publishing/maintenance-check.py	2011-01-07 13:50:46 +0000
@@ -1,7 +1,7 @@
 #!/usr/bin/python
 #
 # python port of the nice maintainace-check script by  Nick Barcet
-# 
+#
 # taken from:
 #  https://code.edge.launchpad.net/~mvo/ubuntu-maintenance-check/python-port
 # (where it will vanish once taken here)
@@ -9,7 +9,7 @@
 # this warning filter is only needed on older versions of python-apt,
 # once the machine runs lucid it can be removed
 import warnings
-warnings.filterwarnings("ignore","apt API not stable yet")
+warnings.filterwarnings("ignore", "apt API not stable yet")
 import apt
 warnings.resetwarnings()
 
@@ -29,21 +29,21 @@
 #  - distros "kubuntu", "edubuntu" and "netbook" need to be
 #    considered *but* only follow SUPPORT_TIMEFRAME
 #  - anything that is in armel follows SUPPORT_TIMEFRAME
-#  
+#
 
 # codename of the lts releases
-LTS_RELEASES = [ "dapper", "hardy", "lucid" ]
+LTS_RELEASES = ["dapper", "hardy", "lucid"]
 
 # architectures that are full supported (including LTS time)
-PRIMARY_ARCHES =  ["i386", "amd64"]
+PRIMARY_ARCHES = ["i386", "amd64"]
 
 # architectures we support (but not for LTS time)
 SUPPORTED_ARCHES = PRIMARY_ARCHES + ["armel"]
 
 # what defines the seeds is documented in wiki.ubuntu.com/SeedManagement
-SERVER_SEEDS = [ "supported-server", "server-ship"]
+SERVER_SEEDS = ["supported-server", "server-ship"]
 DESKTOP_SEEDS = ["ship", "supported-desktop", "supported-desktop-extra"]
-SUPPORTED_SEEDS = [ "all" ]
+SUPPORTED_SEEDS = ["all"]
 
 # normal support timeframe
 # time, seeds, arches
@@ -60,25 +60,46 @@
 ]
 
 # distro names and if they get LTS support (order is important)
-DISTRO_NAMES_AND_LTS_SUPPORT = [ ("ubuntu",   True),
-                                 ("kubuntu",  True),
-                                 ("netbook",  False),
-                               ]
+DISTRO_NAMES_AND_LTS_SUPPORT = [
+    ("ubuntu", True),
+    ("kubuntu", True),
+    ("netbook", False),
+    ]
+
+# Names of the distribution releases that are not supported by this
+# tool. All later versions are supported.
+UNSUPPORTED_DISTRO_RELEASED = [
+    "dapper",
+    "edgy",
+    "feisty",
+    "gutsy",
+    "hardy",
+    "intrepid",
+    "jaunty",
+    "karmic",
+    ]
+
 
 # germinate output base directory
-BASE_URL = "http://people.canonical.com/~ubuntu-archive/germinate-output/"
+BASE_URL = os.environ.get(
+    "MAINTENANCE_CHECK_BASE_URL", 
+    "http://people.canonical.com/~ubuntu-archive/germinate-output/")
 
 # hints dir url, hints file is "$distro.hints" by default
 # (e.g. lucid.hints)
-HINTS_DIR_URL = "http://people.canonical.com/~ubuntu-archive/seeds/platform.%s/SUPPORTED_HINTS"
+HINTS_DIR_URL = os.environ.get(
+    "MAINTENANCE_CHECK_HINTS_DIR_URL", 
+    "http://people.canonical.com/~ubuntu-archive/seeds/platform.%s/SUPPORTED_HINTS")
 
 # we need the archive root to parse the Sources file to support
 # by-source hints
-ARCHIVE_ROOT = "http://archive.ubuntu.com/ubuntu"
+ARCHIVE_ROOT = os.environ.get(
+    "MAINTENANCE_CHECK_ARCHIVE_ROOT", "http://archive.ubuntu.com/ubuntu")
 
 # support timeframe tag used in the Packages file
 SUPPORT_TAG = "Supported"
 
+
 def get_binaries_for_source_pkg(srcname):
     """ Return all binary package names for the given source package name.
 
@@ -92,38 +113,41 @@
             pkgnames.add(binary)
     return pkgnames
 
+
 def expand_src_pkgname(pkgname):
     """ Expand a package name if it is prefixed with src.
 
     If the package name is prefixed with src it will be expanded
     to a list of binary package names. Otherwise the original
     package name will be returned.
-    
+
     :param pkgname: The package name (that may include src:prefix).
-    :return: A list of binary package names (the list may be one element long).
+    :return: A list of binary package names (the list may be one element
+             long).
     """
     if not pkgname.startswith("src:"):
         return [pkgname]
     return get_binaries_for_source_pkg(pkgname.split("src:")[1])
 
+
 def create_and_update_deb_src_source_list(distroseries):
     """ Create sources.list and update cache.
 
-    This creates a sources.list file with deb-src entries for a given 
+    This creates a sources.list file with deb-src entries for a given
     distroseries and apt.Cache.update() to make sure the data is up-to-date.
     :param distro: The code name of the distribution series (e.g. lucid).
     :return: None
     :raises: IOError: When cache update fails.
     """
     # apt root dir
-    rootdir="./aptroot.%s" % distroseries
-    sources_list_dir = os.path.join(rootdir, "etc","apt")
+    rootdir = "./aptroot.%s" % distroseries
+    sources_list_dir = os.path.join(rootdir, "etc", "apt")
     if not os.path.exists(sources_list_dir):
         os.makedirs(sources_list_dir)
-    sources_list = open(os.path.join(sources_list_dir, "sources.list"),"w")
+    sources_list = open(os.path.join(sources_list_dir, "sources.list"), "w")
     for pocket in [
-        "%s" % distroseries, 
-        "%s-updates" % distroseries, 
+        "%s" % distroseries,
+        "%s-updates" % distroseries,
         "%s-security" % distroseries]:
         sources_list.write(
             "deb-src %s %s main restricted\n" % (
@@ -134,13 +158,13 @@
     sources_list.close()
     # create required dirs/files for apt.Cache(rootdir) to work on older
     # versions of python-apt. once lucid is used it can be removed
-    for d in  ["var/lib/dpkg", 
-               "var/cache/apt/archives/partial",
-               "var/lib/apt/lists/partial"]:
-        if not os.path.exists(os.path.join(rootdir,d)):
-            os.makedirs(os.path.join(rootdir,d))
-    if not os.path.exists(os.path.join(rootdir,"var/lib/dpkg/status")):
-        open(os.path.join(rootdir,"var/lib/dpkg/status"),"w")
+    for d in ["var/lib/dpkg",
+              "var/cache/apt/archives/partial",
+              "var/lib/apt/lists/partial"]:
+        if not os.path.exists(os.path.join(rootdir, d)):
+            os.makedirs(os.path.join(rootdir, d))
+    if not os.path.exists(os.path.join(rootdir, "var/lib/dpkg/status")):
+        open(os.path.join(rootdir, "var/lib/dpkg/status"), "w")
     # open cache with our just prepared rootdir
     cache = apt.Cache(rootdir=rootdir)
     try:
@@ -148,24 +172,28 @@
     except SystemError:
         logging.exception("cache.update() failed")
 
+
 def get_structure(distroname, version):
     """ Get structure file conent for named distro and distro version.
-    
+
     :param name: Name of the distribution (e.g. kubuntu, ubuntu, xubuntu).
     :param version: Code name of the distribution version (e.g. lucid).
     :return: List of strings with the structure file content
     """
-    f = urllib2.urlopen("%s/%s.%s/structure" % (BASE_URL, distroname, version))
+    f = urllib2.urlopen("%s/%s.%s/structure" % (
+            BASE_URL, distroname, version))
     structure = f.readlines()
     f.close()
     return structure
 
+
 def expand_seeds(structure, seedname):
     """ Expand seed by its dependencies using the strucure file.
 
     :param structure: The content of the STRUCTURE file as string list.
     :param seedname: The name of the seed as string that needs to be expanded.
-    :return: a set() for the seed dependencies (excluding the original seedname)
+    :return: a set() for the seed dependencies (excluding the original
+             seedname)
     """
     seeds = []
     for line in structure:
@@ -175,9 +203,10 @@
                 seeds += expand_seeds(structure, seed)
     return set(seeds)
 
+
 def get_packages_for_seeds(name, distro, seeds):
     """
-    get packages for the given name (e.g. ubuntu) and distro release 
+    get packages for the given name (e.g. ubuntu) and distro release
     (e.g. lucid) that are in the given list of seeds
     returns a set() of package names
     """
@@ -185,7 +214,7 @@
     for bseed in seeds:
         for seed in [bseed]: #, bseed+".build-depends", bseed+".seed"]:
             pkgs_in_seeds[seed] = set()
-            seedurl = "%s/%s.%s/%s" % (BASE_URL,name, distro, seed)
+            seedurl = "%s/%s.%s/%s" % (BASE_URL, name, distro, seed)
             logging.debug("looking for '%s'" % seedurl)
             try:
                 f = urllib2.urlopen(seedurl)
@@ -204,6 +233,7 @@
                 logging.error("seed %s failed (%s)" % (seedurl, e))
     return pkgs_in_seeds
 
+
 def what_seeds(pkgname, seeds):
     in_seeds = set()
     for s in seeds:
@@ -211,6 +241,7 @@
             in_seeds.add(s)
     return in_seeds
 
+
 def compare_support_level(x, y):
     """
     compare two support level strings of the form 18m, 3y etc
@@ -218,9 +249,10 @@
     :parm y: the second support level
     :return: negative if x < y, zero if x==y, positive if x > y
     """
+
     def support_to_int(support_time):
         """
-        helper that takes a support time string and converts it to 
+        helper that takes a support time string and converts it to
         a integer for cmp()
         """
         # allow strings like "5y (kubuntu-common)
@@ -233,7 +265,9 @@
             raise ValueError("support time '%s' has to end with y or m" % x)
     return cmp(support_to_int(x), support_to_int(y))
 
-def get_packages_support_time(structure, name, pkg_support_time, support_timeframe_list):
+
+def get_packages_support_time(structure, name, pkg_support_time,
+                              support_timeframe_list):
     """
     input a structure file and a list of pair<timeframe, seedlist>
     return a dict of pkgnames -> support timeframe string
@@ -255,18 +289,21 @@
                                 pkg, old_timeframe, timeframe))
                         pkg_support_time[pkg] = timeframe
                 if options.with_seeds:
-                    pkg_support_time[pkg] += " (%s)" % ", ".join(what_seeds(pkg, pkgs_in_seeds))
+                    pkg_support_time[pkg] += " (%s)" % ", ".join(
+                        what_seeds(pkg, pkgs_in_seeds))
 
 
     return pkg_support_time
 
+
 if __name__ == "__main__":
     parser = OptionParser()
     parser.add_option("--with-seeds", "", default=False,
-                      action="store_true", 
-                      help="add seed(s) of the package that are responsible for the maintaince time")
+                      action="store_true",
+                      help="add seed(s) of the package that are responsible "
+                           "for the maintaince time")
     parser.add_option("--source-packages", "", default=False,
-                      action="store_true", 
+                      action="store_true",
                       help="show as source pkgs")
     parser.add_option("--hints-file", "", default=None,
                       help="use diffenrt use hints file location")
@@ -275,8 +312,8 @@
     # init
     if len(args) > 0:
         distro = args[0]
-        if distro[0] < 'h':
-            print "ERROR: only hardy or later is supported"
+        if distro in UNSUPPORTED_DISTRO_RELEASED:
+            logging.error("only lucid or later is supported")
             sys.exit(1)
     else:
         distro = "lucid"
@@ -286,26 +323,32 @@
 
     if options.hints_file:
         hints_file = options.hints_file
-        (schema, netloc, path, query, fragment) = urlparse.urlsplit(hints_file)
+        (schema, netloc, path, query, fragment) = urlparse.urlsplit(
+            hints_file)
         if not schema:
             hints_file = "file:%s" % path
     else:
         hints_file = HINTS_DIR_URL % distro
-        
+
     # go over the distros we need to check
     pkg_support_time = {}
     for (name, lts_supported) in DISTRO_NAMES_AND_LTS_SUPPORT:
 
         # get basic structure file
-        structure = get_structure(name, distro)
-    
+        try:
+            structure = get_structure(name, distro)
+        except urllib2.HTTPError:
+            logging.error("Can not get structure for '%s'." % name)
+            continue
+
         # get dicts of pkgname -> support timeframe string
         support_timeframe = SUPPORT_TIMEFRAME
         if lts_supported and distro in LTS_RELEASES:
-            support_timeframe =  SUPPORT_TIMEFRAME_LTS
+            support_timeframe = SUPPORT_TIMEFRAME_LTS
         else:
             support_timeframe = SUPPORT_TIMEFRAME
-        get_packages_support_time(structure, name, pkg_support_time, support_timeframe)
+        get_packages_support_time(
+            structure, name, pkg_support_time, support_timeframe)
 
     # now go over the bits in main that we have not seen (because
     # they are not in any seed and got added manually into "main"
@@ -321,10 +364,11 @@
         for pkg in cache:
             if not pkg.name in pkg_support_time:
                 pkg_support_time[pkg.name] = support_timeframe[-1][0]
-                logging.warn("add package in main but not in seeds %s with %s" % 
-                             (pkg.name, pkg_support_time[pkg.name]))
+                logging.warn(
+                    "add package in main but not in seeds %s with %s" % (
+                        pkg.name, pkg_support_time[pkg.name]))
 
-    # now check the hints file that is used to overwrite 
+    # now check the hints file that is used to overwrite
     # the default seeds
     try:
         for line in urllib2.urlopen(hints_file):
@@ -337,14 +381,15 @@
                     if support_time == 'unsupported':
                         try:
                             del pkg_support_time[pkgname]
-                            sys.stderr.write("hints-file: marking %s unsupported\n" % pkgname)
+                            sys.stderr.write("hints-file: marking %s "
+                                             "unsupported\n" % pkgname)
                         except KeyError:
                             pass
                     else:
                         if pkg_support_time.get(pkgname) != support_time:
                             sys.stderr.write(
                                 "hints-file: changing %s from %s to %s\n" % (
-                                    pkgname,  pkg_support_time.get(pkgname), 
+                                    pkgname, pkg_support_time.get(pkgname),
                                     support_time))
                             pkg_support_time[pkgname] = support_time
             except:
@@ -353,7 +398,7 @@
         if e.code != 404:
             raise
         sys.stderr.write("hints-file: %s gave 404 error\n" % hints_file)
-    
+
     # output suitable for the extra-override file
     for pkgname in sorted(pkg_support_time.keys()):
         # special case, the hints file may contain overrides that
@@ -362,7 +407,7 @@
             print "%s %s %s" % (
                 pkgname, SUPPORT_TAG, pkg_support_time[pkgname])
         else:
-            # go over the supported arches, they are divided in 
+            # go over the supported arches, they are divided in
             # first-class (PRIMARY) and second-class with different
             # support levels
             for arch in SUPPORTED_ARCHES:
@@ -373,10 +418,11 @@
                 if arch in PRIMARY_ARCHES:
                     # arch with full LTS support
                     print "%s %s %s" % (
-                        pkgname_and_arch, SUPPORT_TAG, pkg_support_time[pkgname])
+                        pkgname_and_arch, SUPPORT_TAG,
+                        pkg_support_time[pkgname])
                 else:
                     # not a LTS supported architecture, gets only regular
                     # support_timeframe
                     print "%s %s %s" % (
-                        pkgname_and_arch, SUPPORT_TAG, SUPPORT_TIMEFRAME[0][0])
-                
+                        pkgname_and_arch, SUPPORT_TAG,
+                        SUPPORT_TIMEFRAME[0][0])
 
=== added directory 'lib/lp/soyuz/scripts/tests/germinate-test-data'
=== added directory 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin'
=== added file 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/germinate'
--- lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/germinate	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/germinate	2011-01-07 13:50:46 +0000
@@ -0,0 +1,5 @@
+#!/bin/sh
+#
+# This is a mock germinate script that just produces enough (empty)
+# files so that the cron.germinate shell script can run.
+touch structure all all.sources minimal standard
 
=== added file 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/lockfile'
--- lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/lockfile	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/scripts/tests/germinate-test-data/mock-bin/lockfile	2011-01-07 13:50:46 +0000
@@ -0,0 +1,8 @@
+#!/bin/sh
+
+# This is a mock "lockfile" command that is used during the test
+# There is no need to have the real lockfile installed (its part
+# of procmail) because there can be no multiple germinate instances
+# while we run the tests
+
+exit 1
 
=== added directory 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root'
=== added directory 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/cronscripts'
=== added directory 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/cronscripts/publishing'
=== added symlink 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/cronscripts/publishing/maintenance-check.py'
=== target is u'../../../../../../../../../cronscripts/publishing/maintenance-check.py'
=== added directory 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/scripts'
=== added directory 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/scripts/ftpmaster-tools'
=== added file 'lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py'
--- lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/scripts/tests/germinate-test-data/mock-lp-root/scripts/ftpmaster-tools/lp-query-distro.py	2011-01-07 13:50:46 +0000
@@ -0,0 +1,27 @@
+#!/usr/bin/python
+#
+# This is a mock version of the lp-query-distro.py script that
+# returns static data so that the cron.germinate shell script
+# can be tested.
+
+import sys
+
+
+def error_and_exit():
+        sys.stderr.write("ERROR: I'm a mock, I only support 'development' "
+                         "and 'supported' as argument\n")
+        sys.exit(1)
+
+
+if __name__ == "__main__":
+        # There is only a very limited subset of arguments that we support,
+        # test for it and error if it looks wrong
+        if len(sys.argv) != 2:
+                error_and_exit()
+        distro = sys.argv[1]
+        if distro == "development":
+                print "natty"
+        elif distro == "supported":
+                print "hardy jaunty karmic lucid maverick"
+        else:
+                error_and_exit()
 
=== added file 'lib/lp/soyuz/scripts/tests/test_cron_germinate.py'
--- lib/lp/soyuz/scripts/tests/test_cron_germinate.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/scripts/tests/test_cron_germinate.py	2011-01-07 13:50:46 +0000
@@ -0,0 +1,230 @@
+#!/usr/bin/python
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""This is a test for the soyuz cron.germinate script."""
+
+__metaclass__ = type
+
+import copy
+import gzip
+import os
+import subprocess
+import unittest
+
+try:
+    from lp.testing import TestCase
+except ImportError:
+    # mvo: *cough* poor mans version if no full LP environment is installed
+    class PoorMvosTestCase(unittest.TestCase):
+        def makeTemporaryDirectory(self):
+            import tempfile
+            return tempfile.mkdtemp(prefix="test_cron_germinate_")
+    TestCase = PoorMvosTestCase
+
+class TestCronGerminate(TestCase):
+
+    DISTRO_NAMES = [ "platform", "ubuntu", "kubuntu", "netbook" ]
+    DISTS = ["hardy", "lucid", "maverick"]
+    DEVELOPMENT_DIST = "natty"
+    COMPONENTS = ["main", "restricted", "universe", "multiverse"]
+    ARCHES = ["i386", "amd64", "armel", "powerpc"]
+    BASEPATH = os.path.abspath(os.path.dirname(__file__))
+    source_root = os.path.normpath(
+        os.path.join(BASEPATH, "..", "..", "..", "..", ".."))
+
+    def setUp(self):
+        super(TestCronGerminate, self).setUp()
+
+        # Setup a temp archive directory and populate it with the right
+        # sub-directories.
+        self.tmpdir = self.makeTemporaryDirectory()
+        self.archive_dir = self.setup_mock_archive_environment()
+        self.ubuntu_misc_dir = os.path.join(self.archive_dir, "ubuntu-misc")
+        self.ubuntu_germinate_dir = os.path.join(
+            self.archive_dir, "ubuntu-germinate")
+        # create a mock archive environment for all the distros we
+        # support and also include "updates" and "security"
+        for dist in self.DISTS + [self.DEVELOPMENT_DIST]:
+            self.populate_mock_archive_environment(
+                self.archive_dir, self.COMPONENTS, self.ARCHES, dist)
+            for component in ["security", "updates"]:
+                self.populate_mock_archive_environment(
+                    self.archive_dir, self.COMPONENTS, self.ARCHES, 
+                    "%s-%s" % (dist, component))
+        # generate test dummies for maintenance-time.py, if this is
+        # set to "None" instead it will use the network to test against
+        # the real data
+        self.germinate_output_dir = self.setup_mock_germinate_output()
+
+    def create_directory_if_missing(self, directory):
+        """Create the given directory if it does not exist."""
+        if not os.path.exists(directory):
+            os.makedirs(directory)
+
+    def create_directory_list_if_missing(self, directory_list):
+        """Create the given directories from the list if they don't exist."""
+        for directory in directory_list:
+            self.create_directory_if_missing(directory)
+
+    def create_gzip_file(self, filepath, content=""):
+        """Create a gziped file in the given path with the given content.
+           If not content is given a empty file is created.
+        """
+        gz = gzip.GzipFile(filepath, "w")
+        gz.write(content)
+        gz.close()
+
+    def create_file(self, filepath, content=""):
+        """Create a file in the given path with the given content.
+           If not content is given a empty file is created.
+        """
+        f = open(filepath, "w")
+        f.write(content)
+        f.close()
+
+    def setup_mock_germinate_output(self):
+        # empty structure files
+        germinate_output_dir = os.path.join(
+            self.tmpdir, "germinate-test-data", "germinate-output")
+        dirs = []
+        for distro_name in self.DISTRO_NAMES:
+            for distro_series in self.DISTS:
+                dirs.append(os.path.join(germinate_output_dir, "%s.%s" % (distro_name, distro_series)))
+        self.create_directory_list_if_missing(dirs)
+        for dir in dirs:
+            self.create_file(os.path.join(dir, "structure"))
+        return germinate_output_dir
+
+    def setup_mock_archive_environment(self):
+        """Creates a mock archive environment and populate
+           it with the subdirectories that germinate will expect.
+        """
+        archive_dir = os.path.join(
+            self.tmpdir, "germinate-test-data", "ubuntu-archive")
+        ubuntu_misc_dir = os.path.join(archive_dir, "ubuntu-misc")
+        ubuntu_germinate_dir = os.path.join(archive_dir, "ubuntu-germinate")
+        ubuntu_dists_dir = os.path.join(archive_dir, "ubuntu", "dists")
+        self.create_directory_list_if_missing([
+                archive_dir,
+                ubuntu_misc_dir,
+                ubuntu_germinate_dir,
+                ubuntu_dists_dir])
+        return archive_dir
+
+    def populate_mock_archive_environment(self, archive_dir, components_list,
+                                          arches_list, current_devel_distro):
+        """Populates a mock archive environment with empty source packages
+           and empty binary packages.
+        """
+        for component in components_list:
+            # Create the environment for the source packages.
+            targetdir = os.path.join(
+                archive_dir,
+                "ubuntu/dists/%s/%s/source" % (
+                    current_devel_distro, component))
+            self.create_directory_if_missing(targetdir)
+            self.create_gzip_file(os.path.join(targetdir, "Sources.gz"))
+
+            # Create the environment for the binary packages.
+            for arch in arches_list:
+                for subpath in ["", "debian-installer"]:
+                    targetdir = os.path.join(
+                        self.archive_dir,
+                        "ubuntu/dists/%s/%s/%s/binary-%s" % (
+                            current_devel_distro, component, subpath, arch))
+                    self.create_directory_if_missing(targetdir)
+                    self.create_gzip_file(os.path.join(
+                            targetdir, "Packages.gz"))
+
+    def create_fake_environment(self, basepath, archive_dir, 
+                                germinate_output_dir):
+        """Create a fake process envirionment based on os.environ that
+           sets TEST_ARCHIVEROOT, TEST_LAUNCHPADROOT and modifies PATH
+           to point to the mock lp-bin directory.
+        """
+        fake_environ = copy.copy(os.environ)
+        fake_environ["TEST_ARCHIVEROOT"] = os.path.abspath(
+            os.path.join(archive_dir, "ubuntu"))
+        fake_environ["TEST_LAUNCHPADROOT"] = os.path.abspath(
+            os.path.join(basepath, "germinate-test-data/mock-lp-root"))
+        # Set the PATH in the fake environment so that our mock germinate
+        # is used. We could use the real germinate as well, but that will
+        # slow down the tests a lot and its also not interessting for this
+        # test as we do not use any of the germinate information.
+        fake_environ["PATH"] = "%s:%s" % (
+            os.path.abspath(os.path.join(
+                basepath, "germinate-test-data/mock-bin")),
+            os.environ["PATH"])
+        # test dummies for get-support-timeframe.py, they need to be
+        # in URI format
+        if germinate_output_dir:
+            # redirect base url to the mock environment
+            fake_environ["MAINTENANCE_CHECK_BASE_URL"] = "file://%s" % \
+                germinate_output_dir
+            # point to mock archive root
+            archive_root_url = "file://%s" % os.path.abspath(
+                os.path.join(archive_dir, "ubuntu"))
+            fake_environ["MAINTENANCE_CHECK_ARCHIVE_ROOT"] = archive_root_url
+            # maintenance-check.py expects a format string
+            hints_file_url = germinate_output_dir + "/platform.%s/SUPPORTED_HINTS"
+            for distro in self.DISTS:
+                open(hints_file_url % distro, "w")
+            fake_environ["MAINTENANCE_CHECK_HINTS_DIR_URL"] = "file://%s" % \
+                os.path.abspath(hints_file_url)
+            # add hints override to test that feature
+            f=open(hints_file_url % "lucid", "a")
+            f.write("linux-image-2.6.32-25-server 5y\n")
+            f.close()
+        return fake_environ
+
+    def test_maintenance_update(self):
+        """Test the maintenance-check.py porition of the soyuz cron.germinate
+           shell script by running it inside a fake environment and ensure
+           that it did update the "Support" override information for
+           apt-ftparchive without destroying/modifying the information
+           that the "germinate" script added to it earlier.
+        """
+        # Write into more-extras.overrides to ensure it is alive after we
+        # mucked around.
+        canary = "abrowser Task mock\n"
+        # Build fake environment based on the real one.
+        fake_environ = self.create_fake_environment(
+            self.BASEPATH, self.archive_dir, self.germinate_output_dir)
+        # Create mock override data files that include the canary string
+        # so that we can test later if it is still there.
+        for dist in self.DISTS:
+            self.create_file(
+                os.path.join(self.ubuntu_misc_dir,
+                             "more-extra.override.%s.main" % dist),
+                canary)
+
+        # Run cron.germinate in the fake environment.
+        cron_germinate_path = os.path.join(
+            self.source_root, "cronscripts", "publishing", "cron.germinate")
+        subprocess.call(
+            [cron_germinate_path], env=fake_environ, cwd=self.BASEPATH)
+
+        # And check the output it generated for correctness.
+        for dist in self.DISTS:
+            supported_override_file = os.path.join(
+                self.ubuntu_misc_dir,
+                "more-extra.override.%s.main.supported" % dist)
+            self.assertTrue(os.path.exists(supported_override_file),
+                            "no override file created for '%s'" % dist)
+            main_override_file = os.path.join(
+                self.ubuntu_misc_dir,
+                "more-extra.override.%s.main" % dist)
+            self.assertIn(canary, open(main_override_file).read(),
+                          "canary no longer there for '%s'" % dist)
+
+        # Check here if we got the data from maintenance-check.py that
+        # we expected. This is a kernel name from lucid-updates and it
+        # will be valid for 5 years.
+        needle = "linux-image-2.6.32-25-server/i386 Supported 5y"
+        lucid_supported_override_file = os.path.join(
+            self.ubuntu_misc_dir, "more-extra.override.lucid.main")
+        self.assertIn(needle, open(lucid_supported_override_file).read())
+
+if __name__ == "__main__":
+    unittest.main()