Patchset for fixing 2604 and 2524, also contains another patch for CG

Patchset for fixing 2604 and 2524, also contains another patch for CG

CG: improve Patchy documentation [Patch set 4]

CG: Add LILYPOND_BUILD_DIR environment variable, fix issues 2604 and 2524. [Patch set 2]

[Graham Percival, 2012-06-27 10:11:33]

LGTM.  One tweak, then it can be pushed directly to staging.

http://codereview.appspot.com/6354044/diff/1/Documentation/contributor/admini...
File Documentation/contributor/administration.itexi (right):

http://codereview.appspot.com/6354044/diff/1/Documentation/contributor/admini...
Documentation/contributor/administration.itexi:271: 02 0-23/2 * * * /bin/bash
-lc /home/patchy/git/lilypond-extra/patches/lilypond-patchy-staging.py
For the website on lilypond.org, I have this in my crontab:


# website-rebuild.cron
LILYPOND_GIT=/home/graham/lilypond/lilypond-git/
LILYPOND_WEB_MEDIA_GIT=/home/graham/lilypond/lilypond-extra/
PATH=/home/graham/usr/bin/:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/bin


11 * * * * $HOME/lilypond/bin/update-git.sh > /dev/null 2>&1
37 * * * * $HOME/lilypond/bin/make-website.sh > /dev/null 2>&1



Do we need to do a similar thing for this crontab?  Cron has a really annoying
habit of not loading your default environment.  I can't remember if Patchy
relies on those environment vars for anything, but it might be safer to add a
warning about this.
(or dump a @ref{Uploading and security} to see the type of thing I mean; lines
211 to 224 in website-work.itexi )

[John Mandereau, 2012-06-27 13:53:51]

I'm on submitting another patchset (later tonight).

http://codereview.appspot.com/6354044/diff/1/Documentation/contributor/admini...
File Documentation/contributor/administration.itexi (right):

http://codereview.appspot.com/6354044/diff/1/Documentation/contributor/admini...
Documentation/contributor/administration.itexi:271: 02 0-23/2 * * * /bin/bash
-lc /home/patchy/git/lilypond-extra/patches/lilypond-patchy-staging.py
On 2012/06/27 10:11:33, Graham Percival wrote:
> For the website on http://lilypond.org, I have this in my crontab:
> 
> 
> # website-rebuild.cron
> LILYPOND_GIT=/home/graham/lilypond/lilypond-git/
> LILYPOND_WEB_MEDIA_GIT=/home/graham/lilypond/lilypond-extra/
> PATH=/home/graham/usr/bin/:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/bin 
> 
> 
> Do we need to do a similar thing for this crontab?

lilypond-patchy-staging reads LILYPOND_GIT from environment only if
git_repository_dir from configuration does not point to a valid Git repository,
so I think a default environment could be used for lilypond-patchy-staging.

>  Cron has a really annoying
> habit of not loading your default environment.  I can't remember if Patchy
> relies on those environment vars for anything, but it might be safer to add a
> warning about this.
> (or dump a @ref{Uploading and security} to see the type of thing I mean; lines
> 211 to 224 in website-work.itexi )

Oh yeah, I rely on ssh-agent for unlocking the ssh-key used by
lilypond-patchy-staging, and my ~/.bashrc (well, the .bashrc of the account used
exclusively for LilyPond development) loads ssh-agent information, so I ended up
using "/bin/bash -lc" in the crontab, but what you propose for the website is
certainly safer.

[Graham Percival, 2012-06-29 11:00:03]

LGTM, I think.  I'm not completely certain what I'm reviewing here.

I'm content to have this (whatever it is) go through, but in the future could
you:
- keep you changes in a separate git branch (locally)
- run:  git cl upload origin/master
  while on that local git branch.

Yes, this will "squish" your commits on that branch together, but this makes it
easier for reviewers to keep track of what's happening when there's multiple
versions.  I know that this is a poor limitation of rietveld.

[John Mandereau, 2012-06-29 11:57:51]

Il giorno ven, 29/06/2012 alle 11.00 +0000, graham@percival-music.ca ha
scritto:
> LGTM, I think.  I'm not completely certain what I'm reviewing here.
> 
> I'm content to have this (whatever it is) go through, but in the future
> could you:
> - keep you changes in a separate git branch (locally)
> - run:  git cl upload origin/master
>    while on that local git branch.

This is what I did, but I introduced confusion by deliberately uploading
two dstinct patches to the same rietveld issue.  These two patches can
be applied independently and in any order by patch, but "git apply"
needs an application order because the two patches touch
administration.itexi and Git patches contain kind-of file checksums.

I'll take care to avoid such confusion in the future.

Cheers,
John