git: 965da1835e - main - Move the MFC doc from my freebsd-git-docs repo to handbook
Warner Losh
imp at FreeBSD.org
Wed Mar 17 02:14:05 UTC 2021
The branch main has been updated by imp:
URL: https://cgit.FreeBSD.org/doc/commit/?id=965da1835ef371c71e528b211406b05aa16452d8
commit 965da1835ef371c71e528b211406b05aa16452d8
Author: Warner Losh <imp at FreeBSD.org>
AuthorDate: 2021-03-17 02:09:36 +0000
Commit: Warner Losh <imp at FreeBSD.org>
CommitDate: 2021-03-17 02:09:36 +0000
Move the MFC doc from my freebsd-git-docs repo to handbook
Integreate MFC procedures into the committer's handbook. This was originally in
my freebsd-git-docs repo where it was collaberatively created. The changes here
are light edits and a conversion from markdown to asciidoc.
Contributions from: jhb@, lwhsu@, delphij@, Ceri Davies, Alexandre
---
.../en/articles/committers-guide/_index.adoc | 494 +++++++++++++++++++++
1 file changed, 494 insertions(+)
diff --git a/documentation/content/en/articles/committers-guide/_index.adoc b/documentation/content/en/articles/committers-guide/_index.adoc
index 10ac725642..5dda2f0f91 100644
--- a/documentation/content/en/articles/committers-guide/_index.adoc
+++ b/documentation/content/en/articles/committers-guide/_index.adoc
@@ -241,6 +241,500 @@ Committers are encouraged to seek review for their work as part of the normal de
this section is a work in progress...
====
+[[mfc-with-git]]
+=== MFC (Merge From Current) Procedures
+==== Summary
+
+MFC workflow can be summarized as `git cherry-pick -x` plus git commit
+--amend to adjust the commit message. For multiple commits, use `git rebase -i`
+to squash them together and edit the commit message.
+
+==== Single commit MFC
+
+[source,shell]
+....
+% git checkout stable/X
+% git cherry-pick -x $HASH --edit
+....
+
+For MFC commits, for example a vendor import, you would need to specify one parent for cherry-pick
+purposes. Normally, that would be the "first parent" of the branch you are cherry-picking from, so:
+
+[source,shell]
+....
+% git checkout stable/X
+% git cherry-pick -x $HASH -m 1 --edit
+....
+
+If things go wrong, you'll either need to abort the cherry-pick with `git cherry-pick --abort` or fix it
+up and do a `git cherry-pick --continue`.
+
+Once the cherry-pick is finished, push with `git push`. If you get an error due to losing the commit race,
+use `git pull --rebase` and try to push again.
+
+==== Multiple commit MFC
+
+[source,shell]
+....
+% git checkout -b tmp-branch stable/X
+% for h in $HASH_LIST; do git cherry-pick -x $h; done
+% git rebase -i stable/X
+# mark each of the commits after the first as 'squash'
+# edit the commit message to be sane
+% git push freebsd HEAD:stable/X
+....
+
+If the push fails due to losing the commit race, rebase and try again:
+
+[source,shell]
+....
+% git checkout stable/X
+% git pull
+% git checkout tmp-branch
+% git rebase stable/X
+% git push freebsd HEAD:stable/X
+....
+
+Once the MFC is complete, you can delete the temporary branch:
+
+[source,shell]
+....
+% git checkout stable/X
+% git branch -d tmp-branch
+....
+
+==== MFC a vendor import
+
+Vendor imports are the only thing in the tree that creates a merge
+commit in the main line. Cherry picking merge commits into stable/XX
+presents an additional difficulty because there are two parents for a
+merge commit. Generally, you'll want the first parent's diff since
+that's the diff to mainline (though there may be some exceptions).
+
+[source,shell]
+....
+% git cherry-pick -x -m 1 $HASH
+....
+is typically what you want. This will tell cherry-pick to apply the correct diff.
+
+There are some, hopefully, rare cases where it's possible that the
+mainline was merged backwards by the conversion script. Should that be
+the case (and we've not found any yet), you'd change the above to '-m 2'
+to pickup the proper parent. Just do
+[source,shell]
+....
+% git cherry-pick --abort
+% git cherry-pick -x -m 2 $HASH
+....
+to do that. The `--aboort` will cleanup the failed first attempt.
+
+==== Redoing a MFC
+
+If you do a MFC, and it goes horribly wrong and you want to start over,
+then the easiest way is to use `git reset --hard` like so:
+[source,shell]
+....
+% git reset --hard freebsd/stable/12
+....
+though if you have some revs you want to keep, and others you don't,
+using 'git rebase -i' is better.
+
+==== Considerations when MFCing
+
+When committing source commits to stable and releng branches, we have
+the following goals:
+
+1. Clearly mark direct commits distinct from commits that land a
+ change from another branch
+2. Avoid introducing known breakage into stable and releng branches
+3. Allow developers to determine which changes have or have not been
+ landed from one branch to another
+
+With subversion, we used the following practices to achieve these goals:
+
+1. Using 'MFC' and 'MFS' tags to mark commits that merged changes from
+ another branch
+2. Squashing fixup commits into the main commit when merging a change
+3. Recording mergeinfo so that `svn mergeinfo --show-revs` worked
+
+With Git, we will need to use different strategies to achieve the same
+goals. This document aims to define best practices when merging
+source commits using git that achieve these goals. In general, we aim
+to use git's native support to achieve these goals rather than
+enforcing practices built on subversion's model.
+
+One general note: due to technical differences with Git, we will not
+be using git "merge commits" (created via `git merge`) in stable or
+releng branches. Instead, when this document refers to "merge
+commits", it means a commit originally made to `main` that is
+replicated or "landed" to a stable branch, or a commit from a stable
+branch that is replicated to a releng branch with some varation of
+`git cherry-pick`.
+
+==== Commit message standards
+===== Marking MFCs
+
+There are two main options for marking MFCs as distinct from direct
+commits:
+
+1. One option that matches our existing practice (the wisdom of which
+ I'm not commenting on) would mark MFCs like this in the commit
+ message: `MFC: 12def6789a3a,ac32ee4a5c2e`
+
+ where the first 12 digits of the hash is used to mark the commit message.
+ This "abbreviated hash" can be retrieved by: `git show --format=%p --no-patch $full_hash`
+
+ This preserves the information, but isn't 'git standard'. It also
+ requires committers to manually edit commit messages to include
+ this information when merging.
+
+2. Use the `-x` flag with `git cherry-pick`. This adds a line to the
+ commit message that includes the hash of the original commit when
+ merging. Since it is added by Git directly, committers do not have
+ to manually edit the commit log when merging.
+
+We feel that the second option is simpler going forward.
+
+===== Finding Eligible Hashes to MFC
+
+Git provides some built-in support for this via the `git cherry` and
+`git log --cherry` commands. These commands compare the raw diffs of
+commits (but not other metadata such as log messages) to determine if
+two commits are identical. This works well when each commit from head
+is landed as a single commit to a stable branch, but it falls over if
+multiple commits from main are squashed together as a single commit to
+a stable branch.
+
+There are a few options for resolving this:
+
+1. We could ban squashing of commits and instead require that committers
+ stage all of the fixup / follow-up commits to stable into a single
+ push. This would still achieve the goal of stability in stable and
+ releng branches since pushes are atomic and users doing a simple pull
+ will never end up with a tree that has the main commit without the
+ fixup(s). `git bisect` is also able to cope with this model via
+ `git bisect skip`.
+
+2. We could adopt a consistent style for describing MFCs and write
+ our own tooling to wrap around `git cherry` to determine the list
+ of eligible commits. A simple approach here might be to use the
+ syntax from `git cherry-pick -x`, but require that a squashed
+ commit list all of the hashes (one line per hash) at the end of
+ the commit message. Developers could do this by using
+ `git cherry-pick -x` of each individual commit into a branch and
+ then use `git rebase` to squash the commits down into a single
+ commit, but collecting the `-x` annotations at the end of the
+ landed commit log.
+
+===== Trim Metadata?
+
+One area that was not clearly documented with subversion (or even CVS)
+is how to format metadata in log messages for MFC commits. Should
+it include the metadata from the original commit unchanged, or should
+it be altered to reflect information about the MFC commit itself?
+
+Historical practice has varied, though some of the variance is by
+field. For example, MFCs that are relevant to a PR generally
+include the PR field in the MFC so that MFC commits are included
+in the bug tracker's audit trail. Other fields are less clear. For
+example, Phabricator shows the diff of the last commit tagged to a
+review, so including Phabricator URLs replaces the `main` commit with
+the landed commits. The list of reviewers is also not clear. If a
+reviewer has approved a change to `main`, does that mean they have
+approved the MFC commit? Is that true if it's identical code only,
+or with merely trivial reworkes? It's clearly not true for more
+extensive reworks. Even for identical code what if the commit doesn't
+conflict but introduces an ABI change? A reviewer may have ok'd a
+commit for `main` due to the ABI breakage but may not approve of
+merging the same commit as-is. One will have to use one's best
+judgement until clear guidelines can be agreed upon.
+
+For MFCs regulated by re@, new metadata fields are added, such as
+the Approved by tag for approved commits. This new metadata will have
+to be added via `git commit --amend` or similar after the original
+commit has been reviewed and approved. We may also want to reserve
+some metadata fields in MFC commits such as Phabricator URLs for use
+by re@ in the future.
+
+Preserving existing metadata provides a very simple workflow.
+Developers can just use `git cherry-pick -x` without having to edit
+the log message.
+
+If instead we choose to adjust metadata in MFCs, developers will
+have to edit log messages explicitly via the use of `git cherry-pick
+--edit` or `git commit --amend`. However, as compared to svn, at
+least the existing commit message can be pre-populated and metadata
+fields can be added or removed without having to re-enter the entire
+commit message.
+
+The bottom line is that developers will likely need to curate their
+commit message for MFCs that are non-trivial.
+
+==== Examples
+
+===== Merging a Single Subversion Commit
+
+This walks through the process of merging a commit to stable/12 that
+was originally committed to head in Subversion. In this case, the
+original commit is r368685.
+
+The first step is to map the Subversion commit to a Git hash. Once
+you have fetched refs/notes/commits, you can pass the revision number
+to `git log --grep`:
+
+[source,shell]
+....
+% git log main --grep 368685
+commit ce8395ecfda2c8e332a2adf9a9432c2e7f35ea81
+Author: John Baldwin <jhb at FreeBSD.org>
+Date: Wed Dec 16 00:11:30 2020 +0000
+
+ Use the 't' modifier to print a ptrdiff_t.
+
+ Reviewed by: imp
+ Obtained from: CheriBSD
+ Sponsored by: DARPA
+ Differential Revision: https://reviews.freebsd.org/D27576
+
+Notes:
+ svn path=/head/; revision=368685
+....
+
+Next, MFC the commit to a `stable/12` checkout:
+
+[source,shell]
+....
+git checkout stable/12
+git cherry-pick -x ce8395ecfda2c8e332a2adf9a9432c2e7f35ea81 --edit
+....
+
+Git will invoke the editor. Use this to remove the metadata that only
+applied to the original commit (Phabricator URL and Reviewed by).
+After the editor saves the updated log message, Git completes the
+commit:
+
+[source,shell]
+....
+[stable/12 3e3a548c4874] Use the 't' modifier to print a ptrdiff_t.
+ Date: Wed Dec 16 00:11:30 2020 +0000
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+....
+
+The contents of the MFCd commit can be examined via `git show`:
+
+[source,shell]
+....
+% git show
+commit 3e3a548c487450825679e4bd63d8d1a67fd8bd2d (HEAD -> stable/12)
+Author: John Baldwin <jhb at FreeBSD.org>
+Date: Wed Dec 16 00:11:30 2020 +0000
+
+ Use the 't' modifier to print a ptrdiff_t.
+
+ Obtained from: CheriBSD
+ Sponsored by: DARPA
+
+ (cherry picked from commit ce8395ecfda2c8e332a2adf9a9432c2e7f35ea81)
+
+diff --git a/sys/compat/linuxkpi/common/include/linux/printk.h b/sys/compat/linuxkpi/common/include/linux/printk.h
+index 31802bdd2c99..e6510e9e9834 100644
+--- a/sys/compat/linuxkpi/common/include/linux/printk.h
++++ b/sys/compat/linuxkpi/common/include/linux/printk.h
+@@ -68,7 +68,7 @@ print_hex_dump(const char *level, const char *prefix_str,
+ printf("[%p] ", buf);
+ break;
+ case DUMP_PREFIX_OFFSET:
+- printf("[%p] ", (const char *)((const char *)buf -
++ printf("[%#tx] ", ((const char *)buf -
+ (const char *)buf_old));
+ break;
+ default:
+....
+
+The MFC commit can now be published via `git push`
+
+[source,shell]
+....
+% git push freebsd
+Enumerating objects: 17, done.
+Counting objects: 100% (17/17), done.
+Delta compression using up to 4 threads
+Compressing objects: 100% (7/7), done.
+Writing objects: 100% (9/9), 817 bytes | 204.00 KiB/s, done.
+Total 9 (delta 5), reused 1 (delta 1), pack-reused 0
+To gitrepo-dev.FreeBSD.org:src.git
+ 525bd9c9dda7..3e3a548c4874 stable/12 -> stable/12
+....
+
+===== Merging a Single Subversion Commit with a Conflict
+
+This example is similar to the previous example except that the
+commit in question encounters a merge conflict. In this case, the
+original commit is r368314.
+
+As above, the first step is to map the Subversion commit to a Git
+hash:
+
+[source,shell]
+....
+% git log main --grep 368314
+commit 99963f5343a017e934e4d8ea2371a86789a46ff9
+Author: John Baldwin <jhb at FreeBSD.org>
+Date: Thu Dec 3 22:01:13 2020 +0000
+
+ Don't transmit mbufs that aren't yet ready on TOE sockets.
+
+ This includes mbufs waiting for data from sendfile() I/O requests, or
+ mbufs awaiting encryption for KTLS.
+
+ Reviewed by: np
+ MFC after: 2 weeks
+ Sponsored by: Chelsio Communications
+ Differential Revision: https://reviews.freebsd.org/D27469
+
+Notes:
+ svn path=/head/; revision=368314
+....
+
+Next, MFC the commit to a `stable/12` checkout:
+
+[source,shell]
+....
+% git checkout stable/12
+% git cherry-pick -x 99963f5343a017e934e4d8ea2371a86789a46ff9 --edit
+Auto-merging sys/dev/cxgbe/tom/t4_cpl_io.c
+CONFLICT (content): Merge conflict in sys/dev/cxgbe/tom/t4_cpl_io.c
+warning: inexact rename detection was skipped due to too many files.
+warning: you may want to set your merge.renamelimit variable to at least 7123 and retry the command.
+error: could not apply 99963f5343a0... Don't transmit mbufs that aren't yet ready on TOE sockets.
+hint: after resolving the conflicts, mark the corrected paths
+hint: with 'git add <paths>' or 'git rm <paths>'
+hint: and commit the result with 'git commit'
+....
+
+In this case, the commit encountered a merge conflict in
+sys/dev/cxge/tom/t4_cpl_io.c as kernel TLS is not present in
+stable/12. Note that Git does not invoke an editor to adjust the
+commit message due to the conflict. `git status` confirms that this
+file has merge conflicts:
+
+[source,shell]
+....
+% git status
+On branch stable/12
+Your branch is up to date with 'upstream/stable/12'.
+
+You are currently cherry-picking commit 99963f5343a0.
+ (fix conflicts and run "git cherry-pick --continue")
+ (use "git cherry-pick --skip" to skip this patch)
+ (use "git cherry-pick --abort" to cancel the cherry-pick operation)
+
+Unmerged paths:
+ (use "git add <file>..." to mark resolution)
+ both modified: sys/dev/cxgbe/tom/t4_cpl_io.c
+
+no changes added to commit (use "git add" and/or "git commit -a")
+....
+
+After editing the file to resolve the conflict, `git status` shows the
+conflict as resolved:
+
+[source,shell]
+....
+% git status
+On branch stable/12
+Your branch is up to date with 'upstream/stable/12'.
+
+You are currently cherry-picking commit 99963f5343a0.
+ (all conflicts fixed: run "git cherry-pick --continue")
+ (use "git cherry-pick --skip" to skip this patch)
+ (use "git cherry-pick --abort" to cancel the cherry-pick operation)
+
+Changes to be committed:
+ modified: sys/dev/cxgbe/tom/t4_cpl_io.c
+....
+
+The cherry-pick can now be completed:
+
+[source,shell]
+....
+% git cherry-pick --continue
+....
+
+Since there was a merge conflict, Git invokes the editor to
+adjust the commit message. Trim the metadata fields from the
+commit log from the original commit to head and save the
+updated log message.
+
+The contents of the MFC commit can be examined via `git show`:
+
+[source,shell]
+....
+% git show
+commit 525bd9c9dda7e7c7efad2d4570c7fd8e1a8ffabc (HEAD -> stable/12)
+Author: John Baldwin <jhb at FreeBSD.org>
+Date: Thu Dec 3 22:01:13 2020 +0000
+
+ Don't transmit mbufs that aren't yet ready on TOE sockets.
+
+ This includes mbufs waiting for data from sendfile() I/O requests, or
+ mbufs awaiting encryption for KTLS.
+
+ Sponsored by: Chelsio Communications
+
+ (cherry picked from commit 99963f5343a017e934e4d8ea2371a86789a46ff9)
+
+diff --git a/sys/dev/cxgbe/tom/t4_cpl_io.c b/sys/dev/cxgbe/tom/t4_cpl_io.c
+index 8e8c2b8639e6..43861f10b689 100644
+--- a/sys/dev/cxgbe/tom/t4_cpl_io.c
++++ b/sys/dev/cxgbe/tom/t4_cpl_io.c
+@@ -746,6 +746,8 @@ t4_push_frames(struct adapter *sc, struct toepcb *toep, int drop)
+ for (m = sndptr; m != NULL; m = m->m_next) {
+ int n;
+
++ if ((m->m_flags & M_NOTAVAIL) != 0)
++ break;
+ if (IS_AIOTX_MBUF(m))
+ n = sglist_count_vmpages(aiotx_mbuf_pages(m),
+ aiotx_mbuf_pgoff(m), m->m_len);
+@@ -821,8 +823,9 @@ t4_push_frames(struct adapter *sc, struct toepcb *toep, int drop)
+
+ /* nothing to send */
+ if (plen == 0) {
+- KASSERT(m == NULL,
+- ("%s: nothing to send, but m != NULL", __func__));
++ KASSERT(m == NULL || (m->m_flags & M_NOTAVAIL) != 0,
++ ("%s: nothing to send, but m != NULL is ready",
++ __func__));
+ break;
+ }
+
+@@ -910,7 +913,7 @@ t4_push_frames(struct adapter *sc, struct toepcb *toep, int drop)
+ toep->txsd_avail--;
+
+ t4_l2t_send(sc, wr, toep->l2te);
+- } while (m != NULL);
++ } while (m != NULL && (m->m_flags & M_NOTAVAIL) == 0);
+
+ /* Send a FIN if requested, but only if there's no more data to send */
+ if (m == NULL && toep->flags & TPF_SEND_FIN)
+....
+
+The MFC commit can now be published via `git push`
+
+[source,shell]
+....
+git push freebsd
+Enumerating objects: 13, done.
+Counting objects: 100% (13/13), done.
+Delta compression using up to 4 threads
+Compressing objects: 100% (7/7), done.
+Writing objects: 100% (7/7), 819 bytes | 117.00 KiB/s, done.
+Total 7 (delta 6), reused 0 (delta 0), pack-reused 0
+To gitrepo.FreeBSD.org:src.git
+ f4d0bc6aa6b9..525bd9c9dda7 stable/12 -> stable/12
+....
+
[[vendor-import-git]]
=== Vendor Imports with Git
More information about the dev-commits-doc-all
mailing list