Skip to content

Instantly share code, notes, and snippets.

@brandon1024
Last active March 3, 2019 20:05
Show Gist options
  • Save brandon1024/41500eb05ed9ffa63efa8f81eb51a081 to your computer and use it in GitHub Desktop.
Save brandon1024/41500eb05ed9ffa63efa8f81eb51a081 to your computer and use it in GitHub Desktop.
Workflow for Contributing to Git Core

Workflow for Contributing to Git Core

Read up!

Before you start contributing to git, you'll need to have a good understanding of a couple of important topics. I've listed some resources below that I found useful.

Join the mailing list

I found this very helpful, because I could go see how people format their emails and patches. Here is how to join.

Getting a copy of the sources

Git hosts a source code mirror on GitHub, which is the easiest way to get a copy of the code. Just clone it from there. You can also clone or browse the repository from Kernel.org git repositories.

Setting up your environment

I personally use CLion for an IDE, but you can use whatever you'd like. Be sure to configure your IDE to use tabs instead of spaces.

If you're using CLion, you'll often find yourself stepping over the .idea directory. To get around this, you can edit .git/info/exclude and add the following line. This is similar to adding to the project .gitignore, but only applies to your local environment.

.idea

Getting started

First, you should (almost) always base your changes off of the maint branch. Read Submitting Patches for more details on this.

Checkout a branch and start making your changes.

Formatting your commits

Once you have a chunk of work ready, it's time to commit. Heres how you do it:

git add <your changes>
git commit --no-gpg-sign --signoff

Then your commit message should look something like this example:

commit-tree: add missing --gpg-sign flag

Add --gpg-sign option in commit-tree, which was documented, but not
implemented, in 55ca3f99ae. Add tests for the --gpg-sign option.

Signed-off-by: Brandon Richardson <[email protected]>

The first line of the commit message should be short and succinct, as it will become the email subject title. Here's where it becomes useful to look at other emails on the mailinglist to get some inspiration. Also note that you should NOT gpg sign your commits.

Creating notes

