Metasploit is built incrementally by the community through GitHub's [Pull Request](https://github.com/rapid7/metasploit-framework/pulls) mechanism. Submitting pull requests (or PRs) is already discussed in the [[Dev environment setup|./dev/Setting-Up-a-Metasploit-Development-Environment.md]] documentation. It's important to realize that PRs are a feature of GitHub, not git, so this document will take a look at how to get your git environment to deal with them sensibly.
- Use `gpg --list-keys` to view your available keys. Note that on certain systems you may need to replace `gpg` with `gpg2`. Sample output can be seen below:
- When merging code from a pull request, always, always `merge -S --no-ff --edit`, and write a meaningful [50/72](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) commit message that references the original PR as `#1234` (not PR1234, not PR#1234, not 1234). For example, your message should look like this:
- Note that the `--no-ff` flag should be used both for PRs that go back to a contributor's branch as well as PRs that land in Metasploit's master branch.
- If you're making changes (often the case), merge to a landing branch, then merge **that** branch to upstream/master with the `-S --no-ff --edit` options.
Check out [this gist](https://gist.github.com/todb-r7/3fbee1a9e7b36d82ca55) that automates (mostly) landing pull requests, signing the merge commit, all while rarely losing a race with other committers.
First, fork and clone the `rapid7/metasploit-framework` repo, [following these instructions](https://help.github.com/articles/fork-a-repo). I like using ssh with `~/.ssh/config` aliases [[as described here|./dev/Setting-Up-a-Metasploit-Development-Environment.md]], but the https method will work, too.
Once this is done, you will have a remote repository called "origin," which points to your forked repository on GitHub. You will be doing most of your work in your own fork of Metasploit, even if you have commit rights to Rapid7's fork. Now, we're going to add an "upstream" repository to talk to the Rapid7 repository.
In addition, we're going to add a magical line to the config file that will let us see all pull requests against the Rapid7 repo (both open and closed). Note that this will take a minute since you're adding some hundreds of megs to your clone's refs.
So, open up `metasploit-framework/.git/config` with your favorite editor, add an upstream remote, and add the pull request refs for both your and Rapid7's forks. In the end, you should have a section that started off like this:
Some people like to copy these over into remotes named "rapid7" and "yourusername" just so they don't have to remember about "origin" and "upstream," but for this doc, we'll just assume you have "origin" and "upstream" defined like this.
Now, you can git fetch the remote PRs. This will take a little bit, since we have a couple dozen MBs of pull request data. Storage is cheap, though, right?
A manageable strategy for dealing with outstanding PRs is to start pre-merge testing on the pull request in isolation. For example, to work on PR #1217, we would:
Now, we're on a local branch identical to the original pull request, and can move on from there. We can make our changes, isolated from master, and then either send them back to the contributor (this requires looking up the original contributor's GitHub username and branch name on GitHub), or if there aren't any changes or the changes are trivial, we can land them (if you have committer rights to Rapid7's repo, this is where you land them to the upstream repo).
In this particular case with PR #1217, I did want to send some changes back to the contributor.
**Important**: If the codebase the contributor's PR is based on is severely outdated (e.g., they branched off an outdated ```master```), you should not test their PR in isolation as described above. Instead, you should create a test branch that is identical to the latest codebase, merge the contributor's PR into the test branch, and then start your testing. You may need to `bundle install` to ensure you're using the right gems.
This ensures that the contributor's PR is being tested against the latest codebase and not an outdated one. **If you do not do this**, when you land the PR, you may end up breaking Metasploit.
Note that the example above will leave you in a detached HEAD state. This is fine if you just want to test the module in question, however if you want to make any changes, don't forget to make a new branch. For the example above this could be done by running the following command:
After your `.git/config` is set up per the above, and you successfully run `git fetch --all`, you are two steps away from being able to check out a branch from a contributor's forked repo.
You need to add their fork once as a remote: `git remote add OTHER_USER git://github.com/OTHER_USER/metasploit-framework.git`. Now pull down the latest from them: `git fetch OTHER_USER`. Now you can check out branches from OTHER_USER per usual, e.g. `git checkout bug/foo`.
This sequence does a few things after editing `.gitconfig`. It creates another copy of landing-1217 (which is itself a copy of upstream/pr/1217)). Next, I push those changes to my branch (todb-r7, aka "origin"). Finally, I have a mighty [.gitconfig alias here](https://gist.github.com/todb-r7/5438391) to open a browser window to send a pull request to the original contributor's branch (you will want to edit yours to reflect your real GitHub username, of course).
I opened that in a browser, and ended up with https://github.com/schierlm/metasploit-framework/pull/1 . Once [@schierlm](https://github.com/schierlm) landed it on his branch (again, using `git merge --no-ff` and a short, informational merge commit message), all I (or anyone) had to do was `git fetch` to get the change reflected in upstream/pr/1217, and then the integration of the PR could continue.
Note the important bit here: **you do not need commit rights to Rapid7 to branch pull requests**. If Alice knows a solution to Bob's pull request that Juan pointed out, it is **easy** for Alice to provide that solution by following the procedure above. `git blame` will still work correctly, commit histories will all be accurate, everyone on the pull request will be notified of Alice's changes, and Juan doesn't have to wait around for Bob to figure out how to use `send_request_cgi()` or whatever the problem was. The hardest part is remembering how to construct the pull request to Bob -- lucky for you, [this .git/config alias](https://gist.github.com/todb-r7/5438391) makes that part pretty push-button.
Back to PR #1217. Turns out, my change was enough to land the original chunk of work. So, someone else ([@jlee-r7](https://github.com/jlee-r7)) was able to to do something like this:
The `--edit` is optional if we have our editor configured correctly in `$HOME/.gitconfig`. The point here is that we *always* want a merge commit, and we *never* want to use the (often useless) default merge commit message. For #1217, this was changed to:
Finally, the -S indicates we are going to sign the merge, using our GPG key. This is a nice way to prove in a secure way that this merge is, in fact, coming from you, and not someone impersonating you. For more on signing merges, see [A Git Horror Story: Repository Integrity With Signed Commits](http://mikegerwitz.com/papers/git-horror-story.html).
After a pull request has been merged, release notes should be added to the pull request in the form of a comment. These release notes will automatically be extracted and used as documentation when creating the [metasploit release notes](https://help.rapid7.com/metasploit/release-notes/).
The [rn-no-release-notes](https://github.com/rapid7/metasploit-framework/issues?utf8=%E2%9C%93&q=label%3Arn-no-release-notes+) label must be added if there are no release notes for the merged pull request.
