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 | |
---|
7 | Checking out a working directory |
---|
8 | Preparing changes for review |
---|
9 | Reviewing changes |
---|
10 | Early checkins |
---|
11 | Third-party sources |
---|
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 |
---|
28 | co -P all", but in almost all cases this would be a big waste of space. |
---|
29 | Simply check out a subdirectory of the source tree with a command like |
---|
30 | "cvs co -P athena/bin/olc". |
---|
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 | |
---|
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 | |
---|
40 | Preparing changes for review |
---|
41 | ---------------------------- |
---|
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 | |
---|
47 | For changes to other parts of the tree, you should perform the |
---|
48 | following steps while preparing your changes for review: |
---|
49 | |
---|
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.) |
---|
54 | |
---|
55 | 2. Be sure to test your changes. |
---|
56 | |
---|
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. |
---|
65 | |
---|
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. |
---|
72 | |
---|
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 |
---|
77 | |
---|
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. |
---|
83 | |
---|
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. |
---|
88 | |
---|
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 |
---|
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. |
---|
98 | |
---|
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 |
---|
101 | obvious. |
---|
102 | |
---|
103 | Reviewing changes |
---|
104 | ----------------- |
---|
105 | |
---|
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 | |
---|
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 |
---|
125 | try out changes before signing onto them, but in some cases the |
---|
126 | inconvenience outweights the benefit of this consideration. |
---|
127 | |
---|
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 | |
---|
135 | 1. The change is obvious and noncontroversial, such as a fix for a |
---|
136 | syntax error. |
---|
137 | |
---|
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." |
---|
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 | |
---|
148 | Third-party sources |
---|
149 | ------------------- |
---|
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 |
---|
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. |
---|
161 | |
---|
162 | Generally, you should only import "clean" third-party source trees |
---|
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. |
---|
167 | |
---|
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 | |
---|
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 | |
---|
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 | |
---|
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 | |
---|
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 |
---|
193 | cvs import -I \! -d -m "Import emacs 22.3." third/emacs vendor emacs-22_3 |
---|
194 | |
---|
195 | Make sure to use the literal word "vendor" for the vendor branch name. |
---|
196 | |
---|
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 | |
---|
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." |
---|
206 | |
---|
207 | It is important to remove such files on the vendor branch, so that |
---|
208 | diffs against the vendor branch will work properly. |
---|
209 | |
---|
210 | Now you can do the merge. Here is an example of how you might do one: |
---|
211 | |
---|
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." |
---|
219 | |
---|
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, |
---|
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 |
---|
225 | involved. Try using "cvs admin -bvendor filename" and then "cvs |
---|
226 | update filename" to make the file go away more naturally. |
---|
227 | |
---|
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. |
---|
232 | |
---|
233 | Here are some things to pay attention to when adding or updating a |
---|
234 | piece of third-party software: |
---|
235 | |
---|
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. |
---|
239 | |
---|
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. |
---|
243 | |
---|
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. |
---|
247 | |
---|
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. |
---|
253 | |
---|
254 | * The package should create directories before installing files in |
---|
255 | them. |
---|
256 | |
---|
257 | * Test your package's build and install. Preferrably, use the "do" |
---|
258 | command, something like: |
---|
259 | |
---|
260 | do prepare |
---|
261 | do clean |
---|
262 | do |
---|
263 | do check |
---|
264 | do -d /var/tmp/inst install |
---|
265 | |
---|
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. |
---|