Gerrit » History » Version 65
neels, 04/25/2017 11:38 AM
1 | 1 | zecke | h1. Contributing using Gerrit |
---|---|---|---|
2 | 1 | zecke | |
3 | 11 | laforge | {{>toc}} |
4 | 11 | laforge | |
5 | 10 | laforge | At [[OpenBSC:OsmoDevCon2016]] we discussed problems with our past contribution / patch submission process using mails on the mailing list as well as patchwork. The result is that we want to give Gerrit a try for some time and see if it helps us to have a better process |
6 | 1 | zecke | |
7 | 10 | laforge | Gerrit is a review tool that integrates nicely with git and ssh. You can find general information about Gerrit at https://www.gerritcodereview.com/ |
8 | 1 | zecke | |
9 | 10 | laforge | The advantages of Gerrit are: |
10 | 10 | laforge | * patch submission status is automatically tracked, also with several revisions for a patch set. |
11 | 10 | laforge | * patches are build-tested (and possibly even further tested) by jenkins before they are applied |
12 | 10 | laforge | * developers + maintainers can formally vote on a patch (developer: -1/0/+1, maintainer: -2/0/+2) |
13 | 10 | laforge | * once a patch has +2 score, it can be (automatically) merged into master |
14 | 10 | laforge | * patch sumissions not via git send-email but direcly from git |
15 | 10 | laforge | |
16 | 10 | laforge | h2. Osmocom Subprojects using Gerrit |
17 | 10 | laforge | |
18 | 1 | zecke | The following projects use Gerrit to contribute changes: |
19 | 1 | zecke | |
20 | 1 | zecke | * libosmocore.git |
21 | 1 | zecke | * libosmo-abis.git |
22 | 1 | zecke | * libosmo-netif.git |
23 | 1 | zecke | * libosmo-sccp.git |
24 | 1 | zecke | * libsmpp34.git |
25 | 1 | zecke | * openbsc.git |
26 | 1 | zecke | * osmo-bts.git |
27 | 1 | zecke | * osmo-iuh.git |
28 | 1 | zecke | * osmo-pcu.git |
29 | 5 | zecke | * cellmgr-ng.git |
30 | 1 | zecke | * osmo-sip-connector.git |
31 | 30 | neels | |
32 | 1 | zecke | h2. Configuring Gerrit/Account |
33 | 1 | zecke | |
34 | 54 | neels | You will need to sign-up at https://gerrit.osmocom.org/login/. If you have an Osmocom Redmine account you can use https://osmocom.org/openid as OpenID provider. |
35 | 1 | zecke | |
36 | 55 | neels | * first sign in on https://osmocom.org. |
37 | 55 | neels | * go to https://gerrit.osmocom.org and click the "Sign in" link. |
38 | 61 | neels | * enter <pre>https://osmocom.org/openid</pre> as OpenID provider and hit the "Sign in" button. |
39 | 61 | neels | |
40 | 61 | neels | *careful:* enter 'https' to ensure that your openid credentials are passed on encryptedly. |
41 | 61 | neels | *pitfall:* if you're logged in on 'projects.osmocom.org', you should also use the openid provider: <pre>https://projects.osmocom.org/openid</pre> |
42 | 61 | neels | *note:* gerrit will create a distinct user for each openid URL you pass. If you logged in successfully but your user seems to have lost permissions, you may have created an evil twin user: contact us on the mailing list so we can fix it in the user database. |
43 | 54 | neels | |
44 | 54 | neels | If you have no Osmocom redmine account, you can simply create one online at the "Register" link in the upper right corner. |
45 | 10 | laforge | Even without an existing or new redmine account, you should also be able to use any other OpenID provider to authenticate against gerrit (untested). |
46 | 10 | laforge | |
47 | 10 | laforge | After the initial sign-up you will need to: |
48 | 1 | zecke | |
49 | 1 | zecke | * Pick a username (can not be changed) |
50 | 1 | zecke | * Add your public ssh key(s) |
51 | 1 | zecke | * Add email addresses you intend to use as author/comitter |
52 | 30 | neels | |
53 | 30 | neels | If you would like to push private branches to the Gerrit repository, you also need to be added to the "known users" group. |
54 | 30 | neels | Please send a short requesting email to openbsc@lists.osmocom.org. |
55 | 1 | zecke | |
56 | 1 | zecke | h2. Setting up Gerrit for commits and pushing |
57 | 1 | zecke | |
58 | 33 | neels | *Note:* it is easiest to work with gerrit when gerrit is the only remote in your git clone. |
59 | 33 | neels | When you clone from git.osmocom.org and add the gerrit remote, git will have two remotes, |
60 | 36 | neels | so when you first checkout a branch you have to supply the remote explicitly (cumbersome). |
61 | 34 | neels | The gerrit repositories and git.osmocom.org are constantly synced, so it is sufficient |
62 | 34 | neels | to clone from gerrit only. |
63 | 33 | neels | |
64 | 33 | neels | h3. Simplest: new clone |
65 | 33 | neels | |
66 | 35 | neels | * Create a new clone from gerrit |
67 | 35 | neels | * Fetch the commit hook that adds Change-Id to each commit to uniquely identify a commit |
68 | 42 | neels | |
69 | 33 | neels | <pre> |
70 | 33 | neels | git clone ssh://$USERNAME@gerrit.osmocom.org:29418/$PROJECT.git |
71 | 33 | neels | scp -P 29418 $USERNAME@gerrit.osmocom.org:hooks/commit-msg $PROJECT/.git/hooks/ |
72 | 33 | neels | </pre> |
73 | 33 | neels | |
74 | 33 | neels | h3. SSH config |
75 | 33 | neels | |
76 | 33 | neels | In '~/.ssh/config', add these lines: |
77 | 33 | neels | <pre> |
78 | 52 | neels | Host go |
79 | 33 | neels | Hostname gerrit.osmocom.org |
80 | 33 | neels | Port 29418 |
81 | 33 | neels | User $USERNAME |
82 | 33 | neels | </pre> |
83 | 52 | neels | ('go' means gerrit.osmocom, replace with your favorite shortcut name, |
84 | 33 | neels | replace '$USERNAME' with your user name as used on the gerrit website) |
85 | 33 | neels | |
86 | 33 | neels | Then you can shorten above commands to |
87 | 1 | zecke | <pre> |
88 | 52 | neels | git clone ssh://go/$PROJECT.git |
89 | 51 | neels | cd $PROJECT |
90 | 51 | neels | scp go:hooks/commit-msg .git/hooks/ |
91 | 33 | neels | </pre> |
92 | 33 | neels | |
93 | 46 | neels | h3. Committer must match |
94 | 46 | neels | |
95 | 47 | neels | Your email address on gerrit and the email address git places in your |
96 | 46 | neels | commits must match, or you will get rejected with an error message like |
97 | 46 | neels | "invalid commiter". You can add email addresses on the gerrit web UI. |
98 | 46 | neels | |
99 | 33 | neels | h3. Add gerrit to an existing clone |
100 | 33 | neels | |
101 | 7 | neels | * Add the remote to be able to fetch and push to gerrit |
102 | 7 | neels | * Fetch the commit hook that adds Change-Id to each commit to uniquely identify a commit |
103 | 7 | neels | |
104 | 7 | neels | <pre> |
105 | 7 | neels | USERNAME=gerrit_user_name |
106 | 7 | neels | PROJECT=$(basename $PWD) |
107 | 1 | zecke | git remote add gerrit ssh://$USERNAME@gerrit.osmocom.org:29418/$PROJECT.git |
108 | 1 | zecke | scp -P 29418 $USERNAME@gerrit.osmocom.org:hooks/commit-msg .git/hooks/ |
109 | 44 | neels | </pre> |
110 | 44 | neels | |
111 | 33 | neels | h2. Push for review |
112 | 1 | zecke | |
113 | 38 | neels | Checkout the revision or branch that you want to submit for review, then |
114 | 38 | neels | |
115 | 31 | neels | <pre> |
116 | 48 | ahuemer | git push gerrit HEAD:refs/for/master |
117 | 1 | zecke | </pre> |
118 | 38 | neels | |
119 | 1 | zecke | You can optionally add a topic name with |
120 | 40 | neels | |
121 | 40 | neels | <pre> |
122 | 48 | ahuemer | git push gerrit HEAD:refs/for/master/my_topic |
123 | 38 | neels | </pre> |
124 | 38 | neels | |
125 | 57 | neels | h2. Merge patch to master |
126 | 57 | neels | |
127 | 57 | neels | A patch can be merged when it has CR+2 and V+1 votes, and if, in case of a |
128 | 57 | neels | series of patches pushed from a branch, when its ancestor patches can also be |
129 | 57 | neels | merged. |
130 | 57 | neels | |
131 | 57 | neels | Sometimes the reviewer that gives CR+2 also hits the "Submit" button right away |
132 | 57 | neels | to merge the patch to master. Sometimes it is left up to the owner of the patch |
133 | 59 | neels | to decide when to hit "Submit" (who needs to be in the "Known Users" group). |
134 | 57 | neels | |
135 | 57 | neels | The V+1 vote means "build is verified" and is usually given by our jenkins |
136 | 59 | neels | gerrit builds: https://jenkins.osmocom.org/jenkins/view/Jenkins-Gerrit/ |
137 | 57 | neels | |
138 | 57 | neels | The CR+2 vote means "code reviewed and ready for merge to master branch". |
139 | 57 | neels | Accounts with the "Reviewer" role for a given project are allowed to give CR+2 |
140 | 57 | neels | votes. Others are allowed to give CR+1 (and CR-1). CR votes _don't_ add up. |
141 | 57 | neels | |
142 | 65 | neels | _Fixed:_ -Sometimes hitting the "Submit" button results in an error message saying |
143 | 57 | neels | "Change is New", which is a bug related to a private branch with the same |
144 | 65 | neels | patches being present. Can be fixed e.g. by an admin's manual push to master.- |
145 | 1 | zecke | |
146 | 38 | neels | h2. Push a "private" user branch |
147 | 33 | neels | |
148 | 1 | zecke | *Note* that you must be a member of the "known users" group, see above. |
149 | 33 | neels | |
150 | 43 | neels | If your local branch name is of the form 'your_name/topic', you can just |
151 | 1 | zecke | <pre> |
152 | 1 | zecke | git push |
153 | 33 | neels | </pre> |
154 | 41 | neels | and git will tell you what to do. |
155 | 1 | zecke | |
156 | 41 | neels | To push from a "nonstandard" local branch name, do |
157 | 41 | neels | <pre> |
158 | 50 | msuraev | git push gerrit HEAD:refs/heads/user/$USERNAME/branch_name |
159 | 33 | neels | </pre> |
160 | 33 | neels | |
161 | 39 | neels | |
162 | 39 | neels | h2. List changesets in gerrit |
163 | 39 | neels | |
164 | 7 | neels | <pre> |
165 | 48 | ahuemer | git ls-remote gerrit changes/* |
166 | 2 | zecke | </pre> |
167 | 12 | msuraev | |
168 | 17 | neels | h1. Tips and Tricks |
169 | 1 | zecke | |
170 | 17 | neels | h2. Throw-away branch |
171 | 17 | neels | |
172 | 17 | neels | If you need to adjust and re-submit patches, it may be handy to create a throw-away branch ("R D" in magit-gerrit in emacs for example), |
173 | 45 | neels | make your changes/amendments and then send patch(es) back to gerrit while removing temporary branch automatically with "git review -f". |
174 | 13 | neels | |
175 | 56 | neels | h2. Fetch a patch from gerrit |
176 | 56 | neels | |
177 | 56 | neels | This script (I called it @P@) makes fetching a patch set from gerrit a breeze: |
178 | 56 | neels | <pre> |
179 | 56 | neels | #!/bin/sh |
180 | 56 | neels | # fetch gerrit patch into new branch named like the patch number. |
181 | 56 | neels | # |
182 | 56 | neels | # Usage: go to a git clone and pass a patch number: |
183 | 56 | neels | # |
184 | 56 | neels | # cd openbsc |
185 | 56 | neels | # P 973 |
186 | 56 | neels | # or |
187 | 56 | neels | # P 973/2 |
188 | 56 | neels | # |
189 | 56 | neels | # Will create new local branches '973_4' (if 4 is the latest patch set) |
190 | 56 | neels | # or '973_2', respectively. |
191 | 56 | neels | |
192 | 56 | neels | patch="$1" |
193 | 56 | neels | |
194 | 56 | neels | if [ -z "$patch" ]; then |
195 | 56 | neels | echo "Usage: P 1234[/5]" |
196 | 56 | neels | exit 1 |
197 | 56 | neels | fi |
198 | 56 | neels | |
199 | 56 | neels | if [ -z "$(echo "$patch" | grep '/')" ]; then |
200 | 56 | neels | patch="/$patch/" |
201 | 56 | neels | fi |
202 | 56 | neels | |
203 | 56 | neels | if [ -z "$(echo "$patch" | grep '^/')" ]; then |
204 | 56 | neels | patch="/$patch" |
205 | 56 | neels | fi |
206 | 56 | neels | |
207 | 56 | neels | last_set="$(git ls-remote origin "changes/*" | grep "$patch" | sed 's#.*/\([^/]*\)$#\1 &#' | sort -n | tail -n 1)" |
208 | 56 | neels | if [ -z "$last_set" ]; then |
209 | 56 | neels | echo "Not found: $patch" |
210 | 56 | neels | exit 1 |
211 | 56 | neels | fi |
212 | 56 | neels | |
213 | 56 | neels | change_name="$(echo "$last_set" | sed 's/.*\(refs.*\)/\1/')" |
214 | 56 | neels | branch_name="$(echo "$change_name" | sed 's#refs/changes/../\([0-9]*\)/\([0-9]*\)#\1_\2#')" |
215 | 56 | neels | |
216 | 56 | neels | set -x |
217 | 56 | neels | git fetch origin "$change_name" |
218 | 56 | neels | git co -b "$branch_name" FETCH_HEAD |
219 | 56 | neels | </pre> |
220 | 56 | neels | |
221 | 25 | neels | h2. Re-submit a Branch with Amended Commits |
222 | 13 | neels | |
223 | 1 | zecke | On a feature branch, one typically has numerous commits that depend on their preceding commits. |
224 | 58 | neels | Often, some of the branch commits need to be amended for fixes. You can re-submit changes to |
225 | 58 | neels | patches on your branch by pushing in the same way that you first submitted the branch. |
226 | 22 | neels | |
227 | 58 | neels | Note: if you modify the Change-Ids in the commit logs, your push would open entirely new |
228 | 58 | neels | review entries and you would have to abandon your previous submission. Comments on the first |
229 | 58 | neels | submission are "lost" and you cannot diff between patch sets. |
230 | 29 | neels | |
231 | 58 | neels | (There used to be a bug in gerrit that required editing the first patch to be able to |
232 | 58 | neels | re-submit a branch, but that's fixed.) |
233 | 29 | neels | |
234 | 29 | neels | |
235 | 29 | neels | |
236 | 26 | neels | h2. Re-submit Previously Abandoned Changes |
237 | 16 | neels | |
238 | 16 | neels | You have to edit the Change-Ids, on a branch that would be every single commit log message. |
239 | 16 | neels | |
240 | 13 | neels | <pre> |
241 | 1 | zecke | cd openbsc |
242 | 1 | zecke | git co my-branch |
243 | 1 | zecke | git rebase -i master |
244 | 1 | zecke | # replace all 'pick' with 'r' (or 'reword'), exit your editor |
245 | 13 | neels | # git presents each commit log message for editing |
246 | 13 | neels | </pre> |
247 | 13 | neels | |
248 | 27 | neels | h2. Submit a "private" branch for master |
249 | 21 | neels | |
250 | 21 | neels | If you've pushed a branch to refs/heads/* somewhere, gerrit will already know the Change-Ids on it. |
251 | 24 | neels | Make sure the option [[Gerrit#Private-Branches-Create-a-new-change-for-every-commit|Create a new change for every commit not in the target branch]] is _TRUE_ for your project, |
252 | 21 | neels | or gerrit will refuse to accept your submission. |
253 | 21 | neels | |
254 | 60 | neels | h2. 502 Bad Gateway |
255 | 60 | neels | |
256 | 60 | neels | When getting a "Bad Gateway" error message upon trying to login on gerrit, you probably just need to restart your web browser. The reason is not clear. |
257 | 60 | neels | |
258 | 16 | neels | h1. Reasons for Particular Configuration |
259 | 13 | neels | |
260 | 16 | neels | h2. Rebase if necessary |
261 | 16 | neels | |
262 | 16 | neels | There are different merge strategies that Gerrit performs to accept patches. |
263 | 13 | neels | Each project can be configured to a specific merge strategy, but unfortunately you can't |
264 | 13 | neels | decide on a strategy per patch submission. |
265 | 13 | neels | |
266 | 13 | neels | It seems that the "Merge if Necessary" strategy is best supported, but it creates non-linear |
267 | 13 | neels | history with numerous merge commits that are usually not at all necessary. |
268 | 13 | neels | |
269 | 13 | neels | Instead, the "Cherry Pick" strategy puts each patch onto current master's HEAD to create |
270 | 13 | neels | linear history. However, this will cause merge failures as soon as one patch depends on |
271 | 13 | neels | another submitted patch, as typical for a feature branch submission. |
272 | 13 | neels | |
273 | 1 | zecke | So we prefer the "Rebase if Necessary" strategy, which always tries to apply your patches to |
274 | 13 | neels | the current master HEAD, in sequence with the previous patches on the same branch. |
275 | 13 | neels | However, some problems still remain, including some bugs in "Rebase if Necessary". |
276 | 1 | zecke | |
277 | 13 | neels | There's a problem with "Rebase if Necessary": If your branch sits at master's HEAD, Gerrit |
278 | 13 | neels | refuses to accept the submission, because it thinks that no new changes are submitted. |
279 | 13 | neels | This is a bug in Gerrit, which holger has fixed manually in our Gerrit installation: |
280 | 1 | zecke | |
281 | 1 | zecke | https://bugs.chromium.org/p/gerrit/issues/detail?id=4158 |
282 | 1 | zecke | |
283 | 1 | zecke | |
284 | 16 | neels | h2. Private Branches: Create a new change for every commit... |
285 | 1 | zecke | |
286 | 13 | neels | Say you have an extensive feature in development, and you want to keep it on the |
287 | 13 | neels | upstream git repository to a) keep it safe and b) collaborate with other devs on it. |
288 | 16 | neels | So, of course, you have regularly pushed to refs/heads/yoyodyne/feature. |
289 | 13 | neels | |
290 | 13 | neels | Since you have the gerrit commit hook installed, your feature branch already has |
291 | 13 | neels | Change-Id tags in all commit log messages. |
292 | 13 | neels | |
293 | 13 | neels | Now your feature is complete and you would like to submit it to master. |
294 | 13 | neels | Alas, Gerrit refuses to accept your patch submission for master, because it |
295 | 13 | neels | knows the Change-Ids are also on a different branch. |
296 | 13 | neels | |
297 | 16 | neels | Gerrit by default enforces that a Change-Id must be unique across all branches, |
298 | 16 | neels | so that each submission for review is separate for each branch. Instead, we |
299 | 16 | neels | want to handle Change-Ids per-branch, so that you can have the same change |
300 | 16 | neels | submitted to different branches, as separate patch submissions, without having |
301 | 16 | neels | to cosmetically adjust the Change-Id. |
302 | 13 | neels | |
303 | 16 | neels | Solution: set the option |
304 | 16 | neels | _Create a new change for every commit not in the target branch_ to _TRUE_ |
305 | 13 | neels | |
306 | 20 | neels | h2. Allow content merges |
307 | 14 | neels | |
308 | 14 | neels | By default, gerrit compares patches only by the files' paths. If two paths are the same, |
309 | 14 | neels | it immediately shows them as conflicts (path conflicts). |
310 | 14 | neels | |
311 | 14 | neels | In software development, a conflict usually means an actual content conflict, so if the |
312 | 14 | neels | edits are in two entirely separate places in the file, we don't consider this a conflict. |
313 | 14 | neels | |
314 | 23 | neels | By setting _Allow content merges_ to _TRUE_ in the git project config, we tell Gerrit to |
315 | 14 | neels | perform text merges of the submitted patches and only complain about actual content |
316 | 14 | neels | conflicts, in the usual software engineering sense. |
317 | 32 | neels | |
318 | 32 | neels | h1. Admin |
319 | 32 | neels | |
320 | 32 | neels | h2. Adding users to groups |
321 | 32 | neels | |
322 | 32 | neels | Normally, the gerrit UI auto-completes a user name in the edit field. It has happened |
323 | 32 | neels | though that an existing user is not auto-completed, as if it didn't exist. In that case, |
324 | 32 | neels | find out the user ID (seven digit number like 1000123) and just enter that. |
325 | 32 | neels | |
326 | 32 | neels | The user ID can be found on the user's "Settings" page, or in the database (s.b.). |
327 | 32 | neels | |
328 | 32 | neels | h2. Querying the database directly |
329 | 32 | neels | |
330 | 32 | neels | If your user has permission to access the database, you can place SQL queries using the |
331 | 32 | neels | 'gerrit gsql' commands over ssh: |
332 | 32 | neels | |
333 | 32 | neels | <pre> |
334 | 64 | neels | ssh go 'gerrit gsql -c "show tables"' |
335 | 64 | neels | ssh go 'gerrit gsql -c "select full_name,account_id from accounts"' |
336 | 1 | zecke | </pre> |
337 | 53 | neels | |
338 | 53 | neels | (see ~/.ssh/config above for the 'go' shortcut) |
339 | 32 | neels | |
340 | 32 | neels | This seems to be the MySQL dialect. |
341 | 62 | neels | |
342 | 62 | neels | h2. Fix evil twin users |
343 | 62 | neels | |
344 | 62 | neels | If differing openid URLs have lead to evil twin users shadowing the same email address just without the permissions, you can fix it like this: |
345 | 62 | neels | |
346 | 62 | neels | <pre> |
347 | 64 | neels | ssh go "gerrit gsql -c \"select * from account_external_ids where email_address like '%foo%'\"" |
348 | 62 | neels | # ACCOUNT_ID | EMAIL_ADDRESS | PASSWORD | EXTERNAL_ID |
349 | 62 | neels | # -----------+-----------------+----------+---------------------------------- |
350 | 62 | neels | # 100004 | foo@example.com | NULL | https://osmocom.org/openid/user/777 |
351 | 62 | neels | # 100021 | foo@example.com | NULL | https://projects.osmocom.org/openid/user/777 |
352 | 62 | neels | |
353 | 64 | neels | ssh go "gerrit gsql -c \"update account_external_ids set account_id = 100004 where email_address like '%foo%'\"" |
354 | 62 | neels | |
355 | 64 | neels | ssh go "gerrit gsql -c \"select * from account_external_ids where email_address like '%foo%'\"" |
356 | 62 | neels | # ACCOUNT_ID | EMAIL_ADDRESS | PASSWORD | EXTERNAL_ID |
357 | 62 | neels | # -----------+-----------------+----------+---------------------------------- |
358 | 62 | neels | # 100004 | foo@example.com | NULL | https://osmocom.org/openid/user/777 |
359 | 62 | neels | # 100004 | foo@example.com | NULL | https://projects.osmocom.org/openid/user/777 |
360 | 62 | neels | </pre> |