From 70fccf7aad2e0f5e62e5dca5fb5f9e2cecc35865 Mon Sep 17 00:00:00 2001 From: James Lowe Date: Sat, 12 Jul 2014 22:41:57 +0100 Subject: [PATCH] CG: Update of Patchy instructions Issue 4013 Added fuller explanatory information regarding Patchy. --- .../contributor/administration.itexi | 713 +++++++++++++++--- 1 file changed, 617 insertions(+), 96 deletions(-) diff --git a/Documentation/contributor/administration.itexi b/Documentation/contributor/administration.itexi index 90fe795a0c..787a099463 100644 --- a/Documentation/contributor/administration.itexi +++ b/Documentation/contributor/administration.itexi @@ -1,4 +1,3 @@ -@c -*- coding: utf-8; mode: texinfo; -*- @node Administrative policies @chapter Administrative policies @@ -9,7 +8,7 @@ don't fit anywhere else. * Meta-policy for this document:: * Environment variables:: * Meisters:: -* Patchy:: +* Automated testing with Patchy:: * Administrative mailing list:: * Grand Organization Project (GOP):: * Grand LilyPond Input Syntax Standardization (GLISS):: @@ -137,153 +136,675 @@ Currently: Francisco @end itemize -@node Patchy -@section Patchy -@subheading Introduction +@node Automated testing with Patchy +@section Automated testing with Patchy -Patchy is a set of Python scripts to automate two administrative -tasks: +@menu +* Overview of Patchy:: +* Patchy requirements:: +* Installing Patchy:: +* Configuring Patchy:: +* Running the scripts:: +* Automating Patchy:: +* Troubleshooting Patchy:: +@end menu + +@node Overview of Patchy +@subsection Overview of Patchy + +Patchy is a set of Python scripts used for testing patches or testing & +pushing new commits added to @code{remote/origin/staging} to +@code{remote/origin/master}. + +No programmatic skill is required to run either of the scripts; although +knowledge of compiling LilyPond and its documentation along with +understanding how to configure the @var{PATH} environment of your +computer is required. See @ref{Working with source code}. + +The two scripts that are used for each function are: @itemize @item -@code{lilypond-patchy-staging.py}: checks that new commits in -@code{staging} can compile the regtests and documentation before -merging @code{staging} into @code{master}. +@code{test-patches.py}. This script tests issues labelled as +@qq{Patch-New} on the LilyPond issue tracker. Part of the testing +process involves running the regression tests, so this script always +requires some human intervention in order to visually check for any +differences that may be present after a successful test has occurred. + +@item +@code{lilypond-patchy-staging.py}. This script checks for any new +commits in @code{remote/origin/staging}, makes sure that the new HEAD +compiles along with all the LilyPond documentation. Then finally +pushing to @code{remote/origin/master}. This script can be run and left +unattended, requiring no human intervention. + +@end itemize + +Both of the scripts can be run independently of each other and it is not +necessary to be able to run both. So if you wanted to contribute to +LilyPond development, for example by @emph{just} testing patches then +this would still be a helpful contribution to LilyPond's development. + +Patchy can also be configured to send emails after each successful (or +unsuccessful) operation. This is not a requirement and is turned off +by default. + +@c Need to explain in more detail how to set up Patchy for email but +@c as I don't use myself it I have no experience - JL + + +@node Patchy requirements +@subsection Patchy requirements + +@unnumberedsubsec Testing new patches -(completely automatic) +@itemize + +@item +A full local copy of the source code. See +@ref{Working with source code}. @item -@code{test-patches.py}: checks that patches apply to Git @code{master}, -compile, and lets a human check that there are no big unintended -changes to the regtests. +All the software needed for compiling LilyPond @emph{and} the +documentation. Although being able to build the full set of LilyPond's +manuals is not mandatory for testing (most) patches, part of the patch +testing process requires that the regression tests are run and it is +this that requires the software normally used for compiling +documentation. See @ref{Compiling}. -(requires some human input) +@item +Commit access is @emph{not} required to test patches, but a valid login +to @uref{http://code.google.com/} @emph{is}. @end itemize -@subheading Installing Patchy -To install Patchy, you should do the following: +@unnumberedsubsec Testing & pushing new commits + +@itemize + +@item +A full local copy of the source code. See +@ref{Working with source code}. -@enumerate @item -Create a new user on your box to run Patchy; this is a security -step for your own protection. It is recommended that this should -not be an administrator. New users are created from System; -Administration; Users and Groups. +All the software needed for compiling LilyPond @emph{and} the +documentation. Unlike testing patches, being able to build the full set +of LilyPond's documentation is required to be able to test & push new +commits. See @ref{Compiling}. @item -Get the Patchy scripts from +Commit access @emph{is} required to test and push new commits, but a +valid login to @uref{http://code.google.com/} is @emph{not}. See +@ref{Commit access}. + +@end itemize + + +@node Installing Patchy +@subsection Installing Patchy + +The Patchy scripts are not part of the LilyPond code base, but can be +downloaded from @uref{https://github.com/gperciva/lilypond-extra/}. The +scripts and related Python libraries are all located in the +@file{patches/} directory. + +Alternatively, use @code{git clone}; + @example -@uref{https://github.com/gperciva/lilypond-extra/} +git clone https://github.com/gperciva/lilypond-extra/ @end example -Patchy is in the @file{patches/} directory. + +This makes it simpler to update the scripts if any changes are ever made +to them. Finally, add the location of the @file{patches/} directory to +your @var{PATH}. + + +@node Configuring Patchy +@subsection Configuring Patchy + +@warning{It is recommended to create a new user on your computer +specifically to run the Patchy scripts as a security precaution and that +this user should not have any administrative privileges. Also do not +set password protection for your ssh key else you will not be able to +run the scripts unattended.} + +@enumerate @item -Put the scripts and Python libraries contained in @file{patches} in a -sensible place on your system; this can be done by appending -@file{patches/} full path to the @var{PATH} of the user that runs -Patchy. +Make sure the environment variables @var{LILYPOND_GIT} and +@var{LILYPOND_BUILD_DIR} are configured appropriately. See +@ref{Environment variables}. @item -Create a new git repository with +To save being prompted for your login and password to +@uref{http://code.google.com/} when testing patches. create a +@emph{plain-text} file in your Patchy user's @code{$HOME} directory +called @code{.lilypond-project-hosting-login} containing your login and +password, each on a separate line. + @example -git clone git://git.sv.gnu.org/lilypond.git +joe_smith123@@gmail.com +mYp455w0rd! @end example -This will create a directory called lilypond with the repo in it. -Make sure it's where you want it and name it lilypond-git -(assuming you want to follow the standard naming conventions). @item -Create environment variables @var{LILYPOND_GIT} and -@var{LILYPOND_BUILD_DIR}, see @ref{Environment variables}. +Manually run either the @code{test-patches.py} or +@code{lilypond-patchy-staging.py} scripts and when prompted: + +@smallexample +Warning: using default config; please edit /home/joe/.lilypond-patchy-config +Are you sure that you want to continue with the default config? (y/[n]) +@end smallexample + +Answer @qq{@code{n}} and press enter. + +The next time either of the scripts are run they will use the +@code{.lilypond-patchy-config} settings copied to your @code{$HOME} +directory. @item -Run Patchy once to set up config files, answer @q{@code{n}} when it -asks for going on, unless the default config file happens to suit your -setup: +Manually edit the @file{.lilypond-patchy-config} file, located in your +@code{$HOME} directory to change any of the default settings. + +@end enumerate + +These include: + +@itemize + +@item +All @code{make} operations are run with; @example -lilypond-patchy-staging.py +extra_make_options = -j3 CPU_COUNT=3 @end example +See @ref{Saving time with the -j option} + @item -Edit @file{$HOME/.lilypond-patchy-config} to provide the location of -your local lilypond Git repository, working directories for your build -directory, your results directory, compiler options and notification -method. If you don't want to use email notification, then delete -everything after @code{smtp_command:}. +A complete build of all the LilyPond documentation is @emph{not} +performed; +@example +patch_test_build_docs = no +@end example @item -Ensure that your new user has git push access. Follow the -instructions in the CG at @ref{Commit access}. Do not set -password protection for the key --- if you do you will not be able -to run patchy unattended. +Each instance of either a patch test or commit test & push is logged in; +@example +auto_compile_results_dir = ~/lilypond-auto-compile-results/ +@end example -@end enumerate +@item +Both scripts will perform their build operations in; +@example +build_dir = /tmp/lilypond-autobuild/ +@end example + +@end itemize -@subheading lilypond-patchy-staging.py +Each completed patch test will also generate its own directory in +@file{/tmp/...} labelled with the tracker issue number prefixed by +@code{show-}. +@example +\tmp\show-3446 +@end example + +Both the scripts create clones of @code{staging} and @code{master} +branches (prefixed with @code{test-}) with a third branch, called +@code{test-master-lock} used as a check to protect against two or more +instances of Patchy being run locally at the same time. + + +@node Running the scripts +@subsection Running the scripts + +@unnumberedsubsec Testing & pushing new commits + +@code{lilypond-patchy-staging.py} is run @emph{without} any arguments. +It then checks to see if @code{remote/origin/staging} is +@qq{further ahead} than @code{remote/origin/master}. + +@noindent +If there are no new differences between the two branches since the last +run check, the script will report something like this: + +@smallexample +(UTC) Begin LilyPond compile, previous commit at 4726764cb591f622e7893407db0e7d42bcde90d9 +Success: No new commits in staging +@end smallexample + +@noindent +If there are any differences between the two branches since the last +run check, (or if the script cannot for any reason, locate the last +instance of a commit that it checked) it will report something like +this: + +@smallexample +(UTC) Begin LilyPond compile, previous commit at 4726764cb591f622e7893407db0e7d42bcde90d9 +Merged staging, now at: 79e98a773b6570cfa28a15775a9dea3d3e54d6b5 + Success: ./autogen.sh --noconfigure + Success: /tmp/lilypond-autobuild/configure --disable-optimising +... +@end smallexample + +and proceed with running @code{make}, @code{make test} and a +@code{make doc}. Unlike @code{test-patches.py} if all the tests pass, +the script then pushes the changes to @code{remote/origin/master}. + +@smallexample +... +Success: nice make clean +Success: nice make -j7 CPU_COUNT=7 +Success: nice make test -j7 CPU_COUNT=7 +Success: nice make doc -j7 CPU_COUNT=7 +To ssh://joe@@git.sv.gnu.org/srv/git/lilypond.git + 79e98a7..4726764 test-staging -> master + Success: pushed to master +@end smallexample + +@warning{In the case where any of the @code{lilypond-patchy-staging.py} +tests fail, do not try to push your own fixes but report the failures to +the Developers List for advice.} + + +@unnumberedsubsec Testing new patches + +When run without any argument, @code{test-patches.py} will check +@uref{http://code.google.com/p/lilypond/issues/list} for all tracker +issues that are marked with the label @code{Patch-new}. It then scrapes +the issue, looking for the last Rietveld URL entered. It then downloads +the patch file and applies it to @code{test-master}. + +Here is an example where two tracker issues labeled as @code{Patch-new} +were detected: + +@smallexample +... +issues [4007, 4008] +Trying issue 4007 +Found url: http://codereview.appspot.com/112210043 +Found patch: 4007,/home/joe/lilypond-git/issue112210043_1.diff, +Trying issue 4008 +Found url: http://codereview.appspot.com/115770043 +Found patch: 4008,/home/joe/lilypond-git/issue115770043_1.diff, +Fetching, cloning, compiling master. +... +@end smallexample + +If run no tracker items with the @var{Patch-New} label are found it will +report: -@code{lilypond-patchy-staging.py} is run with @example -python lilypond-patchy-staging.py +issues [] @end example -Not much appears to happen except you can see a lot of CPU gets used -if you open System Monitor. There's not much point running -@code{lilypond-patchy-staging.py} unless there is something in -@code{staging} to be merged to @code{master}, however, if there's -nothing new in @code{staging} then the script won't waste resources by -compiling anything. - -The script fetches the current patches in staging and runs -@code{make}, @code{make test} and @code{make doc} to ensure that all of -these complete error-free. If you have set Patchy up to use email, -it emails its results to you. If you haven't, then you can view -them in a logfile. It also merges @code{staging} into @code{master}. - -@warning{in case the build fails, do not try to push fixes on top of -staging branch, for details see @ref{Pushing to staging}.} - -When you have run Patchy a few successful times with email sending, -you are ready for running it as a cron job. First, make sure you have -the following in @file{$HOME/.lilypond-patchy-config} to avoid -email flood: + +The script can also be run using the tracker issue number(s) as an +argument regardless if the @var{Patch-New} label has been assigned; @example -[notification] -notify_non_action = no +test-patches.py 4006 +@end example + +or + +@example +test-patches.py 4006 3992 4020 +@end example + +The script then checks to see if any previously +@code{make test-baseline}s have been generated and if the commit ID of +@code{remote/origin/master} is different from that previously completed +test. + +@noindent +If no previous @code{make test-baseline} test is discovered or if the +commit ID of @code{remote/origin/master} has changed, then a new +@code{make test-baseline} will run first automatically before the patch +is tested: + +This shows when the commit ID has changed: + +@smallexample +... +(UTC) Begin LilyPond compile, previous commit at 3f92dcb2c81dcd2755542b57a0a5f2039f29a211 +Merged master, now at: 4726764cb591f622e7893407db0e7d42bcde90d9 + Success: ./autogen.sh --noconfigure + Success: /tmp/lilypond-autobuild/configure --disable-optimising + Success: nice make clean + Success: nice make -j3 CPU_COUNT=3 + Success: nice make test-baseline -j3 CPU_COUNT=3 + Success: nice make doc -j3 CPU_COUNT=3 + Success: nice make doc-clean +... +@end smallexample + +@noindent +If a previous regression test @emph{is} discovered @emph{and} if the +commit ID of @code{remote/origin/master} has not changed, then the patch +will be tested against the previous @code{make test-baseline} without +the need to re-generate a new one: + +@smallexample +... +issues [4009] +Trying issue 4009 +Found url: http://codereview.appspot.com/110540043 +Found patch: 4009,/home/joe/lilypond-extra/patches/issue110540043_1.diff, +Fetching, cloning, compiling master. +(UTC) Begin LilyPond compile, previous commit at 4726764cb591f622e7893407db0e7d42bcde90d9 + Success: No new commits in master +Using test baseline from previous build. +... +@end smallexample + +The patch is then applied and a @code{make} and @code{make check} are +run. A full @code{make doc} is also run if the +@file{.lilypond-patchy-config} file has been edited accordingly; + +@smallexample +... +Issue 4009: +Issue 4009: Testing patch issue110540043_1.diff + Success: git apply --index /home/joe/lilypond-git/issue112210043_1.diff + Success: ./autogen.sh --noconfigure + Success: /tmp/lilypond-autobuild/configure --disable-optimising + Success: nice make clean + Success: nice make -j3 CPU_COUNT=3 + Success: nice make check -j3 CPU_COUNT=3 + Success: nice make doc -j3 CPU_COUNT=3 +... +@end smallexample + +Once all the tests have run (successfully or not), the script will clean +up from the previous patch and, if required, start testing the next +issue; + +@smallexample +... +Issue 4007: Cleaning up + Success: nice make test-clean + Success: nice make doc-clean + Success: nice make clean + Success: git reset --hard +Issue 4007: Done. +Issue 4008: +Issue 4008: Testing patch issue115770043_1_diff + Success: git apply --index /home/joe/lilypond-git/issue115770043_1.diff + Success: ./autogen.sh --noconfigure +... +@end smallexample + +and so on. + +@unnumberedsubsubsec Checking the regression test results + +Assuming the patch passed all the @code{make} tests, the regression +differences will be located in the @file{/test-results/} directory +within the build location for the patch issue number; + +@example +/tmp/show-4007/test-results/ +@end example + +Open @file{index.html} in a browser of your choice to view any +differences. + +@noindent +Alternatively if the Firefox browser is installed, then the regression +test results can be opened by calling the appropriate +@file{show-regtests-} file located in the auto-compile log location; + +@example +sh ~/lilypond-auto-compile-results/show-regtests-4007 +@end example + +See @ref{Regression tests}. + +@unnumberedsubsec Reporting test results + +Once a patch has been tested and the regression tests have been +manually checked, the tracker can be updated manually by editing the +tracker issue directly in the web browser or by using two additional +python scripts that are included as part of the Patchy suite. + +@unnumberedsubsubsec For patches that have passed + +Use the @code{accept-patch.py} script and run it with the Google issue +tracker number (not the Rietveld issue number) as an argument; + +@example +accept-patch.py 4007 +@end example + +This will automatically update the tracker issue with the phrase +@qq{Patchy the autobot says: passes tests.}. + +@noindent +It is also possible to add additional information to the default +message by adding a second argument within double-quote marks. + +@example +accept-patch.py 4007 "This also includes a full documentation build." +@end example + +The tracker issue's label is then changed automatically to +@qq{Patch-review}. + +@unnumberedsubsubsec Patches that have failed + +Use the @code{reject-patch.py} script and run it with the Google issue +tracker number (not the Rietveld issue number) as an argument but you +@emph{must} also include a second argument, in double-quotes, stating +the reason the patch has been rejected; + +@example +reject-patch.py 4007 "Fails the 'make check' test." @end example -Then, assuming Patchy run with user account @code{patchy}, write the -following to @file{$HOME/lilypond-patchy.cron}, adapting it as -necessary (the @code{/2} means @qq{run this every 2 hours}): +Once the @code{reject-patch.py} script has been run, the tracker issue's +label is changed automatically to @qq{Patch-Needs_work}. + + +@node Automating Patchy +@subsection Automating Patchy + +To run as a cron job make sure you have; + @example -02 0-23/2 * * * /home/patchy/git/lilypond-extra/patches/lilypond-patchy-staging.py +[notification] +notify_non_action = no @end example -@warning{@code{cron} will not inherit environment variables from -your main setup, so you must re-define any variables inside -@file{$HOME/lilypond-patchy.cron}. For instance, @var{LILYPOND_GIT} -may need to be defined if @var{git_repository_dir} is not correctly -set in @file{$HOME/.lilypond-patchy-config}.} +in @file{$HOME/.lilypond-patchy-config} to avoid any unintentional email +flooding: + +Assuming that Patchy run a user @qq{patchy}, create a file called +@file{$HOME/lilypond-patchy.cron}, adapting it as necessary (the +@code{/2} means @qq{run this every 2 hours}): + +@smallexample +02 0-23/2 * * * /home/patchy/lilypond-extra/patches/lilypond-patchy-staging.py +@end smallexample -Finally, install the cron job (you may need superuser privileges for +@warning{@code{cron} will not inherit environment variables so you must +re-define any variables inside @file{$HOME/lilypond-patchy.cron}. For +instance, @var{LILYPOND_GIT} may need to be defined if +@var{git_repository_dir} is not correctly set in +@file{$HOME/.lilypond-patchy-config}.} + +Finally, apply the cron job (you may need superuser privileges for this): + @example crontab -u patchy /home/patchy/lilypond-patchy.cron @end example -@subheading test-patches.py -@code{test-patches.py} prepares a regtest comparison for a human to -quickly glance at, to determine if the patch is ready for a review. -After looking at the comparison (or the lack of a comparison in the -case of problems), run @code{accept-patch.py} or -@code{reject-patch.py}. - -Once a patch has gotten a "LGTM" from Patchy, it should be -reviewed by relevant developers, and if it passes this, it can be -considered for countdown (see @ref{Commits and patches}) and -pushing to staging (see @ref{Pushing to staging}). + +@node Troubleshooting Patchy +@subsection Troubleshooting Patchy + +The following is a list of the most common messages that the scripts +may report with explanations. + +@itemize + +@item +@example +issues [] +@end example + +@itemize + +@item +There are currently no tracker issues with the @code{Patch-New} status. + +@item +If specific tracker issue number has been used as an argument when +running @code{test-patches.py}, then the issue contains no URL to +Rietveld. + +@end itemize + +@item +@smallexample +this Git revision has already been pushed by an operator other than this Patchy. +@end smallexample + +@itemize + +@item +Another, remote, machine has already tested and pushed the new commits +in staging. + +@item +You may also see this on your local machine if the auto-build files +have been deleted and this computer has previously already pushed the +listed commit ID to @code{master}. + +@end itemize + + + +@item +@smallexample +Git revision has not changed but checksum of test baseline has, must rebuild. +@end smallexample + +This occurs when Patchy detects that the commit ID has not changed +since the last test but it cannot locate the last +@code{make test-baseline} (usually because it has been deleted or moved) +and so a new @code{test-baseline} is rebuilt. + +@item +@example +Last patch for issue xxxx already tested or under testing +by another Patchy instance, skipping. +@end example + +@itemize + +@item +There is another instance of Patchy running on your computer that is +testing the same tracker issue. + +@item +A previous test attempt was unsuccessful for some reason and the scripts +were not able to tidy up after themselves (for example if you manually +halt the testing process by killing it or closing the terminal you may +have been running the script in). + +@noindent +There is a hidden file located in the @code{$HOME} directory of the user +running Patchy called @code{.lilypond-patchy-cache} that records the +current patches that are being tested, have been tested and the commit +ID of @code{remote/origin/master} since the last test. It will contain +a line like this: + +@example +[3995] +issue105560044_120001_diff = testing +@end example + +for any issue that it thinks is still in the process of being tested. + +@noindent +Manually remove this entry and re-run the script. + +@end itemize + +@item +@example +test-master-lock and PID entry exist but previous Patchy +run (PID xxxxx) died, resetting test-master-lock anyway. +@end example + +@item +A previous test attempt was unsuccessful for some reason and the scripts +were not able to tidy up after themselves (for example if you manually +halt the testing process by killing it or closing the terminal you may +have been running the script in). The @code{test-master-lock} branch +was therefore not able to be deleted cleanly however, nothing needs to +be done the scripts will rebuild any tests it needs to. + +@item +@example +fatal: A branch named 'test-master-lock' already exists. +@end example + +@itemize + +@item +There is another instance of Patchy running on your computer that is +testing the same tracker issue. + +@item +A previous test attempt was unsuccessful for some reason and the scripts +were not able to tidy up after themselves (for example if you manually +halt the testing process by killing it or closing the terminal you may +have been running the script in). The @code{test-master-lock} branch +was therefore not able to be deleted cleanly, in this case you must +manually delete the @code{test-master-lock} branch in your +@code{$LILYPOND_GIT} directory. + +@example +git branch -d test-master-lock +@end example + +@noindent +It may be wise to also manually delete @code{test-master} and +@code{test-staging} too, just to be safe. + +@end itemize + +@item +@example +*** FAILED STEP *** + merge from staging + Another instance (PID xxxxx) is already running. +@end example + +@noindent +This occurs when trying to run @code{lilypond-patchy-staging.py} when +another instance of either script is already running locally. + +@item +@example +Warning: something went wrong; omitting patch for issue 3976 +@end example + +@itemize + +@item +The Rietveld URL as listed in the tracker is incorrect (e.g. missing or +incorrect issue number + +@item +The patch on Rietveld is too large to download + +@end itemize + +@end itemize @node Administrative mailing list -- 2.39.2