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 | "cvs import" is a fragile system with lots of flaws and corner cases. |
---|
152 | Fortunately, we have a script to make imports convenient and mostly |
---|
153 | error-free. It lives in $CVSROOT/CVSROOT/import.sh, and is documented |
---|
154 | briefly in comments at the top of the script. Here are some use |
---|
155 | cases: |
---|
156 | |
---|
157 | # Import a nicely named tarfile. |
---|
158 | $CVSROOT/CVSROOT/import.sh /tmp/bash-2.03.tar.gz 2.02 |
---|
159 | |
---|
160 | # Import a not so nicely named tarfile. |
---|
161 | $CVSROOT/CVSROOT/import.sh -n pine -v 4.53 /tmp/pine4.53.tar.bz2 4.33 |
---|
162 | |
---|
163 | # Import into a non-default repository directory. |
---|
164 | $CVSROOT/CVSROOT/import.sh -d third/glib2 /tmp/glib-2.2.1.tar.bz2 2.2.0 |
---|
165 | |
---|
166 | # Import a package for the first time. |
---|
167 | $CVSROOT/CVSROOT/import.sh /tmp/gnome-shiny-0.0.1.tar.gz "" |
---|
168 | |
---|
169 | After the initial import, here are some things to pay attention to |
---|
170 | when adding or updating a piece of third-party software: |
---|
171 | |
---|
172 | * If the package's build system does not use autoconf, you will |
---|
173 | probably need to write a Makefile.athena file telling the build |
---|
174 | system how to build it. |
---|
175 | |
---|
176 | * If the package's build system does use autoconf, you may need to |
---|
177 | write a configure.athena giving special options to pass to the |
---|
178 | configure script. |
---|
179 | |
---|
180 | * A few packages will need to be taught how to use DESTDIR. Make |
---|
181 | sure that DESTDIR references don't make it into the installed |
---|
182 | program. |
---|
183 | |
---|
184 | * If the package installs a file setuid, it needs to specify the |
---|
185 | owner (probably "-o root" if it didn't specify one before). |
---|
186 | Likewise, a setgid program needs a specified group owner, although |
---|
187 | this is usually done already. Other than that, our fix_owners |
---|
188 | program will coerce unspecified owners and groups to 0. |
---|
189 | |
---|
190 | * The package should create directories before installing files in |
---|
191 | them. |
---|
192 | |
---|
193 | * If possible, test your package's build and install. Preferrably, |
---|
194 | use the "do" command, something like: |
---|
195 | |
---|
196 | do dist |
---|
197 | do prepare |
---|
198 | do |
---|
199 | do -d /var/tmp/inst install |
---|
200 | |
---|
201 | (Replace "do" with "sh /mit/source/packs/build/do.sh" or use a |
---|
202 | shell alias.) This testing may not be possible if the package |
---|
203 | depends on other packages not in the current Athena release. |
---|