xref:
> if [[ $SCM_MODE == "unknown" ]];then
> print -u2 "Unable to determine SCM type currently in use."
> print -u2 "For teamware: webrev looks for \$CODEMGR_WS either in"
> print -u2 " the environment or in the file list."
> print -u2 "For mercurial: webrev runs 'hg root'."
> exit 1
Why the references to webrev here? (Same applies to flg.flp.)
> elif [[ $SCM_MODE == "mercurial" ]]; then
> hg_root=`hg root 2>/dev/null`
> codemgr_ws=$hg_root
> SRC=$codemgr_ws/usr/src export SRC
> MACH=`uname -p` export MACH
I would set MACH and SRC outside of this if, and get rid of the use of
$hg_root -- just set codemgr_ws directly. Ditto with the teamware clause.
> elif [[ $SCM_MODE == "teamware" ]]; then
> codemgr_ws=$CODEMGR_WS
You set codemgr_ws here, even though CODEMGR_WS is potentially set later in
this clause. But why introduce codemgr_ws at all, instead of just using
CODEMGR_WS throughout? I don't see a reason to avoid that environment
variable in mercurial scenarios. (Same applies to flg.flp.)
> #
> # If $CODEMGR_WS isn't set, then attempt to glean it from the workspace
> # command before giving up.
> #
> if [ -z "$CODEMGR_WS" ]; then
> if whence workspace > /dev/null; then
> #
> # Since ws(1) hasn't been run, set up SRC and MACH too.
> # Note that other environment variables, such as
> # ENVCPPFLAGS*, can also affect the resulting
> # cross-reference, but we assume that if the developer
> # really cared, he would've ws'd first.
> #
> CODEMGR_WS=`workspace name` export CODEMGR_WS
> SRC=$CODEMGR_WS/usr/src export SRC
> MACH=`uname -p` export MACH
> else
> fail "No active workspace; run \"ws <workspace_name>\""
> fi
> fi
>
> [ -d "$CODEMGR_WS" ] || fail "\$CODEMGR_WS ($CODEMGR_WS) is not a
> directory"
> fi
>
> print -u2 " SCM detected: $SCM_MODE"
This needs to be removed before integration (I assume it's just for
debugging).
> make -e -f $XREFMK xref.flg
You removed the "> /dev/null" here. Why?
flg.flp:
> #
> # detect_scm
> #
> # We dynamically test the SCM type; this allows future extensions to
> # new SCM types
> #
> function detect_scm
> {
> #
> # The presence of $CODEMGR_WS and a Codemgr_wsdata directory
> # is our clue that this is a teamware workspace.
> #
> if [[ -n $CODEMGR_WS && -d "$CODEMGR_WS/Codemgr_wsdata" ]]; then
Wouldn't the output of workspace name be more appropriate here?
> elif [ "$SCM_MODE" = "mercurial" ]; then
> for dir; do
> if [ -d $codemgr_ws/$dir ]; then
> cd $codemgr_ws
> hg locate "glob:${dir}/${pat##s.}" | prpath
You just need a single '#' there. But I'm not sure that this does the same
thing as the find command in the teamware clause above. There, if $pat is
"Makefile", then it'll find all Makefiles under $dir. This will simply
find $dir/Makefile.
Getting this working right runs into mercurial bug 108:
http://www.selenic.com/mercurial/bts/issue108
for which there's a workaround. So I think you probably want something
like
hg locate "glob:$dir/**/${pat#s.}"
It's a bit sad that this takes ten or twelve times as long as the find
command. You could use something like
hg manifest | grep "^$dir/.*/${pat#s.}\$"
which is faster, but still not as fast as the find command.
> while read srcfile; do
> if [ "$RELPATHS" != y -o $CURTREE = $codemgr_ws ]; then
Why the change of adding "-o $CURTREE = $codemgr_ws"?
> elif [ "$SCM_MODE" = "teamware" ]; then
> print "Start parsing in Teamware Workspace"
This is just a debug command?
> [ -z "$CODEMGR_WS" ] && fail "No active workspace; run \"ws
> <workspace_name>\""
> codemgr_ws=$CODEMGR_WS
> elif [ "$SCM_MODE" = "mercurial" ]; then
> print "Start parsing in Mercurial Workspace"
Ditto?
> hg_root=`hg root 2>/dev/null`
> codemgr_ws=$hg_root
> SRC=$codemgr_ws/usr/src
Why didn't you set SRC in the teamware clause?
> if [ "${SUBTREE##$codemgr_ws}" = "$SUBTREE" ]; then
> fail "$SUBTREE is not a subtree of Workspace"
> fi
>
> if [ "${CURTREE##$codemgr_ws}" = "$CURTREE" ]; then
> fail "$CURTREE is not a subtree of Workspace"
> fi
These used to read "$SUBTREE is not a subtree of \$CODEMGR_WS"; presumably
this change isn't related to making this work with mercurial, is it? (It
may be a perfectly good change, but should probably fall under a different
bugid).
Thanks,
Danek
