From ee063d100b5d93a0ae0127a7b3f86e1d5a37a660 Mon Sep 17 00:00:00 2001 From: Evan Martin Date: Thu, 16 Aug 2012 11:25:55 -0700 Subject: many updates for HACKING.md In particular, describe a policy for good patches. --- HACKING.md | 104 +++++++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 78 insertions(+), 26 deletions(-) diff --git a/HACKING.md b/HACKING.md index 8a477f3..bc23773 100644 --- a/HACKING.md +++ b/HACKING.md @@ -1,11 +1,59 @@ +## Basic overview + +`./configure.py` generates the `build.ninja` files used to build +ninja. It accepts various flags to adjust build parameters. + +The primary build target of interest is `ninja`, but when hacking on +Ninja your changes should be testable so it's more useful to build +and run `ninja_test` when developing. + +(`./bootstrap.py` creates a bootstrap `ninja` and runs the above +process; it's only necessary to run if you don't have a copy of +`ninja` to build with.) ### Adjusting build flags - CFLAGS=-O3 ./configure.py +Build in "debug" mode while developing (disables optimizations and builds +way faster on Windows): + + ./configure.py --debug + +To use clang, set `CXX`: + + CXX=clang++ ./configure.py + +## How to successfully make changes to Ninja + +Github pull requests are convenient for me to merge (I can just click +a button and it's all handled server-side), but I'm also comfortable +accepting pre-github git patches (via `send-email` etc.). + +Good pull requests have all of these attributes: + +* Are scoped to one specific issue +* Include a test to demonstrate their correctness +* Update the docs where relevant +* Match the Ninja coding style (see below) +* Don't include a mess of "oops, fix typo" commits -### Testing +These are typically merged without hesitation. If a change is lacking +any of the above I usually will ask you to fix it, though there are +obvious exceptions (fixing typos in comments don't need tests). -#### Installing gtest +I am very wary of changes that increase the complexity of Ninja (in +particular, new build file syntax or command-line flags) or increase +the maintenance burden of Ninja. Ninja is already successfully in use +by hundreds of developers for large projects and it already achieves +(most of) the goals I set out for it to do. It's probably best to +discuss new feature ideas on the mailing list before I shoot down your +patch. + +## Testing + +### Installing gtest + +The `ninja_test` binary, containing all the tests, depends on the +googletest (gtest) library. * On older Ubuntus it'll install as libraries into `/usr/lib`: @@ -16,24 +64,26 @@ apt-get install libgtest-dev ./configure --with-gtest=/usr/src/gtest -* Otherwise you need to download it, unpack it, and pass --with-gtest - as appropriate. +* Otherwise you need to download it, unpack it, and pass + `--with-gtest` to `configure.py`. Get it from [its downloads + page](http://code.google.com/p/googletest/downloads/list). -#### Test-driven development +### Test-driven development Set your build command to ./ninja ninja_test && ./ninja_test --gtest_filter=MyTest.Name -now you can repeatedly run that while developing until the tests pass. -Remember to build "all" before committing to verify the other source -still works! +now you can repeatedly run that while developing until the tests pass +(I frequently set it as my compilation command in Emacs). Remember to +build "all" before committing to verify the other source still works! -### Testing performance impact of changes +## Testing performance impact of changes -If you have a Chrome build handy, it's a good test case. -Otherwise, https://github.com/martine/ninja/downloads has a copy of -the Chrome build files (and depfiles). You can untar that, then run +If you have a Chrome build handy, it's a good test case. Otherwise, +[the github downoads page](https://github.com/martine/ninja/downloads) +has a copy of the Chrome build files (and depfiles). You can untar +that, then run path/to/my/ninja chrome @@ -55,7 +105,8 @@ Generally it's the [Google C++ coding style][], but in brief: * Member methods are camelcase, expect for trivial getters which are underscore separated. * Local variables are underscore separated. -* Member variables are underscore separated and suffixed by an extra underscore. +* Member variables are underscore separated and suffixed by an extra + underscore. * Two spaces indentation. * Opening braces is at the end of line. * Lines are 80 columns maximum. @@ -78,7 +129,12 @@ Generally it's the [Google C++ coding style][], but in brief: sudo apt-get install asciidoc --no-install-recommends ./ninja manual -## Building on Windows +### Building the code documentation + + sudo apt-get install doxygen + ./ninja doxygen + +## Building for Windows While developing, it's helpful to copy `ninja.exe` to another name like `n.exe`; otherwise, rebuilds will be unable to write `ninja.exe` because @@ -92,6 +148,13 @@ it's locked while in use. [Python for Windows]: http://www.python.org/getit/windows/ +### Via mingw on Windows (not well supported) + +* Install mingw, msys, and python +* In the mingw shell, put Python in your path, and `python bootstrap.py` +* To reconfigure, run `python configure.py` +* Remember to strip the resulting executable if size matters to you + ### Via mingw on Linux (not well supported) * `sudo apt-get install gcc-mingw32 wine` @@ -100,14 +163,3 @@ it's locked while in use. * Build `ninja.exe` using a Linux ninja binary: `/path/to/linux/ninja` * Run: `./ninja.exe` (implicitly runs through wine(!)) -### Via mingw on Windows (not well supported) -* Install mingw, msys, and python -* In the mingw shell, put Python in your path, and: python bootstrap.py -* To reconfigure, run `python configure.py` -* Remember to strip the resulting executable if size matters to you - -## Clang - -To use clang, set `CXX`: - - CXX=clang++ ./configure.py -- cgit v0.12