Code review comment for lp://staging/~gary/launchpad/honor_revisions

Revision history for this message
Gary Poster (gary) wrote :

Hi.

This branch changes the sourcecode update script to honor revision numbers. It also adds revision numbers to sourcedeps.conf. This change is important to allow us to move production-devel to buildbot.

An early plan for this branch was to remove our sourcecode update script entirely in favor of config-manager. However, our current script already handles our "optional" special cases of canonical-identity-provider and shipit. It also has more informative output than config-manager. Finally, in my tests, it was twice as fast locally for an update of launchpad's dependencies than config-manager was (one minute for the current script, and two minutes for config-manager). I'm not sure why config-manager was slower; there are probably good reasons for why. However, our update script is specific to our needs and does noticeably better with them, so I stuck with it.

Updating the script to honor revision numbers was pretty easy, especially with the config-manager example to work from for bzrlib guidance.

I chose to use the same syntax for specifying revisions that config-manager used, because config-manager represented a precedent that actually matters to our team (it is used for deployment), and I didn't have a strong preference for another syntax.

I chose to switch to using whitespace for delimiting the values in the configuration file (rather than "=") because it was very easy, especially to accommodate the config-manager revision syntax (which uses a "=").

I kept to about the same level of automated tests as we had before: tests for the parser, but not for the meat of the code, using bzrlib. I would feel better about stubbing out bzrlib using something like Gustavo's Mocker, but what I have is expedient and has precedent.

To test, first you can run ``./bin/test -t test_sourcecode``. Then (for test or QA), run ``./utilities/update-sourcecode``. If you have updated your sourcecode lately, you should see a number of updates that report "No change". To verify that I specified all of the most recent revisions of these branches, run ``./utilities/update-sourcecode --tip``. This should again report "No change" for each branch. To verify that it does what you expect in other contexts, you can remove a sourcecode branch and rerun ``./utilities/update-sourcecode``, to see it build a branch; and you can modify revision numbers in ``./utiltities/sourcedeps.conf`` to see if the changes you expect happen (reverting and updating).

I'm asking Tom Haddon to review this because I'm going to suggest that deployment use this script to get the sourcecode branches. Then, for production, the change will be to insert a new step 2 into the following rough process: (1) run config-manager to check out the correct launchpad branch and the download-cache, then (2) run this script with no arguments to get the sourcecode branches, and then (3) run make as usual.

This is instead of what we had discussed before, which was to insert steps 2 and 3 in this version of the process: (1) run config-manager to check out the correct launchpad branch and the download-cache, then (2) run some launchpad script that generates another config-manager file, then (3) rerun config-manager with the new generated file, and then (4) run make as usual. What I have now seemed cleaner. That said, if you would prefer the version we had originally discussed, with running config-manager twice, it will be easy to code. Just let me know. I might put it in another branch though.

Thank you!

Gary

« Back to merge proposal