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