Documentation / SubmittingPatcheson commit Merge branch 'nd/corrupt-worktrees' (000bce0)
   1Submitting Patches
   2==================
   3
   4== Guidelines
   5
   6Here are some guidelines for people who want to contribute their code
   7to this software.
   8
   9[[base-branch]]
  10=== Decide what to base your work on.
  11
  12In general, always base your work on the oldest branch that your
  13change is relevant to.
  14
  15* A bugfix should be based on `maint` in general. If the bug is not
  16  present in `maint`, base it on `master`. For a bug that's not yet
  17  in `master`, find the topic that introduces the regression, and
  18  base your work on the tip of the topic.
  19
  20* A new feature should be based on `master` in general. If the new
  21  feature depends on a topic that is in `pu`, but not in `master`,
  22  base your work on the tip of that topic.
  23
  24* Corrections and enhancements to a topic not yet in `master` should
  25  be based on the tip of that topic. If the topic has not been merged
  26  to `next`, it's alright to add a note to squash minor corrections
  27  into the series.
  28
  29* In the exceptional case that a new feature depends on several topics
  30  not in `master`, start working on `next` or `pu` privately and send
  31  out patches for discussion. Before the final merge, you may have to
  32  wait until some of the dependent topics graduate to `master`, and
  33  rebase your work.
  34
  35* Some parts of the system have dedicated maintainers with their own
  36  repositories (see the section "Subsystems" below).  Changes to
  37  these parts should be based on their trees.
  38
  39To find the tip of a topic branch, run `git log --first-parent
  40master..pu` and look for the merge commit. The second parent of this
  41commit is the tip of the topic branch.
  42
  43[[separate-commits]]
  44=== Make separate commits for logically separate changes.
  45
  46Unless your patch is really trivial, you should not be sending
  47out a patch that was generated between your working tree and
  48your commit head.  Instead, always make a commit with complete
  49commit message and generate a series of patches from your
  50repository.  It is a good discipline.
  51
  52Give an explanation for the change(s) that is detailed enough so
  53that people can judge if it is good thing to do, without reading
  54the actual patch text to determine how well the code does what
  55the explanation promises to do.
  56
  57If your description starts to get too long, that's a sign that you
  58probably need to split up your commit to finer grained pieces.
  59That being said, patches which plainly describe the things that
  60help reviewers check the patch, and future maintainers understand
  61the code, are the most beautiful patches.  Descriptions that summarize
  62the point in the subject well, and describe the motivation for the
  63change, the approach taken by the change, and if relevant how this
  64differs substantially from the prior version, are all good things
  65to have.
  66
  67Make sure that you have tests for the bug you are fixing.  See
  68`t/README` for guidance.
  69
  70[[tests]]
  71When adding a new feature, make sure that you have new tests to show
  72the feature triggers the new behavior when it should, and to show the
  73feature does not trigger when it shouldn't.  After any code change, make
  74sure that the entire test suite passes.
  75
  76If you have an account at GitHub (and you can get one for free to work
  77on open source projects), you can use their Travis CI integration to
  78test your changes on Linux, Mac (and hopefully soon Windows).  See
  79GitHub-Travis CI hints section for details.
  80
  81Do not forget to update the documentation to describe the updated
  82behavior and make sure that the resulting documentation set formats
  83well (try the Documentation/doc-diff script).
  84
  85We currently have a liberal mixture of US and UK English norms for
  86spelling and grammar, which is somewhat unfortunate.  A huge patch that
  87touches the files all over the place only to correct the inconsistency
  88is not welcome, though.  Potential clashes with other changes that can
  89result from such a patch are not worth it.  We prefer to gradually
  90reconcile the inconsistencies in favor of US English, with small and
  91easily digestible patches, as a side effect of doing some other real
  92work in the vicinity (e.g. rewriting a paragraph for clarity, while
  93turning en_UK spelling to en_US).  Obvious typographical fixes are much
  94more welcomed ("teh -> "the"), preferably submitted as independent
  95patches separate from other documentation changes.
  96
  97[[whitespace-check]]
  98Oh, another thing.  We are picky about whitespaces.  Make sure your
  99changes do not trigger errors with the sample pre-commit hook shipped
 100in `templates/hooks--pre-commit`.  To help ensure this does not happen,
 101run `git diff --check` on your changes before you commit.
 102
 103[[describe-changes]]
 104=== Describe your changes well.
 105
 106The first line of the commit message should be a short description (50
 107characters is the soft limit, see DISCUSSION in linkgit:git-commit[1]),
 108and should skip the full stop.  It is also conventional in most cases to
 109prefix the first line with "area: " where the area is a filename or
 110identifier for the general area of the code being modified, e.g.
 111
 112* doc: clarify distinction between sign-off and pgp-signing
 113* githooks.txt: improve the intro section
 114
 115If in doubt which identifier to use, run `git log --no-merges` on the
 116files you are modifying to see the current conventions.
 117
 118[[summary-section]]
 119It's customary to start the remainder of the first line after "area: "
 120with a lower-case letter. E.g. "doc: clarify...", not "doc:
 121Clarify...", or "githooks.txt: improve...", not "githooks.txt:
 122Improve...".
 123
 124[[meaningful-message]]
 125The body should provide a meaningful commit message, which:
 126
 127. explains the problem the change tries to solve, i.e. what is wrong
 128  with the current code without the change.
 129
 130. justifies the way the change solves the problem, i.e. why the
 131  result with the change is better.
 132
 133. alternate solutions considered but discarded, if any.
 134
 135[[imperative-mood]]
 136Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
 137instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
 138to do frotz", as if you are giving orders to the codebase to change
 139its behavior.  Try to make sure your explanation can be understood
 140without external resources. Instead of giving a URL to a mailing list
 141archive, summarize the relevant points of the discussion.
 142
 143[[commit-reference]]
 144If you want to reference a previous commit in the history of a stable
 145branch, use the format "abbreviated sha1 (subject, date)",
 146with the subject enclosed in a pair of double-quotes, like this:
 147
 148....
 149        Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30)
 150        noticed that ...
 151....
 152
 153The "Copy commit summary" command of gitk can be used to obtain this
 154format, or this invocation of `git show`:
 155
 156....
 157        git show -s --date=short --pretty='format:%h ("%s", %ad)' <commit>
 158....
 159
 160[[git-tools]]
 161=== Generate your patch using Git tools out of your commits.
 162
 163Git based diff tools generate unidiff which is the preferred format.
 164
 165You do not have to be afraid to use `-M` option to `git diff` or
 166`git format-patch`, if your patch involves file renames.  The
 167receiving end can handle them just fine.
 168
 169[[review-patch]]
 170Please make sure your patch does not add commented out debugging code,
 171or include any extra files which do not relate to what your patch
 172is trying to achieve. Make sure to review
 173your patch after generating it, to ensure accuracy.  Before
 174sending out, please make sure it cleanly applies to the `master`
 175branch head.  If you are preparing a work based on "next" branch,
 176that is fine, but please mark it as such.
 177
 178[[send-patches]]
 179=== Sending your patches.
 180
 181:security-ml: footnoteref:[security-ml,The Git Security mailing list: git-security@googlegroups.com]
 182
 183Before sending any patches, please note that patches that may be
 184security relevant should be submitted privately to the Git Security
 185mailing list{security-ml}, instead of the public mailing list.
 186
 187Learn to use format-patch and send-email if possible.  These commands
 188are optimized for the workflow of sending patches, avoiding many ways
 189your existing e-mail client that is optimized for "multipart/*" mime
 190type e-mails to corrupt and render your patches unusable.
 191
 192People on the Git mailing list need to be able to read and
 193comment on the changes you are submitting.  It is important for
 194a developer to be able to "quote" your changes, using standard
 195e-mail tools, so that they may comment on specific portions of
 196your code.  For this reason, each patch should be submitted
 197"inline" in a separate message.
 198
 199Multiple related patches should be grouped into their own e-mail
 200thread to help readers find all parts of the series.  To that end,
 201send them as replies to either an additional "cover letter" message
 202(see below), the first patch, or the respective preceding patch.
 203
 204If your log message (including your name on the
 205Signed-off-by line) is not writable in ASCII, make sure that
 206you send off a message in the correct encoding.
 207
 208WARNING: Be wary of your MUAs word-wrap
 209corrupting your patch.  Do not cut-n-paste your patch; you can
 210lose tabs that way if you are not careful.
 211
 212It is a common convention to prefix your subject line with
 213[PATCH].  This lets people easily distinguish patches from other
 214e-mail discussions.  Use of markers in addition to PATCH within
 215the brackets to describe the nature of the patch is also
 216encouraged.  E.g. [RFC PATCH] (where RFC stands for "request for
 217comments") is often used to indicate a patch needs further
 218discussion before being accepted, [PATCH v2], [PATCH v3] etc.
 219are often seen when you are sending an update to what you have
 220previously sent.
 221
 222The `git format-patch` command follows the best current practice to
 223format the body of an e-mail message.  At the beginning of the
 224patch should come your commit message, ending with the
 225Signed-off-by: lines, and a line that consists of three dashes,
 226followed by the diffstat information and the patch itself.  If
 227you are forwarding a patch from somebody else, optionally, at
 228the beginning of the e-mail message just before the commit
 229message starts, you can put a "From: " line to name that person.
 230To change the default "[PATCH]" in the subject to "[<text>]", use
 231`git format-patch --subject-prefix=<text>`.  As a shortcut, you
 232can use `--rfc` instead of `--subject-prefix="RFC PATCH"`, or
 233`-v <n>` instead of `--subject-prefix="PATCH v<n>"`.
 234
 235You often want to add additional explanation about the patch,
 236other than the commit message itself.  Place such "cover letter"
 237material between the three-dash line and the diffstat.  For
 238patches requiring multiple iterations of review and discussion,
 239an explanation of changes between each iteration can be kept in
 240Git-notes and inserted automatically following the three-dash
 241line via `git format-patch --notes`.
 242
 243[[attachment]]
 244Do not attach the patch as a MIME attachment, compressed or not.
 245Do not let your e-mail client send quoted-printable.  Do not let
 246your e-mail client send format=flowed which would destroy
 247whitespaces in your patches. Many
 248popular e-mail applications will not always transmit a MIME
 249attachment as plain text, making it impossible to comment on
 250your code.  A MIME attachment also takes a bit more time to
 251process.  This does not decrease the likelihood of your
 252MIME-attached change being accepted, but it makes it more likely
 253that it will be postponed.
 254
 255Exception:  If your mailer is mangling patches then someone may ask
 256you to re-send them using MIME, that is OK.
 257
 258[[pgp-signature]]
 259Do not PGP sign your patch. Most likely, your maintainer or other people on the
 260list would not have your PGP key and would not bother obtaining it anyway.
 261Your patch is not judged by who you are; a good patch from an unknown origin
 262has a far better chance of being accepted than a patch from a known, respected
 263origin that is done poorly or does incorrect things.
 264
 265If you really really really really want to do a PGP signed
 266patch, format it as "multipart/signed", not a text/plain message
 267that starts with `-----BEGIN PGP SIGNED MESSAGE-----`.  That is
 268not a text/plain, it's something else.
 269
 270:security-ml-ref: footnoteref:[security-ml]
 271
 272As mentioned at the beginning of the section, patches that may be
 273security relevant should not be submitted to the public mailing list
 274mentioned below, but should instead be sent privately to the Git
 275Security mailing list{security-ml-ref}.
 276
 277Send your patch with "To:" set to the mailing list, with "cc:" listing
 278people who are involved in the area you are touching (the `git
 279contacts` command in `contrib/contacts/` can help to
 280identify them), to solicit comments and reviews.
 281
 282:current-maintainer: footnote:[The current maintainer: gitster@pobox.com]
 283:git-ml: footnote:[The mailing list: git@vger.kernel.org]
 284
 285After the list reached a consensus that it is a good idea to apply the
 286patch, re-send it with "To:" set to the maintainer{current-maintainer} and "cc:" the
 287list{git-ml} for inclusion.
 288
 289Do not forget to add trailers such as `Acked-by:`, `Reviewed-by:` and
 290`Tested-by:` lines as necessary to credit people who helped your
 291patch.
 292
 293[[sign-off]]
 294=== Certify your work by adding your "Signed-off-by: " line
 295
 296To improve tracking of who did what, we've borrowed the
 297"sign-off" procedure from the Linux kernel project on patches
 298that are being emailed around.  Although core Git is a lot
 299smaller project it is a good discipline to follow it.
 300
 301The sign-off is a simple line at the end of the explanation for
 302the patch, which certifies that you wrote it or otherwise have
 303the right to pass it on as an open-source patch.  The rules are
 304pretty simple: if you can certify the below D-C-O:
 305
 306[[dco]]
 307.Developer's Certificate of Origin 1.1
 308____
 309By making a contribution to this project, I certify that:
 310
 311a. The contribution was created in whole or in part by me and I
 312   have the right to submit it under the open source license
 313   indicated in the file; or
 314
 315b. The contribution is based upon previous work that, to the best
 316   of my knowledge, is covered under an appropriate open source
 317   license and I have the right under that license to submit that
 318   work with modifications, whether created in whole or in part
 319   by me, under the same open source license (unless I am
 320   permitted to submit under a different license), as indicated
 321   in the file; or
 322
 323c. The contribution was provided directly to me by some other
 324   person who certified (a), (b) or (c) and I have not modified
 325   it.
 326
 327d. I understand and agree that this project and the contribution
 328   are public and that a record of the contribution (including all
 329   personal information I submit with it, including my sign-off) is
 330   maintained indefinitely and may be redistributed consistent with
 331   this project or the open source license(s) involved.
 332____
 333
 334then you just add a line saying
 335
 336....
 337        Signed-off-by: Random J Developer <random@developer.example.org>
 338....
 339
 340This line can be automatically added by Git if you run the git-commit
 341command with the -s option.
 342
 343Notice that you can place your own Signed-off-by: line when
 344forwarding somebody else's patch with the above rules for
 345D-C-O.  Indeed you are encouraged to do so.  Do not forget to
 346place an in-body "From: " line at the beginning to properly attribute
 347the change to its true author (see (2) above).
 348
 349[[real-name]]
 350Also notice that a real name is used in the Signed-off-by: line. Please
 351don't hide your real name.
 352
 353[[commit-trailers]]
 354If you like, you can put extra tags at the end:
 355
 356. `Reported-by:` is used to credit someone who found the bug that
 357  the patch attempts to fix.
 358. `Acked-by:` says that the person who is more familiar with the area
 359  the patch attempts to modify liked the patch.
 360. `Reviewed-by:`, unlike the other tags, can only be offered by the
 361  reviewer and means that she is completely satisfied that the patch
 362  is ready for application.  It is usually offered only after a
 363  detailed review.
 364. `Tested-by:` is used to indicate that the person applied the patch
 365  and found it to have the desired effect.
 366
 367You can also create your own tag or use one that's in common usage
 368such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
 369
 370== Subsystems with dedicated maintainers
 371
 372Some parts of the system have dedicated maintainers with their own
 373repositories.
 374
 375- `git-gui/` comes from git-gui project, maintained by Pat Thoyts:
 376
 377        git://repo.or.cz/git-gui.git
 378
 379- `gitk-git/` comes from Paul Mackerras's gitk project:
 380
 381        git://ozlabs.org/~paulus/gitk
 382
 383- `po/` comes from the localization coordinator, Jiang Xin:
 384
 385        https://github.com/git-l10n/git-po/
 386
 387Patches to these parts should be based on their trees.
 388
 389[[patch-flow]]
 390== An ideal patch flow
 391
 392Here is an ideal patch flow for this project the current maintainer
 393suggests to the contributors:
 394
 395. You come up with an itch.  You code it up.
 396
 397. Send it to the list and cc people who may need to know about
 398  the change.
 399+
 400The people who may need to know are the ones whose code you
 401are butchering.  These people happen to be the ones who are
 402most likely to be knowledgeable enough to help you, but
 403they have no obligation to help you (i.e. you ask for help,
 404don't demand).  +git log -p {litdd} _$area_you_are_modifying_+ would
 405help you find out who they are.
 406
 407. You get comments and suggestions for improvements.  You may
 408  even get them in an "on top of your change" patch form.
 409
 410. Polish, refine, and re-send to the list and the people who
 411  spend their time to improve your patch.  Go back to step (2).
 412
 413. The list forms consensus that the last round of your patch is
 414  good.  Send it to the maintainer and cc the list.
 415
 416. A topic branch is created with the patch and is merged to `next`,
 417  and cooked further and eventually graduates to `master`.
 418
 419In any time between the (2)-(3) cycle, the maintainer may pick it up
 420from the list and queue it to `pu`, in order to make it easier for
 421people play with it without having to pick up and apply the patch to
 422their trees themselves.
 423
 424[[patch-status]]
 425== Know the status of your patch after submission
 426
 427* You can use Git itself to find out when your patch is merged in
 428  master. `git pull --rebase` will automatically skip already-applied
 429  patches, and will let you know. This works only if you rebase on top
 430  of the branch in which your patch has been merged (i.e. it will not
 431  tell you if your patch is merged in pu if you rebase on top of
 432  master).
 433
 434* Read the Git mailing list, the maintainer regularly posts messages
 435  entitled "What's cooking in git.git" and "What's in git.git" giving
 436  the status of various proposed changes.
 437
 438[[travis]]
 439== GitHub-Travis CI hints
 440
 441With an account at GitHub (you can get one for free to work on open
 442source projects), you can use Travis CI to test your changes on Linux,
 443Mac (and hopefully soon Windows).  You can find a successful example
 444test build here: https://travis-ci.org/git/git/builds/120473209
 445
 446Follow these steps for the initial setup:
 447
 448. Fork https://github.com/git/git to your GitHub account.
 449  You can find detailed instructions how to fork here:
 450  https://help.github.com/articles/fork-a-repo/
 451
 452. Open the Travis CI website: https://travis-ci.org
 453
 454. Press the "Sign in with GitHub" button.
 455
 456. Grant Travis CI permissions to access your GitHub account.
 457  You can find more information about the required permissions here:
 458  https://docs.travis-ci.com/user/github-oauth-scopes
 459
 460. Open your Travis CI profile page: https://travis-ci.org/profile
 461
 462. Enable Travis CI builds for your Git fork.
 463
 464After the initial setup, Travis CI will run whenever you push new changes
 465to your fork of Git on GitHub.  You can monitor the test state of all your
 466branches here: https://travis-ci.org/__<Your GitHub handle>__/git/branches
 467
 468If a branch did not pass all test cases then it is marked with a red
 469cross.  In that case you can click on the failing Travis CI job and
 470scroll all the way down in the log.  Find the line "<-- Click here to see
 471detailed test output!" and click on the triangle next to the log line
 472number to expand the detailed test output.  Here is such a failing
 473example: https://travis-ci.org/git/git/jobs/122676187
 474
 475Fix the problem and push your fix to your Git fork.  This will trigger
 476a new Travis CI build to ensure all tests pass.
 477
 478[[mua]]
 479== MUA specific hints
 480
 481Some of patches I receive or pick up from the list share common
 482patterns of breakage.  Please make sure your MUA is set up
 483properly not to corrupt whitespaces.
 484
 485See the DISCUSSION section of linkgit:git-format-patch[1] for hints on
 486checking your patch by mailing it to yourself and applying with
 487linkgit:git-am[1].
 488
 489While you are at it, check the resulting commit log message from
 490a trial run of applying the patch.  If what is in the resulting
 491commit is not exactly what you would want to see, it is very
 492likely that your maintainer would end up hand editing the log
 493message when he applies your patch.  Things like "Hi, this is my
 494first patch.\n", if you really want to put in the patch e-mail,
 495should come after the three-dash line that signals the end of the
 496commit message.
 497
 498
 499=== Pine
 500
 501(Johannes Schindelin)
 502
 503....
 504I don't know how many people still use pine, but for those poor
 505souls it may be good to mention that the quell-flowed-text is
 506needed for recent versions.
 507
 508... the "no-strip-whitespace-before-send" option, too. AFAIK it
 509was introduced in 4.60.
 510....
 511
 512(Linus Torvalds)
 513
 514....
 515And 4.58 needs at least this.
 516
 517diff-tree 8326dd8350be64ac7fc805f6563a1d61ad10d32c (from e886a61f76edf5410573e92e38ce22974f9c40f1)
 518Author: Linus Torvalds <torvalds@g5.osdl.org>
 519Date:   Mon Aug 15 17:23:51 2005 -0700
 520
 521    Fix pine whitespace-corruption bug
 522
 523    There's no excuse for unconditionally removing whitespace from
 524    the pico buffers on close.
 525
 526diff --git a/pico/pico.c b/pico/pico.c
 527--- a/pico/pico.c
 528+++ b/pico/pico.c
 529@@ -219,7 +219,9 @@ PICO *pm;
 530            switch(pico_all_done){      /* prepare for/handle final events */
 531              case COMP_EXIT :          /* already confirmed */
 532                packheader();
 533+#if 0
 534                stripwhitespace();
 535+#endif
 536                c |= COMP_EXIT;
 537                break;
 538....
 539
 540(Daniel Barkalow)
 541
 542....
 543> A patch to SubmittingPatches, MUA specific help section for
 544> users of Pine 4.63 would be very much appreciated.
 545
 546Ah, it looks like a recent version changed the default behavior to do the
 547right thing, and inverted the sense of the configuration option. (Either
 548that or Gentoo did it.) So you need to set the
 549"no-strip-whitespace-before-send" option, unless the option you have is
 550"strip-whitespace-before-send", in which case you should avoid checking
 551it.
 552....
 553
 554=== Thunderbird, KMail, GMail
 555
 556See the MUA-SPECIFIC HINTS section of linkgit:git-format-patch[1].
 557
 558=== Gnus
 559
 560"|" in the `*Summary*` buffer can be used to pipe the current
 561message to an external program, and this is a handy way to drive
 562`git am`.  However, if the message is MIME encoded, what is
 563piped into the program is the representation you see in your
 564`*Article*` buffer after unwrapping MIME.  This is often not what
 565you would want for two reasons.  It tends to screw up non ASCII
 566characters (most notably in people's names), and also
 567whitespaces (fatal in patches).  Running "C-u g" to display the
 568message in raw form before using "|" to run the pipe can work
 569this problem around.