[8979] | 1 | This file contains a description of the process developers should go |
---|
| 2 | through to get changes into the source tree. Although it discusses |
---|
| 3 | the use of CVS, it is not a CVS tutorial; read the CVS info pages |
---|
| 4 | (available in M-x info in emacs on Athena) for a general introduction |
---|
| 5 | to CVS. Areas covered in this file are: |
---|
| 6 | |
---|
[18524] | 7 | Checking out a working directory |
---|
| 8 | Preparing changes for review |
---|
| 9 | Reviewing changes |
---|
| 10 | Early checkins |
---|
| 11 | Third-party sources |
---|
[8979] | 12 | |
---|
| 13 | You should use cvs from the gnu locker with the source repository. |
---|
| 14 | People without write access to the repository can use "cvs -u" (a |
---|
| 15 | local modification to CVS) to access the repository without making |
---|
| 16 | read locks. If you do not have write access to the repository and you |
---|
| 17 | want to submit a change, follow the guidelines below up and including |
---|
| 18 | sending mail to source-reviewers, and note in your mail that your |
---|
| 19 | reviewer should check in the change because you cannot do so. |
---|
| 20 | |
---|
| 21 | Checking out a working directory |
---|
| 22 | -------------------------------- |
---|
| 23 | |
---|
| 24 | Set CVSROOT to "/afs/dev.mit.edu/source/repository" before trying to |
---|
| 25 | check out a working directory. |
---|
| 26 | |
---|
| 27 | The entire source tree is very large. You can check it out with "cvs |
---|
[13857] | 28 | co -P all", but in almost all cases this would be a big waste of space. |
---|
[8979] | 29 | Simply check out a subdirectory of the source tree with a command like |
---|
[13857] | 30 | "cvs co -P athena/bin/olc". |
---|
[8979] | 31 | |
---|
| 32 | CVS knows nothing about AFS permissions, so all directories created |
---|
| 33 | will have the same permissions as their parent. It is generally |
---|
| 34 | safest to do your checkouts in a private area of the filesystem. |
---|
| 35 | |
---|
[13857] | 36 | You should use the -P option for checkout because the source tree |
---|
| 37 | contains some historical directories (now empty) which will conflict |
---|
| 38 | with builds. |
---|
| 39 | |
---|
[9000] | 40 | Preparing changes for review |
---|
| 41 | ---------------------------- |
---|
[8979] | 42 | |
---|
| 43 | Changes to the doc hierarchy do not typically need to be reviewed; |
---|
| 44 | notification is typically good enough, since no software will break as |
---|
| 45 | a result of changes to the source tree documentation. |
---|
| 46 | |
---|
[9000] | 47 | For changes to other parts of the tree, you should perform the |
---|
| 48 | following steps while preparing your changes for review: |
---|
[8979] | 49 | |
---|
[18524] | 50 | 1. Do a "cvs update" in your working directory to merge in changes |
---|
| 51 | other people may have made. (You can do "cvs -n update" if you |
---|
| 52 | want to see what needs to be merged in without actually doing the |
---|
| 53 | merge.) |
---|
[8979] | 54 | |
---|
[18524] | 55 | 2. Be sure to test your changes. |
---|
[8979] | 56 | |
---|
[18524] | 57 | 3. Make sure your changes are made in reviewable chunks to the |
---|
| 58 | greatest extent possible. If you have many changes to make of |
---|
| 59 | several different types, prepare one patch for each type of |
---|
| 60 | change; in particular, if you have some cosmetic changes to make |
---|
| 61 | and some functional fixes to make, submit them as two different |
---|
| 62 | patches if they add up to a significant number of changes. This |
---|
| 63 | requirement creates more work for the submitter, but it greatly |
---|
| 64 | increases the effectiveness of the review process. |
---|
[13283] | 65 | |
---|
[18524] | 66 | 4. Use "cvs diff -u -N" piped to a file to prepare your changes. (Do |
---|
| 67 | not cut and paste diffs from an xterm; your tabs will be |
---|
| 68 | converted to spaces.) If your change involves reindentation of |
---|
| 69 | code, you may want to also use the "-w" flag to diff. If you |
---|
| 70 | find that your change is clearer when presented as a context diff |
---|
| 71 | ("-c" instead of "-u"), feel free to submit it that way. |
---|
[9000] | 72 | |
---|
[18524] | 73 | 5. Look over your diffs. Make sure you haven't been sloppy about |
---|
| 74 | spacing, punctuation, and naming, and that you have tried to |
---|
| 75 | conform to the guidelines in the file "standards" in this |
---|
| 76 | directory |
---|
[9000] | 77 | |
---|
[18524] | 78 | 6. Send your diffs, along with a clear description of the change you |
---|
| 79 | are making, to source-reviewers@mit.edu. If the diffs are very |
---|
| 80 | large (more than 50K), put the changes somewhere world-readable |
---|
| 81 | (unless the source code in question is restricted) and mail a |
---|
| 82 | pointer. |
---|
[9000] | 83 | |
---|
[18524] | 84 | 7. If you do not have write access to the source tree and submitted |
---|
| 85 | your diff using the -w flag, submit it again without the -w flag |
---|
| 86 | so that the full patch can be checked in by someone with write |
---|
| 87 | access. |
---|
[13283] | 88 | |
---|
[8979] | 89 | Ideally, at least one person will respond to your mail within a day or |
---|
| 90 | two, either expressing concerns or signing onto your change. You |
---|
[13049] | 91 | should wait at least one day for people to voice their objections. If |
---|
| 92 | you receive objections or requests for further information from staff |
---|
| 93 | members, you must either satisfy those concerns or resolve the issue |
---|
| 94 | with the release team before committing your change. If after one |
---|
| 95 | day, you have received no objections and someone has signed onto your |
---|
| 96 | change, you may commit your change. You may also commit your change |
---|
| 97 | if no one objects within five days, even if no one has signed onto it. |
---|
[8979] | 98 | |
---|
[9000] | 99 | When you check in your change, be sure to include a clear log message. |
---|
| 100 | Explain why you are making the change you are making if it's not |
---|
[9707] | 101 | obvious. |
---|
[9000] | 102 | |
---|
| 103 | Reviewing changes |
---|
| 104 | ----------------- |
---|
| 105 | |
---|
[9484] | 106 | Sometimes you can review a change by looking at the patch. Other |
---|
| 107 | times you will want to check out a tree and apply the patch, with |
---|
| 108 | "patch -E -p < message-file" if you have the mail message in a file, |
---|
| 109 | or "dsgrep -p -t trn-number source-reviewers | patch -E -p" if what |
---|
| 110 | you have is a transaction number in the source-reviewers discuss |
---|
| 111 | meeting. |
---|
| 112 | |
---|
[8979] | 113 | When reviewing a change, be sure to make your position on the change |
---|
| 114 | clear. Say "I object to this change" if you are not merely voicing a |
---|
| 115 | concern, or "I would like these questions answered before this change |
---|
| 116 | is committed" if you have asked questions and are not merely curious. |
---|
| 117 | When your objections are responded to, you should in turn respond in a |
---|
| 118 | timely fashion saying whether your objections have been satisfied or |
---|
| 119 | not. If the dispute appears intractable, say so, so that the issue |
---|
| 120 | may be brought up before the release team. |
---|
| 121 | |
---|
| 122 | If you have reviewed a change carefully and have found nothing wrong |
---|
| 123 | with it, and no one else has responded to the change, you should sign |
---|
| 124 | onto the change rather than remaining silent. You are encouraged to |
---|
[9484] | 125 | try out changes before signing onto them, but in some cases the |
---|
| 126 | inconvenience outweights the benefit of this consideration. |
---|
[8979] | 127 | |
---|
[12753] | 128 | Early checkins |
---|
| 129 | -------------- |
---|
| 130 | |
---|
| 131 | In some cases it may be appropriate to check in a change in advance of |
---|
| 132 | the normal review period. The following should be true of those |
---|
| 133 | cases: |
---|
| 134 | |
---|
[18524] | 135 | 1. The change is obvious and noncontroversial, such as a fix for a |
---|
| 136 | syntax error. |
---|
[12753] | 137 | |
---|
[18524] | 138 | 2. The problem being fixed is causing an immediate difficulty, |
---|
| 139 | usually "I'm doing a build of /mit/source and it blows out at |
---|
| 140 | this point." |
---|
[12753] | 141 | |
---|
| 142 | The change should still be sent to source-reviewers with a note about |
---|
| 143 | the early checkin. If the immediate difficulty is "the wash is broken |
---|
| 144 | and I want the next wash to work," then it is good to get a positive |
---|
| 145 | review of the change before checking it in. Close to a release cycle, |
---|
| 146 | though, that can be ignored. |
---|
| 147 | |
---|
[13049] | 148 | Third-party sources |
---|
| 149 | ------------------- |
---|
[8980] | 150 | |
---|
| 151 | For modules in the third hierarchy, we generally use the "cvs import" |
---|
| 152 | command to track development from outside. (To find out if this |
---|
| 153 | applies to a given module, to a "cvs log" of a file in the tree; if |
---|
[10518] | 154 | you see a revision 1.1.1.1, then we're using cvs import.) Do not |
---|
| 155 | check in a new outside version of a third-party package onto the |
---|
| 156 | mainline if it was originally imported with cvs import; it's very |
---|
| 157 | difficult to recover from that particular mistake. Do, however, check |
---|
| 158 | local changes you made yourself onto the mainline. Always refer to |
---|
| 159 | doc/third-party before doing an import to see if there are any special |
---|
| 160 | notes on the module you are importing. |
---|
[8980] | 161 | |
---|
| 162 | Generally, you should only import "clean" third-party source trees |
---|
[9000] | 163 | with no modifications. If you absolutely need to make changes to the |
---|
| 164 | source tree before importing it (check with a release engineer before |
---|
| 165 | deciding that you have to), make a note in the doc/third-party file so |
---|
| 166 | that people doing future imports will know about it. |
---|
[8980] | 167 | |
---|
[15015] | 168 | Before doing an import, run timestamps.pl from the repository CVSROOT |
---|
| 169 | directory. This script helps work around the problem that CVS does |
---|
| 170 | not version metadata such as timestamps, by crunching all of the |
---|
| 171 | timestamps up near the current time while preserving their relative |
---|
| 172 | order. Use the "-d" flag to cvs import to use the resulting |
---|
| 173 | timestamps as the time of import. |
---|
| 174 | |
---|
[15291] | 175 | Also before doing an import, remove any .cvsignore files which might |
---|
| 176 | be present in the tree. We are builders, not developers, of |
---|
| 177 | third-party software, and want to track the entire distribution |
---|
| 178 | including any auto-generated files. |
---|
| 179 | |
---|
[18525] | 180 | It is not uncommon for packages to include files and directories which |
---|
| 181 | match the CVS default ignores list (directories named "core", |
---|
| 182 | filenames ending in ".old", etc.). Use the "-I \!" option to cvs |
---|
| 183 | import to avoid losing files. |
---|
| 184 | |
---|
[15015] | 185 | Here is an example of how one might import a hypothetical new version |
---|
| 186 | of emacs (assuming the CVSROOT environment variable is set to point at |
---|
| 187 | the Athena repository): |
---|
| 188 | |
---|
[18524] | 189 | gtar xzf emacs-22.3.tar.gz |
---|
| 190 | cd emacs-22.3 |
---|
| 191 | find . -name .cvsignore -print | xargs rm |
---|
| 192 | perl $CVSROOT/CVSROOT/timestamps.pl |
---|
[18525] | 193 | cvs import -I \! -d -m "Import emacs 22.3." third/emacs vendor emacs-22_3 |
---|
[15015] | 194 | |
---|
[18294] | 195 | Make sure to use the literal word "vendor" for the vendor branch name. |
---|
[16910] | 196 | |
---|
[16682] | 197 | After importing, you should take care of files which have been removed |
---|
| 198 | from one version to the next, since CVS doesn't do a good job. Here |
---|
| 199 | is an example: |
---|
| 200 | |
---|
[18524] | 201 | cd /tmp |
---|
| 202 | cvs co -r vendor third/emacs |
---|
| 203 | cd third/emacs |
---|
| 204 | cvs update -j emacs-22_2 -j emacs-22_3 |
---|
| 205 | cvs ci -m "Not present in emacs 22.3." |
---|
[16682] | 206 | |
---|
| 207 | It is important to remove such files on the vendor branch, so that |
---|
[17049] | 208 | diffs against the vendor branch will work properly. |
---|
[16682] | 209 | |
---|
[17049] | 210 | Now you can do the merge. Here is an example of how you might do one: |
---|
| 211 | |
---|
[18524] | 212 | cd /tmp |
---|
| 213 | rm -rf third/emacs |
---|
| 214 | cvs co -kk third/emacs |
---|
| 215 | cd third/emacs |
---|
| 216 | cvs update -kk -j emacs-22_2 -j emacs-22_3 |
---|
| 217 | # Resolve conflicts. |
---|
| 218 | cvs ci -m "Merge with emacs 22.3." |
---|
[17049] | 219 | |
---|
[17125] | 220 | Using a "-kk" working directory and merge will prevent merge conflicts |
---|
| 221 | based on RCS IDs; it is unnecessary for a package which does not use |
---|
| 222 | RCS keyword strings. If the merge results in any files being deleted, |
---|
[17049] | 223 | check those files (with "cvs log") to make sure they were in fact |
---|
| 224 | modified on the mainline. If they weren't, then some CVS oddity is |
---|
[18294] | 225 | involved. Try using "cvs admin -bvendor filename" and then "cvs |
---|
| 226 | update filename" to make the file go away more naturally. |
---|
[17049] | 227 | |
---|
[8979] | 228 | If you add a new piece of third-party software or import a new |
---|
| 229 | version, you should look over doc/third-party and see if any notes |
---|
| 230 | should be added or modified. This file is instrumental in locating |
---|
| 231 | new versions of software. |
---|
[13049] | 232 | |
---|
| 233 | Here are some things to pay attention to when adding or updating a |
---|
| 234 | piece of third-party software: |
---|
| 235 | |
---|
[18524] | 236 | * If the package's build system does not use autoconf, you will |
---|
| 237 | probably need to write a Makefile.athena file telling the build |
---|
| 238 | system how to build it. |
---|
[13049] | 239 | |
---|
[18524] | 240 | * If the package's build system does use autoconf, you may need to |
---|
| 241 | write a configure.athena giving special options to pass to the |
---|
| 242 | configure script. |
---|
[13049] | 243 | |
---|
[18524] | 244 | * Most packages will need to be taught how to use DESTDIR. Make |
---|
| 245 | sure that DESTDIR references don't make it into the installed |
---|
| 246 | program. |
---|
[13049] | 247 | |
---|
[18524] | 248 | * If the package installs a file setuid, it needs to specify the |
---|
| 249 | owner (probably "-o root" if it didn't specify one before). |
---|
| 250 | Likewise, a setgid program needs a specified group owner, although |
---|
| 251 | this is usually done already. Other than that, our fix_owners |
---|
| 252 | program will coerce unspecified owners and groups to 0. |
---|
[13049] | 253 | |
---|
[18524] | 254 | * The package should create directories before installing files in |
---|
| 255 | them. |
---|
[13049] | 256 | |
---|
[18524] | 257 | * Test your package's build and install. Preferrably, use the "do" |
---|
| 258 | command, something like: |
---|
[13049] | 259 | |
---|
[18524] | 260 | do prepare |
---|
| 261 | do clean |
---|
| 262 | do |
---|
| 263 | do check |
---|
| 264 | do -d /var/tmp/inst install |
---|
[13049] | 265 | |
---|
[18524] | 266 | (Replace "do" with "sh /mit/source/packs/build/do.sh" or use a |
---|
| 267 | shell alias.) If the package relies on libraries, you can use "-c |
---|
| 268 | -d /afs/dev.mit.edu/system/<sysname>/srvd-current" in the |
---|
| 269 | non-install steps to point at them. |
---|