push access guidelines
This is a page for cURL team members who have push access. It is mainly intended for new members to outline some of the once unwritten guidelines of the curl repo.
Your github account should have a fork of the curl repository, and you have cloned that fork locally. In your clone you will have a remote --typically named 'origin'-- that points to your fork. You will need to add a remote --typically named 'upstream'-- that points to this canonical repo:
git remote add upstream [email protected]:curl/curl.git
Now if you run git remote -v you should have at least an origin and upstream:
origin [email protected]:jay/curl.git (fetch)
origin [email protected]:jay/curl.git (push)
upstream [email protected]:curl/curl.git (fetch)
upstream [email protected]:curl/curl.git (push)
master should reflect upstream/master or what it is about to become once you push. If you work from your local master you could you end up accidentally pushing something you didn't intend to. Branches are cheap in git, work from topic branches to make changes.
We use a linear history, do not push merge commits on master. Instead, when you are done with your work checkout your topic branch and rebase on upstream/master after fetching the latest changes.
The person who did the work in a commit must be set in that commit as the Author. The person who pushes that commit to the repo must be set in that commit as the Committer. In this way you take responsibility for what you commit. A good way to check the commiter and author name is
git log --pretty=fullerGitHub's merge button methods ('Create a merge commit', 'Squash and merge') are not acceptable since we don't use merge commits and our author/committer attribution is different from GitHub's squash.
Commits that are all the same area and change should be squashed before you push. A scenario where this might happen is a PR associated branch updated by adding more commits instead of updating their original commit. For example, a poster may point out a typo or some other problem and as the owner fixes those problems they add more commits.
Before:
http2: handle closed streams when uploading
http2: typo
http2: show info message when stream closed
After:
http2: handle closed streams when uploading
We have a coding style that must be followed. checksrc enforces a lot of that style, and you should run it before you push. On Windows you may instead run checksrc.bat from the command prompt as long as you do it from the projects directory (cd projects && checksrc).
Read the CONTRIBUTE document if you haven't already! There's a section in it on writing good commit messages that must be followed. Notably, not everyone's git editor wraps commit messages. For example I use gitk and if a commit message is one long line it's not wrapped and I can't easily read it. You must wrap the message at 72 with some rare exceptions like URLs, shell commands that shouldn't be broken up, etc. The subject ideally should be less than 60 columns but we don't strictly enforce that.
Commit subject messages should be in imperative style present tense.
Yes:
nss: work around race condition in PK11_FindSlotByName()
No:
nss: worked around race condition in PK11_FindSlotByName()
Also see A Note About Git Commit Messages.
Add reference links to the discussions related to your commit in the commit message. This is typically done through the Bug: <URL> line as described in CONTRIBUTE but we also use lines like Ref: <URL> or Closes <URL> or Fixes <URL>. (Note the latter two signal to GitHub to automatically close an issue).
When a pull request has been reviewed and is about to be pushed, rebase it on master and adjust the commit message. For example, when pull request 123 to solve issue 100 has been reviewed by Alice Li and Bob Smith, the commit message could look like this:
docs/foo: make sure foo is spelled Foo
Foo is the official name.
Fixes #100
Closes #123
Reviewed-by: Alice Li
Reviewed-by: Bob Smith
Other tags that could be added:
-
Suggested-by: <name here>- someone who proposed the change. -
Reported-by: <name here>- someone who reported the issue (and is not covered by the issue).
We encourage discussion and review for all substantial changes, and any change in external behavior. There's no hard-and-fast rule, just use your judgment. A typo for example does not require a review or a discussion, just commit it. Team members can start a PR like anyone else with proposed changes, so if you're even a little unsure just do that.
Do not force your push to upstream/master, as you would just overwrite someone else's work and cause problems for those who have cloned the repo in that state. github will prevent forced pushes as the master branch is "protected".
In feature-branches and especially when working on a pull-request, a forced push after a rebase and squash can in fact often be suggested and preferred - to keep the set of commits clean and easily reviewable. At least as long as that branch is primarily developed by a single developer.
You should always git fetch upstream to get the latest changes before rebasing on upstream/master and then pushing. If someone else has pushed in the meantime your push will be rejected with a message like this:
To [email protected]:curl/curl.git
! [rejected] master -> master (fetch first)
error: failed to push some refs to '[email protected]:curl/curl.git'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
Do not use git pull to resolve this, since we don't use merge commits. Instead use git fetch upstream and then gitk (or some other visualizer) to identify where the divergence is. The divergence should not be far back. Review the commits made to upstream/master since the divergence. If you see any signs of tampering contact senior members. Otherwise reset your master to upstream/master. That will wipe out your changes, so save them to temp if they aren't there already. Then you can rebase your changes on upstream/master and try again.
git fetch upstream # Get the latest changes from upstream
gitk upstream/master # Find the divergence from your master
git branch temp master # Save your master to temp
git reset --hard upstream/master # Reset your master to upstream/master
git checkout temp # Checkout your to-be-pushed work
git rebase upstream/master # Rebase it on upstream/master
git checkout master # Checkout your master
git merge --ff-only temp # Merge your work *without* a merge commit
git push upstream # Double-check everything then push again
Using all you've learned above, let's say you want to commit PR 239 which you did not author. First name it as local branch pr_239, then check out a temporary branch from that and rebase on upstream/master.
# First make sure your index is clean.
git diff-index --quiet --cached HEAD || echo "WARNING: INDEX ISN'T CLEAN"
C=239;git fetch upstream refs/pull/$C/head:pr_$C
git checkout -b temp pr_$C
git fetch upstream && git rebase upstream/master
Count the number of commits. If there is a single commit, regardless of whether the message needs to be modified you need to amend the commit to become the committer.
git commit --amend -c HEAD
If however let's say there are 3 commits we want to squash them into one. To do that we'll soft reset back to their 1st commit which will move the changes from the later two commits to the index. Then append to the first commit. (You could instead use git rebase -i and choose those commits to squash or reword.) Otherwise if there is not more than a single commit skip these steps.
git reset --soft HEAD~2
git commit --amend
Modify the commit message to meet guidelines. At the very least you will want to add a Bug: line or Fixes (etc.). Examples: 3a5d5de 8e176a7
Confirm that you ARE listed as the committer and ARE NOT listed as the author for all the commits that will be added. git gui in particular will amend commits in a way that you can be listed as the author for someone else's work, likely not what you intended. Fix any error, for example git commit --amend --author="who <[email protected]>".
Once you are ready, and you have met all of the guidelines discussed here, merge into your local master using fast-forward merge only, so there will not be a merge commit. If this step fails, master has some commit that is not in upstream/master (note earlier guideline don't work in master).
git checkout master
git merge --ff-only temp
Check once again to make sure master looks the way you expect it to look, and then push upstream.
git push upstream
When you write a fix for an issue it's best to do it in a local branch that you push to your origin, not upstream. Then share that branch in the thread so that other people can comment on it and the reporter can try your fix. Make sure in your commit you reference the issue as Bug: and the reporter in Reported-by:. Use the reporter's real name if known (go to their user page). If not known use [email protected]. See issue 983 for an example. After others confirm the fix you will need to follow steps similar to the PR steps.
git checkout -b temp disable_schannel_alpn_wine
git fetch upstream && git rebase upstream/master
git checkout master
git merge --ff-only temp
# Make certain the changes reflect what you want to go upstream, then send it
git push upstream
git branch -D temp
The result in this case is 895168b