Overall it looks good, but I have a few concerns about findunref.py.
First, I gave up trying to understand walk_scm_hg(). I don't know the
Mercurial API well enough, and I couldn't find any real documentation on
the website. I tried poking around in the Mercurial source, but the
comments are sufficiently sparse that I figured it would take me the
rest of the afternoon to figure it all out. For example, I have no idea
if repo.status() (no arguments) gives you what you need. Putting some
comments in walk_scm_hg() would help.
Second, I wonder if symbolic links or lofs paths could give you
problems. Test case: the user specifies "/home/kupfer/ws/foo", which is
really a symbolic link to /net/somehost/export/kupfer/ws/foo. We want
this to work on both somehost and on other hosts. Note that
localrepo.root is generated using os.path.realpath().
Third, I found the code overall to be a bit sparsely commented. For
example, I think it would be helpful to document that
ExceptionList.match() takes a glob pattern.
Nits:
* checkpaths.sh:
- line 105: continuation line should only be indented by 4 spaces
* exception_list.open:
- lines 8, 14: "ususr" -> "usr"
* findunref.py:
- Should '.hgignore' be in the built-in exception list?
- the error exits all exit with status code 2, whereas I think the C
version exits with code 1. It's not documented anywhere what it
*should* exit with, so hopefully no scripts are depending on a
specific value. But I don't think we should use a different value
unless we have a good reason.
- line 59: why not just line.strip()?
>>>>> "stevel" == Stephen Lau <stevel at sun.com> writes:
stevel> Other notes:
stevel> o nightly.sh:2795, you can see where I pass the findunref output
stevel> through sed to get the output to look like the form of the old
stevel> findunref output, that is to say, the usr/closed output will
stevel> look like:
stevel> ../closed/cmd/foo
stevel> and the usr/src output will look like:
stevel> ./uts/foo
stevel> I'm open to changing that. I just made it that way to
stevel> maintain uniformity for now. I personally think it makes more
stevel> sense to have it be usr/closed/cmd/foo and usr/src/cmd/foo,
stevel> but whatever. Your comments on this point would be
stevel> appreciated.
The unref-*.out files are structured that way so that `cat
unref<mumble>.out` will give you a file list that you can use from the
same directory (e.g., ls -l `cat ...`). I don't know if anyone takes
advantage of that, but I'd rather not throw it away without good reason.
cheers,
mike