This step is not mandatory, but useful if you need to attach more information to your patch that will help reviewers understand your changes (information that wouldn't go directly in your commit message).

Notes are created using git notes add, and will default to reference the most recent commit (HEAD).

Once you create your note, you will embed it into your patch. See Creating patches for details.

Generating patches

To generate email-friendly patches, use git format-patch. Here is an example:

git format-patch -1 --subject-prefix="PATCH" -o <output dir>

The above command will create a single (-1) patch from the lastest commit on the current branch. The --subject-prefix=[<subject>] is optional here, because it will be "PATCH" by default.

If your patch should consist of more than one commit, use -<n> rather than -1.

If your patch has already been reviewed and you addressed some reviewer feedback, the subject of your next iteration should be "PATCH v2".

To attach a note:

git format-patch -1 --notes

See Creating Notes above.

Sending patches

Patches are sent to the mailinglist through the git send-email tool. These steps will vary depending on what email service you use, so you should read the git-send-email documentation. Here are the steps to send your patches using your gmail account.

First you'll need to update your .gitconfig:

[sendemail]
	smtpEncryption = tls
	smtpServer = smtp.gmail.com
	smtpUser = <email>@gmail.com
	smtpServerPort = 587

Then, you'll need to configure your Google account to allow Less secure app access.

Next, send the email. Don't worry, you'll be prompted to accept before the email is actually sent. If you're paranoid (like me), use the --dry-run option.

git send-email changes.patch

If this is a second iteration of your changes, and you used --subject="PATCH v2", you should also send the email to your reviewers. For example:

git send-email [email protected] [email protected],[email protected] changes.patch

If you have multiple commits, you should send your patches as a series. This is easily done by using a pattern, rather than a specific patch:

git send-email /Users/brandon/dev/patches/git/bugfix/v2/000*

TL;DR

To send a (v1) patch:

<make changes>
git add <changes>
git commit --no-gpg-sign --signoff
git format-patch -1
git send-email [email protected] <patch file>

To send a (v2) patch with notes:

<make changes>
git add <changes>
git commit --no-gpg-sign --signoff
git notes add
git format-patch -1 --subject-prefix="PATCH v2" --notes
git send-email [email protected] [(--cc=<reviewer email>)...] <patch file>

To send a (v3) patch series with notes:

<make changes>
git add <changes>
git commit --no-gpg-sign --signoff
git notes add

<make changes>
git add <changes>
git commit --no-gpg-sign --signoff
git notes add

git format-patch -2 --subject-prefix="PATCH v3" --notes
git send-email [email protected] [(--cc=<reviewer email>)...] <patch file>
From 2eff9251b8e0ab7e2443d964fcf1dc15e1e79c91 Mon Sep 17 00:00:00 2001
From: Brandon Richardson <[email protected]>
Date: Sat, 2 Mar 2019 06:31:17 -0500
Subject: [PATCH v3] commit-tree: utilize parse-options api
Rather than parse options manually, which is both difficult to
read and error prone, parse options supplied to commit-tree
using the parse-options api.
It was discovered that the --no-gpg-sign option was documented
but not implemented in commit 70ddbd7767 (commit-tree: add missing
--gpg-sign flag, 2019-01-19), and the existing implementation
would attempt to translate the option as a tree oid. It was also
suggested earlier in commit 55ca3f99ae (commit-tree: add and document
--no-gpg-sign, 2013-12-13) that commit-tree should be migrated to
utilize the parse-options api, which could help prevent mistakes
like this in the future. Hence this change.
Signed-off-by: Brandon Richardson <[email protected]>
---
Notes:
GitHub Pull Request: https://github.com/brandon1024/git/pull/3
Travis CI Build: https://travis-ci.com/brandon1024/git/builds/102953064
Documentation/git-commit-tree.txt | 8 +-
builtin/commit-tree.c | 159 ++++++++++++++++--------------
parse-options.h | 11 +++
3 files changed, 104 insertions(+), 74 deletions(-)
diff --git a/Documentation/git-commit-tree.txt b/Documentation/git-commit-tree.txt
index 002dae625e..f4e20b62a0 100644
--- a/Documentation/git-commit-tree.txt
+++ b/Documentation/git-commit-tree.txt
@@ -23,6 +23,9 @@ Creates a new commit object based on the provided tree object and
emits the new commit object id on stdout. The log message is read
from the standard input, unless `-m` or `-F` options are given.
+When mixing `-m` and `-F` options, the commit log message will be
+composed in the order in which the options are given.
+
A commit object may have any number of parents. With exactly one
parent, it is an ordinary commit. Having more than one parent makes
the commit a merge between several lines of history. Initial (root)
@@ -41,7 +44,7 @@ state was.
OPTIONS
-------
<tree>::
- An existing tree object
+ An existing tree object.
-p <parent>::
Each `-p` indicates the id of a parent commit object.
@@ -52,7 +55,8 @@ OPTIONS
-F <file>::
Read the commit log message from the given file. Use `-` to read
- from the standard input.
+ from the standard input. This can be given more than once and the
+ content of each file becomes its own paragraph.
-S[<keyid>]::
--gpg-sign[=<keyid>]::
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 12cc403bd7..d4a911acf5 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -12,8 +12,14 @@
#include "builtin.h"
#include "utf8.h"
#include "gpg-interface.h"
+#include "parse-options.h"
+#include "string-list.h"
-static const char commit_tree_usage[] = "git commit-tree [(-p <sha1>)...] [-S[<keyid>]] [-m <message>] [-F <file>] <sha1>";
+static const char * const commit_tree_usage[] = {
+ N_("git commit-tree [(-p <parent>)...] [-S[<keyid>]] [(-m <message>)...] "
+ "[(-F <file>)...] <tree>"),
+ NULL
+};
static const char *sign_commit;
@@ -23,7 +29,7 @@ static void new_parent(struct commit *parent, struct commit_list **parents_p)
struct commit_list *parents;
for (parents = *parents_p; parents; parents = parents->next) {
if (parents->item == parent) {
- error("duplicate parent %s ignored", oid_to_hex(oid));
+ error(_("duplicate parent %s ignored"), oid_to_hex(oid));
return;
}
parents_p = &parents->next;
@@ -39,91 +45,100 @@ static int commit_tree_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
+static int parse_parent_arg_callback(const struct option *opt,
+ const char *arg, int unset)
+{
+ struct object_id oid;
+ struct commit_list **parents = opt->value;
+
+ BUG_ON_OPT_NEG_NOARG(unset, arg);
+
+ if (get_oid_commit(arg, &oid))
+ die(_("not a valid object name %s"), arg);
+
+ assert_oid_type(&oid, OBJ_COMMIT);
+ new_parent(lookup_commit(the_repository, &oid), parents);
+ return 0;
+}
+
+static int parse_message_arg_callback(const struct option *opt,
+ const char *arg, int unset)
+{
+ struct strbuf *buf = opt->value;
+
+ BUG_ON_OPT_NEG_NOARG(unset, arg);
+
+ if (buf->len)
+ strbuf_addch(buf, '\n');
+ strbuf_addstr(buf, arg);
+ strbuf_complete_line(buf);
+
+ return 0;
+}
+
+static int parse_file_arg_callback(const struct option *opt,
+ const char *arg, int unset)
+{
+ int fd;
+ struct strbuf *buf = opt->value;
+
+ BUG_ON_OPT_NEG_NOARG(unset, arg);
+
+ if (buf->len)
+ strbuf_addch(buf, '\n');
+ if (!strcmp(arg, "-"))
+ fd = 0;
+ else {
+ fd = open(arg, O_RDONLY);
+ if (fd < 0)
+ die_errno(_("git commit-tree: failed to open '%s'"), arg);
+ }
+ if (strbuf_read(buf, fd, 0) < 0)
+ die_errno(_("git commit-tree: failed to read '%s'"), arg);
+ if (fd && close(fd))
+ die_errno(_("git commit-tree: failed to close '%s'"), arg);
+
+ return 0;
+}
+
int cmd_commit_tree(int argc, const char **argv, const char *prefix)
{
- int i, got_tree = 0;
+ static struct strbuf buffer = STRBUF_INIT;
struct commit_list *parents = NULL;
struct object_id tree_oid;
struct object_id commit_oid;
- struct strbuf buffer = STRBUF_INIT;
+
+ struct option options[] = {
+ { OPTION_CALLBACK, 'p', NULL, &parents, N_("parent"),
+ N_("id of a parent commit object"), PARSE_OPT_NONEG,
+ parse_parent_arg_callback },
+ { OPTION_CALLBACK, 'm', NULL, &buffer, N_("message"),
+ N_("commit message"), PARSE_OPT_NONEG,
+ parse_message_arg_callback },
+ { OPTION_CALLBACK, 'F', NULL, &buffer, N_("file"),
+ N_("read commit log message from file"), PARSE_OPT_NONEG,
+ parse_file_arg_callback },
+ { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
+ N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
+ OPT_END()
+ };
git_config(commit_tree_config, NULL);
if (argc < 2 || !strcmp(argv[1], "-h"))
- usage(commit_tree_usage);
-
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
- if (!strcmp(arg, "-p")) {
- struct object_id oid;
- if (argc <= ++i)
- usage(commit_tree_usage);
- if (get_oid_commit(argv[i], &oid))
- die("Not a valid object name %s", argv[i]);
- assert_oid_type(&oid, OBJ_COMMIT);
- new_parent(lookup_commit(the_repository, &oid),
- &parents);
- continue;
- }
+ usage_with_options(commit_tree_usage, options);
- if (!strcmp(arg, "--gpg-sign")) {
- sign_commit = "";
- continue;
- }
+ argc = parse_options(argc, argv, prefix, options, commit_tree_usage, 0);
- if (skip_prefix(arg, "-S", &sign_commit) ||
- skip_prefix(arg, "--gpg-sign=", &sign_commit))
- continue;
+ if (argc != 1)
+ die(_("must give exactly one tree"));
- if (!strcmp(arg, "--no-gpg-sign")) {
- sign_commit = NULL;
- continue;
- }
-
- if (!strcmp(arg, "-m")) {
- if (argc <= ++i)
- usage(commit_tree_usage);
- if (buffer.len)
- strbuf_addch(&buffer, '\n');
- strbuf_addstr(&buffer, argv[i]);
- strbuf_complete_line(&buffer);
- continue;
- }
-
- if (!strcmp(arg, "-F")) {
- int fd;
-
- if (argc <= ++i)
- usage(commit_tree_usage);
- if (buffer.len)
- strbuf_addch(&buffer, '\n');
- if (!strcmp(argv[i], "-"))
- fd = 0;
- else {
- fd = open(argv[i], O_RDONLY);
- if (fd < 0)
- die_errno("git commit-tree: failed to open '%s'",
- argv[i]);
- }
- if (strbuf_read(&buffer, fd, 0) < 0)
- die_errno("git commit-tree: failed to read '%s'",
- argv[i]);
- if (fd && close(fd))
- die_errno("git commit-tree: failed to close '%s'",
- argv[i]);
- continue;
- }
-
- if (get_oid_tree(arg, &tree_oid))
- die("Not a valid object name %s", arg);
- if (got_tree)
- die("Cannot give more than one trees");
- got_tree = 1;
- }
+ if (get_oid_tree(argv[0], &tree_oid))
+ die(_("not a valid object name %s"), argv[0]);
if (!buffer.len) {
if (strbuf_read(&buffer, 0, 0) < 0)
- die_errno("git commit-tree: failed to read");
+ die_errno(_("git commit-tree: failed to read"));
}
if (commit_tree(buffer.buf, buffer.len, &tree_oid, parents, &commit_oid,
diff --git a/parse-options.h b/parse-options.h
index 14fe32428e..3a442eee26 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -202,6 +202,17 @@ const char *optname(const struct option *opt, int flags);
BUG("option callback does not expect an argument"); \
} while (0)
+/*
+ * Similar to the assertions above, but checks that "arg" is always non-NULL.
+ * This assertion also implies BUG_ON_OPT_NEG(), letting you declare both
+ * assertions in a single line.
+ */
+#define BUG_ON_OPT_NEG_NOARG(unset, arg) do { \
+ BUG_ON_OPT_NEG(unset); \
+ if(!(arg)) \
+ BUG("option callback expects an argument"); \
+} while(0)
+
/*----- incremental advanced APIs -----*/
enum {
--
2.21.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment