Development/SubmittingPatches

We migrated from Gerrit to GitLab on August 23rd, 2020

We migrated from Gerrit to GitLab. The instructions below are being updated for use with GitLab. See the Migrating From Gerrit section if you have an open change there.

For complete instructions on contributing code, see the contribution section of the Developer's Guide.

Quickstart

The sample below demonstrates the workflow for a patch. Replace USERNAME with your GitLab username and BRANCH_NAME with the name you choose for your branch:

$ # Prepare your environment:
$ git clone -o upstream git@gitlab.com:wireshark/wireshark.git
$ cd wireshark
$ git remote add downstream git@gitlab.com:USERNAME/wireshark.git
$ cp tools/pre-commit .git/hooks
$ chmod a+x .git/hooks/pre-commit

$ # Submit a patch:
$ git checkout -b BRANCH_NAME
Switched to a new branch 'BRANCH_NAME'
<Make changes>
$ git commit
$ git push downstream HEAD
Enumerating objects: 14, done.
Counting objects: 100% (14/14), done.
Delta compression using up to 8 threads
Compressing objects: 100% (13/13), done.
Writing objects: 100% (13/13), 148.07 KiB | 21.15 MiB/s, done.
Total 13 (delta 6), reused 0 (delta 0), pack-reused 0
remote:
remote: To create a merge request for BRANCH_NAME, visit:
remote:   https://gitlab.com/USERNAME/wireshark/merge_requests/new?merge_request%5Bsource_branch%5D=BRANCH_NAME
remote:
To gitlab.com:USERNAME/wireshark.git
 * [new branch]      HEAD -> BRANCH_NAME
$ # Switch back to master for the next change:
$ git checkout master

$ # Amend a change (e.g.: edit commit message, add additional tests):
$ git checkout BRANCH_NAME
<Make changes>
$ git commit -a --amend
$ git push downstream +HEAD
$ # ...or add a separate commit on top of the current change:
< Make some changes>
$ git commit -a
$ git push downstream HEAD

$ # Work complete! Delete the local BRANCH_NAME branch:
$ git checkout master
Switched to branch 'master'
$ git branch -D BRANCH_NAME
Deleted branch BRANCH_NAME (was c159b39).

$ # Oops! Need to make more changes to the BRANCH_NAME branch:
$ git fetch downstream
$ git checkout downstream BRANCH_NAME

$ # Oops! Need to squash commits:
$ git checkout BRANCH_NAME
$ # Replace the '10' with the number of commits to squash together
$ # In your editor, keep 'pick' for your first commit and replace the other 'pick's with 'squash':
$ git rebase -i @~10 --rebase-merges
$ git push downstream +HEAD

Setup

Git Hooks

In your local repository directory, there will be a .git/hooks/ directory, with sample git hooks for running automatic actions before and after git commands.

Wireshark provides a custom pre-commit hook which performs general checks along with Wireshark-specific API and formatting checks, but it might return false positives. If you want to install it, copy the pre-commit file from the tools directory (cp ./tools/pre-commit .git/hooks/) and make sure it is executable. Alternatively, you can use Git's sample git pre-commit hook which detects whitespace errors such as mixed tabs and spaces. To install it, copy or rename the existing .git/hooks/pre-commit.sample file to .git/hooks/pre-commit. If the pre-commit hook is preventing you from committing what you believe is a valid change, you can run git commit --no-verify to skip running the hooks.

The Gerrit code review system required a commit-msg hook. This is no longer necessary.

Submitting A Change

The Review Process

To see the status of the review process, go to the URL returned in the previous step and see/discuss the patch and its current review status. When you create a merge requests, a series of tests will be run.

Writing A Good Commit Message

When running git commit, you will be prompted to describe the change. Here are some guidelines on how to make that message actually useful to other people (and to scripts that may try to parse it):

As mentioned above, you can use "#" to reference issues. "Closes #1234 (closed)" is special -- it will close issue 1234 when the change is merged, while other references such as "see #4512 (closed)" will simply link to the issue.

💡 If you're contributing a non-trivial fix to a dissector, you should open an issue and attach a sample capture file.

Putting all that together, we get the following example:

MIPv6: Fix dissection of Service Selection Identifier

APN field is not encoded as a dotted string so the first character is not a
length. Closes #10323.

Amending a Change

If you need to update your merge request you can do so by doing the following:

Fixing Merge Errors

Sometimes a reviewer/core developer will tell you your change has merge issues. This just means some other changes have conflicted with your change in ways that git cannot fix automatically. To resolve this, you need to fix the merge errors in your local branch and push a new patch set to GitLab for the same review. Here are the steps:

NOTE: syntax above should be git rebase upstream/master or git pull --rebase upstream master ???
See git rebase upstream/master vs git pull --rebase upstream master for discussion.

This fetches Wireshark's master branch and reapplies your changes on top of it.

Cleaning Up After A Merge

At some point (hopefully soon) your code changes will be merged into the master branch in gitlab.com/wireshark/wireshark. When that happens, it's safe to delete the changes branch in your local and personal GitLab repository.

You can delete both from the command line:

# Delete your local branch
git branch -d my-branch-name
# Delete your remote branch
git push -d downstream my-branch-name

You can also delete merged branches in your personal repository in the GitLab web UI.

Testing Someone Else's Merge Request

If you would like to test someone else's merge request or personal repository branch you can do the following:

# Fetch their branch to a local branch named FETCH_HEAD.
git fetch https://gitlab.com/some-other-user/wireshark.git their-branch-name
# Create a branch from FETCH_HEAD with a more useful name.
git checkout -b other-user-branch-name FETCH_HEAD

Each merge request will have a "Check out branch" button with similar instructions.

Undoing A Change

If you haven't committed yet and want to undo changes to a file, run git checkout -- $FILES to revert to the last version. To undo changes to the whole tree, run git checkout with no path.

If you've staged a change with git add but haven't committed yet, run git reset HEAD $FILES to unstage them (and then git checkout to actually undo the changes). Note that "HEAD" is literal text, not a variable.

If your change is in master, you should revert the change in a new commit with git revert $SHA. This generates a new commit, which must go through the normal review process.

Backporting A Change To A Release Branch

Some changes that fix bugs should also be applied to release branches, so that the bugs are fixed in the next regular Wireshark releases.

Changes that backport cleanly by cherry-picking them can be backported by using the "Cherry-pick" option in the "Options" item on the page for the commit. (N.B., not the page for the merge request.) The "Cherry-pick" option will pop up a dialog; remove the "master" branch name suggestion and choose the branch to which the change should be cherry-picked, and leave "Start a '''new merge request''' with these changes" checked. Clicking "Cherry-pick" will take you to a page for a new merge request; follow the instructions for merge requests in "Submitting A Change" above.

Changes that don't backport cleanly by cherry-picking them will require some editing on your part in order to backport them. To backport a change to a release branch that can't be cleanly cherry-picked:

The rest of the steps given above apply to this change.

A Super-Short Overview Of Git

Git manages a collection of commits, each identified by its unique SHA. Each commit (except the very first) contains a pointer to one or more parent commits, thus forming a history (technically a DAG).

Since multiple commits can have the same parent, and a single commit can have multiple parents, this allows branches of development to diverge and be merged. A branch diverges when two commits share a parent, and the branch merges when a single commit contains both branches as its parent:

  A
  |
  B
  |
  C
 / \
D   E
|   |
F   |
|   |
G   H
 \ /
  I

A label is a fixed mapping from a text string to a commit SHA, so for example tag "v1.10.6" simply points to SHA "3dac7877" which is a commit.

A "branch" is exactly the same; a mapping from a text string to a commit SHA. However, a branch is not fixed; when you make a new commit whose parent is currently a branch, that branch is updated to the new commit.

Before:

A
|
B <- branch: master-1.10, tag: v1.10.6

After a new commit:

A
|
B <- tag: v1.10.6
|
C <- branch: master-1.10

You can run git checkout BRANCHNAME to switch between branches you're working on, branches you're reviewing, and master.

A more comprehensive description of git can be found in this book

Watching changes

GitLab does not currently provide a way to track the changes happening on a specific file. It is a frequently requested open issue, so support may be added some time in the future.

Migrating From Gerrit

Prior to using GitLab, Wireshark used the Gerrit code review system. Here are a few things to keep in mind when migrating to GitLab:

GitLab associates branches with merge requests

Gerrit required a unique identifier (e.g. Change-Id: I58b2f0f5eeec85c891bd7fdbb6132eb8147baabf) in git commit messages in order to associate a commit with a change. GitLab uses a branch name in your personal repository. When you update a merge request, be sure to push it to the same branch name as before.

You need to remove your commit-msg hook.

Gerrit enforced change IDs using a commit-msg hook script. You should remove .git/hooks/commit-msg.

You need to update your remotes.

Your local repository might have a remote named "origin" or "gerrit" that points to "ssh://code.wireshark.org/wireshark". You can leave it as-is or rename it, but it might not be updated after the migration.

The documentation here and in the Developer's Guide assumes that you have an "upstream" remote for the main repository and a "downstream" remote for your personal repository:

git remote add upstream git@gitlab.com:wireshark/wireshark.git
git remote add downstream git@gitlab.com:USERNAME/wireshark.git

You can name the remotes anything you like.

If you mirror your fork you only need the "downstream" remote.

Gerrit changes weren't migrated to GitLab merge requests

Although bugs were migrated to issues, changes must be migrated individually. Since our Gerrit system has been pulled offline this is no longer possible.

Bugs / issues are linked differently

Prior to the migration you could link to bugs using "Bug: 1234" or "Ping-Bug: 1234". Issue numbers in GitLab must be prefixed with a number sign (#). You can automatically close an issue with "closes", e.g. "closes #1234 (closed)".

There are multiple CLI options.

Prior to the migration, we used the git-review tool to manage changes on the command line. The following tools might let us manage merge requests, but more testing needs to be done.


Imported from https://wiki.wireshark.org/Development/SubmittingPatches on 2020-08-11 23:13:08 UTC