0f4d40a6b3c6636ac7e9bc50fc36dd702c2e7752
[akaros.git] / Documentation / Contributing.md
1 Akaros Contribution Policies
2 ===========================
3 2015-04-27 Barret Rhoden (brho)
4
5 Interested in contributing to Akaros?  This document covers coding standards,
6 version control usage, and other contribution policies.
7
8 Contents
9 ---------------------------
10 Licensing
11 Contribution Philosophy
12 Contribution Steps
13 Coding Standard
14 Git Workflow
15         - Setup
16         - A Basic Commit
17         - A Bigger Change: Several Commits!
18         - Someone Changed origin/master; My Branch is Now Old
19         - Oh No, I Committed and Pushed Something I Want to Change
20         - Crap, Someone Reset a Branch I was Tracking!
21         - What Happens When Two People Push -f
22
23
24 Licensing
25 ---------------------------
26 By contributing to Akaros, you are agreeing with the Developer Certificate of
27 Origin (found in Documentation/ and at http://developercertificate.org/).
28
29 All contributions of any sort, including code from other projects, must be
30 compatible with the GPLv2.
31
32 All new code should be licensed "GPLv2 or later".
33
34 You (or your employer) may retain the copyright to your contributions, if
35 applicable.  We do not require copyright assignment.
36
37
38 Contribution Philosophy
39 ---------------------------
40 "Do the right thing."  If you've discovered a problem, figure out how to fix it
41 and fix it right.  You do not need to solve every problem, but think a little
42 and don't be short-sighted.  (Grep the codebase for TODO.)  For example, if you
43 want to disable the legacy USB devices, then go ahead.  You do not need to
44 write an entire USB stack, but you should expect that stack to be written in
45 the future.
46
47 You 'own' your patches and changes, meaning both that they are your
48 responsibility and you are free to pursue a solution in your own way.  You must
49 be happy with your commits; I must be ok with them.  If you're happy with
50 something half-assed, then you're in the wrong line of work.
51
52 Expect push-back on various things and plan on reworking your commits.  We
53 should be able to reach a good solution through discussion and, on occasion,
54 arguments.
55
56 Discussion and arguments about patches and technical content is fine.
57 Attacking people is not.
58
59
60 Contribution Steps
61 ---------------------------
62 The short version of adding code to Akaros:
63 - Email the list early and often
64 - Push to a remote branch (not master)
65 - Get someone (brho) to shepherd your work
66 - Fix up your commits until they are merged
67
68 Contact the list before starting on a major project.  I may already have plans
69 or initial thoughts on the matter, and someone on the list can provide advice.
70 Likewise, don't break long term plans or be too invasive for no reason.
71 Otherwise, I might not accept the change at all.
72
73 The end goal for a major change is to have a sane series of patches that
74 applies cleanly to origin/master.  Sane, means there are not a lot of "oh wait,
75 this patch is wrong, reverting".  The patch series should tell a story of
76 getting from A to B, where each patch is easily reviewable.
77
78 Push your changes to a separate branch, and email the list asking for advice or
79 review.  Someone, usually me, will shepherd your work, providing feedback and
80 TODOs.  Name your branch something reasonable (e.g. "inferno-b" and
81 "inferno-r").  Your username is a suitable branch name, if you don't want to
82 bother with a title.  If you do not have access to the brho:origin/ repo, then
83 push it somewhere else on github and email us your git remotes info.  The only
84 people with push access are those I know personally.  As the group grows, that
85 will not scale and people will need to start using their own repos.  For now, a
86 central repo works fine.
87
88 Thanks to git's fast and simple branching, we can work and rework on branches
89 that are not master.  Reworking branches includes rewriting commits and git's
90 history.  (See below for some git commands).  The general rule is that there is
91 no expectation of stability in any branch other than master.  Those branches
92 may be rebased without warning by people working on the branch.  You 'own'
93 whatever branch you push.
94
95 In general, I prefer not to merge, but it may be necessary based on the age of
96 the changed branch.  If we scale out and have various submaintainers, we'll
97 merge those branches into the main repo.  But at this point, the brho:origin/
98 repo is a 'leaf' (the one and only leaf).  We'll hold off on merging until we
99 have many leaves.
100
101 For minor changes and bug fixes, just push them or email a patch.  Use your
102 better judgement.
103
104 When adding new patches to a branch, you should rebase your commits on top of
105 the branch's latest commit, then do a fast-forward merge.
106
107 Your commits should be of a reasonable size.  Try to isolate your commits to
108 logically consistent, easily verifiable changes.  Each commit should compile,
109 but you don't want to change everything in one commit.
110
111 If a patch relied on spatch, provide the commands or scripts used.
112
113 Remember that someone other than yourself  has to look at each of these
114 commits.
115
116
117 Coding Standard
118 ---------------------------
119 Our coding standard is similar to the Linux kernel style (but with a tabstop of
120 4), so when in doubt, do what they do.
121
122 - If you are bringing in code from a project other than Linux, run
123   scripts/lindent.sh on it.  Lindent isn't perfect, but it's close enough.
124   Linux code is usually fine (even with 8 space tabs), and lindent sometimes
125   does more harm than good on that code.
126
127 - You'll note that old code may lack the correct style, especially due to the
128   large bodies of code from other systems involved.  Typically, we fix it when
129   fixing the code for other reasons, so that git blame points at the most
130   recent relevant change.
131
132 - No spaces after the name of a function in a call.  For example,
133   printk("hello") not printk ("hello").
134
135 - Functions that take no arguments are declared f(void) not f().
136
137 - Function names are all lower-case, separated by underscores.
138
139 - One space after commas.  For example foo(x, y, z), not foo(x,y,z);
140
141 - One space after keywords "if", "for", "while", "switch".  For example, if
142   (x) not if(x).
143
144 - Space before braces.  For example, if (x) { not if (x){.
145
146 - For if/while/etc, the opening brace is on the same line as the if.  If you
147   have just one line, no brace is necessary.  If you have an if / else clause,
148   and one of them has braces, put braces on both.  An else line should start
149   with a brace.
150
151 - Beginning-of-line indentation via tabs, not spaces.  Use spaces for
152   additional formatting (such as lining up text when word wrapping).
153
154 - Preprocessor macros are usually upper-case.  Try to avoid using macros
155   unless necessary (I prefer static inlines).
156
157 - Pointer declarations have the * on the variable.  void *x, not void* x.
158
159 - Multi-word names are lower_case_with_underscores.  Keep your names short,
160   especially for local variables.
161
162 - 'Permanent' comments in are C /* ... */ comments.  Only use C++ style (//)
163   for temporary commenting-out, such as if you want to not call a function.
164   If you want to block out large chunks of code, use #if 0 ... #endif
165
166 - In a function definition, the function name shoud NOT start a new line.  Old
167   code incorrectly does this, and should be changed over time.
168  
169 - Feel free to use gotos, especially in functions that have cleanup before
170   they exit, or in other places that enhance readability.
171
172 - 80 characters per line.  When you wrap, try to line things up so they make
173   sense, such as space-indenting so function arguments line up vertically.
174
175 - Do not typedef structs.  You can typedef function pointers when convenient.
176   Keep in mind that typedefs require people to look up the real type when they
177   analyze the code.
178
179 - Try to avoid making lots of separate files or extravagant directory
180   hierarchies.  Keep in mind that people will need to work with and find these
181   files (even via tab completion, it can be a pain).
182
183
184 Git Workflow
185 ------------------------------------
186 Git allows us to do a lot of cool things.  One of its primary purposes is to
187 provide a means for us to review each others code.  In this section, I'll
188 describe how I use git to make changes to Akaros.
189
190 ### Setup
191
192 First off, visualizing the git tree is extremely useful.  I use:
193
194 $ gitk --all &
195
196 If you only have a terminal, you can use git log for a similar effect:
197
198 $ git log --graph --full-history --all --color
199 --pretty=format:"%x1b[31m%h%x09%x1b[32m%d%x1b[0m%x20%s%x20%x1b[33m(%an)%x1b[0m"
200
201 That's pretty crazy, so I have the following in my ~/.gitconfig
202
203 [alias]
204             gr  = log --graph --full-history --all --color --pretty=format:"%x1b[31m%h%x09%x1b[32m%d%x1b[0m%x20%s%x20%x1b[33m(%an)%x1b[0m" 
205         grs = log --graph --full-history --all --color --pretty=format:"%x1b[31m%h%x09%x1b[32m%d%x1b[0m%x20%s%x20%x1b[33m(%an)%x1b[0m" --simplify-by-decoration
206
207 With those aliases, I only need to do `git gr`.
208
209
210 ### A Basic Commit
211
212 Let's say you want to make a minor change to say, the kernel monitor.  To start
213 things off, make sure you are up to date.
214
215 $ git checkout master           # make sure you're on master
216 $ git fetch                             # grab the latest from origin/
217 $ git merge origin/master
218
219 Many people simply do a git pull to get the latest.  I tend to fetch, then
220 merge manually after glancing at the stream of changes.  If you ever see a
221 (XCC) in one of the commit messages, then you may need to rebuild your
222 toolchain.
223
224 Time to make our change.  Let's make a branch called 'monitor-change':
225
226 $ git checkout -b monitor-change
227
228 Edit, compile, and test a change, say in kern/src/monitor.c
229
230 $ git add -p kern/src/monitor.c
231
232 The -p flag allows you to incrementally add and review each diff chunk.  It's
233 very useful for committing only what you want from a set of changes in your
234 working directory.
235
236 $ git status
237
238 Take a look at things before you commit; see if you missed any files or added
239 anything you don't want.
240
241 $ git commit
242
243 Write a descriptive message, with a short, one-line summary first.  e.g.
244
245 ```
246 commit 52cc9b19c6edfc183276b1ebf24af9baea4d20d2
247 Author: Barret Rhoden <brho@cs.berkeley.edu>
248 Date:   Mon Jan 19 18:36:07 2015 -0500
249
250     Fixes kernel argument checking in "m"
251     
252     Need at least one argument before dereferencing.
253     
254     Also, debugcmd is pretty handy.
255 ```
256
257 Oh wait, say you messed up the commit message or some part of the code.  You
258 can make the change and amend the commit.
259
260 $ vi kern/src/monitor.c
261 $ git add -p kern/src/monitor.c
262 $ git commit --amend                    # can also change the commit message
263
264 Now that you are happy, push it.
265
266 $ git push origin monitor-change
267
268 Email the mailing list or your nearest shepherd, and we'll take a look!
269
270
271 ### A Bigger Change: Several Commits!
272
273 Say you are working on the monitor, and you realize your change is quite large
274 and probably should be several smaller, logically consistent commits.
275
276 You may have already added some of the changes to the index.  To reset the
277 index (what git will commit in the next commit):
278
279 $ git reset HEAD
280
281 Now, your changes are unstaged.  Confirm with:
282
283 $ git status
284
285 Now you want to add some, but not all, of your changes to monitor.c:
286
287 $ git add -p kern/src/monitor.c kern/src/other_files_too.c
288
289 Select only the changes you want, then commit:
290
291 $ git commit  # write message
292
293 Then make further commits:
294
295 $ git add -p kern/src/monitor ; git commit
296 $ git add -p kern/src/monitor ; git commit
297
298 $ git push origin monitor-change
299
300
301 ### Someone Changed origin/master; My Branch is Now Old
302
303 Typically, we want branches to descend from origin/master.  But if multiple
304 people are working at a time, someone will fall behind.  With Git, dealing with
305 this situation is extremely easy.  You just need to rebase your current branch
306 on the new master.
307
308 Say you are on the branch monitor-change, and origin/master has updated.
309 Here's a reasonable workflow to deal with this situation:
310
311 $ git fetch                             # notice origin/master changed
312 $ git checkout master           # switch to master
313 $ git merge origin/master       # fast-forward merge - straight line of commits
314
315 Master is now up to date.  git pull --rebase does this too, though I prefer to
316 always fetch first.
317
318 $ git checkout monitor-change   # needed if you aren't on monitor-change
319 $ git rebase master
320
321 Now monitor-change descends from the latest master.
322
323 A short-cut for the last two commands is:
324
325 $ git rebase master monitor-change
326
327 The last parameter is simply an implied `git checkout`.
328
329 Don't forget to recompile and skim the log from the master update.  If you
330 already pushed your branch to the repo, you'll want to update that branch.
331
332 $ git push -f origin monitor-change
333
334 The -f replaces the origin/monitor-change with your monitor-change.  See below
335 for more on -f.
336
337 If you don't want to bother with maintaining your master right now, you can cut
338 it down to a few steps (assuming you are on SOMEBRANCH):
339
340 $ git fetch
341 $ git rebase origin/master
342 $ git push -f origin/SOMEBRANCH
343
344
345 ### Oh No, I Committed and Pushed Something I Want to Change
346
347 Fear not, adjusting git's history is very easy, and we do it all the time on
348 branches other than origin/master.
349
350 Let's say you have a string of commits, e.g.: (edited from my git gr)
351
352 * 876d5b0        (HEAD, vmmcp, origin/vmmcp) VMM: handle EPT page faults
353 * f926c02        Fixes VMR creating off-by-one
354 * f62dd6c        VMM: Removes the epte_t from pte_t   Crap - bugs!
355 * 08e42d6        VMM: Call EPT ops for every KPT op
356 * d76b4c6        Redefines PTE present vs mapped      Crap - debug prints!
357 * ddb9fa7        x86: EPT and KPT are contiguous
358 * 1234567        (master, origin/master) Some Other Commit
359
360 You're working on the vmmcp branch, and you realize that d76b4c6 adds a bunch
361 of debug printks in it and that there is a bug in HEAD that you introduced in
362 f62dd6c.
363
364 The lousy option would be to make a new commit on top of 876d5b0 (HEAD) that
365 removes the prints and fixes the bugs.  The problem with this is that there is
366 no reason for people to even know those existed in the first place, and by
367 leaving the bugs and printks in old code you only increase the load on your
368 shepherd.
369
370 Not to worry, you can use git rebase -i to interactively replay the commits
371 from master..vmmcp and rewrite the commits along the way.
372
373 First up, make sure you are on vmmcp and do a rebase -i.
374
375 $ git checkout vmmcp
376 $ git rebase -i master
377
378 This will replay all commits from (1234567, 876d5b0] (not including 1234567) on
379 top of master (1234567).  Check out git help rebase for more info, including some nifty diagrams.  In short, that rebase command is the shorthand for:
380
381 $ git rebase -i --onto master master vmmcp
382
383 The rebase command -i option (interactive) will pop open an editor, where you
384 can change the order of commits, squash multiple commits into a single commit,
385 or stop and edit the commit.  Note that commits are presented in the order in which they will be committed, which is the opposite of gitk and git gr, where the newest commit is on top.
386
387 Let's edit the commit d76b4c6 (change 'pick' to 'e').  Save and exit, and the rebase will proceed.  When git hits that commit, it will stop and you can amend your commit.
388
389 $ vi somefile                   # make your changes
390 $ git add -p somefile   # add them to the index (building a commit)
391 $ git commit --amend    # amend your *old* commit (d76b4c6)
392 $ git status                    # see how you are doing
393 $ git rebase --continue
394
395 Now check it out in gitk or git gr.  It's like your debug printks never
396 existed.
397
398 Another approach is to build a temporary commit and use it as a fixup commit.
399 Let's do that to fix the bug in f62dd6c.
400
401 $ git checkout vmmcp    # start from the top of the tree
402 $ vi somefile                   # make your changes
403 $ git add -p somefile   # stage the changes
404 $ git commit -m WIP-fixes-vmm-bug
405
406 You now have something like:
407
408 * 13a95cc        (HEAD, vmmcp) WIP-fixes-vmm-bug
409 * 876d5b0        (origin/vmmcp) VMM: handle EPT page faults
410 * f926c02        Fixes VMR creating off-by-one
411 * f62dd6c        VMM: Removes the epte_t from pte_t   Crap - bugs!
412 * 08e42d6        VMM: Call EPT ops for every KPT op
413 * d76b4c6        Redefines PTE present vs mapped      Crap - debug prints!
414 * ddb9fa7        x86: EPT and KPT are contiguous
415 * 1234567        (master, origin/master) Some Other Commit
416
417 Clearly you don't want that WIP commit (WIP = work in progress).  Let's rebase!
418
419 $ git rebase -i master
420
421 Reorder the commits so that the WIP happens after f62dd6c, and change the WIP's
422 command from 'pick' to 'f'.
423
424 Assuming everything applies cleanly, you're done.  The WIP approach is even
425 easier than the rebase with amend.  I use it when a change applies to both the
426 current tip as well as the faulty commit.
427
428 You can even use git rebase to logically reorder commits so that together, the
429 'story' is more cohesive.
430
431 But wait, your version of vmmcp differs from origin/vmmcp!  The repo's vmmcp
432 branch still looks like the old one.  You need to push -f to force the repo's
433 branch to agree with yours:
434
435 $ git push -f origin vmmcp
436
437 This will destroy the original origin/vmmcp and replace it with your vmmcp.
438 Anyone who was tracking origin/vmmcp will need to reset their branch.  Do not
439 do this to origin/master.
440
441 Just remember: gitk, git rebase, and git commit --amend are your friends.
442
443
444 ### Crap, Someone Reset a Branch I was Tracking!
445
446 Say you were working on origin/vmmcp while someone else did the steps from the
447 previous section.  You do a git fetch and see that you differ!
448
449 If you don't have any changes from the original origin/vmmcp, then you can
450 simply:
451
452 $ git checkout vmmcp
453 $ git reset --hard origin/vmmcp
454
455 This throws away any differences between your tree and origin/vmmcp, in essence
456 making it replicate the current state of origin/vmmcp.  If you had any changes,
457 you just lost them.  Yikes!  If in doubt, you can make a temp branch before
458 doing anything dangerous with reset.
459
460 $ git branch temp               # keeps a pointer called 'temp' to the current tip
461
462 Say you do have changes.  Now what?  If they are just diffs, but not commits,
463 you can stash them, then replay the changes on the new vmmcp branch:
464
465 $ git stash
466 $ git reset --hard origin/vmmcp
467 $ git stash pop         # might not apply cleanly
468
469 If you did have commits, you'll want to rebase.  You are trying to rebase the
470 commits from your old vmmcp branch to the new one:
471
472 $ git rebase --onto origin/vmmcp PREVIOUS-ORIGIN-VMMCP
473
474 You'll need to look at gitk, git log, git gr, or whatever to find the commit
475 name (SHA1) of the original origin/vmmcp (before you fetched).  For this
476 reason, you could have a branch called vmmcp-mine for all of your changes, and
477 keep your branch vmmcp around to just track origin/vmmcp.  This is what many
478 people do with master: it only tracks origin/master, and all changes happen on
479 separate branches.
480
481 Note that you can probably just do a:
482
483 $ git rebase origin/vmmcp
484
485 and git will figure out and exclude which commits you have in common with
486 origin/vmmcp from the rebase.  Check your results in gitk or git gr.
487
488 This can be rather tricky, which is part of the reason why you shouldn't reset
489 branches that other people rely on (e.g. origin/master).  It's fine for small
490 group or individual work.
491
492 Just remember: gitk and git rebase are your friends.
493
494
495 ### What Happens When Two People Push -f
496
497 Whoever pushes most recently is the one to clobber the branch.  The previous
498 person's commits would only exist in that person's personal repo!  Yikes!
499
500 In general, don't clobber (push -f) other people's branches without
501 coordinating first.  I reset my origin branches all the time.  I won't usually
502 reset other people's branches without checking with them.
503
504 If clobbering repo branches is a problem, you can always create your own repo
505 branch; branches are very cheap and easy.
506
507 To delete a remote branch, say after it is merged into origin/master:
508
509 $ git push -f origin :BRANCH-NAME
510
511 The : is part of the "refspec"; whatever is on the left is the source (in this
512 case, nothing).  Check out git help push for more info.
513
514 Again, these techinques work well with small groups, but not for big branches
515 like origin/master.