Danek:
Very thank you for your comments,I know I should have finished earlier
but took so long time to know it step by step.
Yes,I copy these code from webrev.I should have changed it.And I should
have removed all debugging information and its code,thank you,Danek.:)
I am modifying the flg.flp scripts and try to send snapshot for xref and
benchmark next day.
Danek Duvall wrote:
> 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.)
>
>
How about this:
if [[ $SCM_MODE == "unknown" ]];then
print -u2 "Unable to determine SCM type currently in use."
print -u2 "For teamware: please looks for \$CODEMGR_WS either in"
print -u2 " the environment or in the file list."
print -u2 "For mercurial: please runs 'hg root'."
exit 1
>> 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.
>
>
Yes,I agree,thanks.
>> 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.)
>
>
I think CODEMGR_WS is environment variable for teamware and it might be
a little bit confusion for mercurial users.
Anyway,I think I am fine with CODEMGR_WS so far.And shall change it anyway.
>> #
>> # 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?
>
>
I deleted it for debugging,should be added.
> 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?
>
I am not sure what you meant?
output the workspace name?
>
>> 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.
>
>
Thanks for telling me.
I made a test,the time take almost the same for "hg locate" and "hg
manifest" in the directory of "hg root",see below.
######### hg locate "glob:$dir/**/${pat#s.}" #########
# time flg.flp -r
......
real 3m38.654s
user 2m41.522s
sys 0m24.115s
######### hg manifest | grep "^$dir/.*/${pat#s.}\$" #######
# time flg.flp -r
......
real 10m11.090s
user 8m2.345s
sys 0m45.250s
For either way,it it really slow for find command.
Then I have an idea,why couldn't we establish a snapshot of all filelist
when the workspace is detected as "mercurial",then try to find the
$dir/${pat#s.} in this snapshot.Is it a better,I am modifying the script
to make a test.Like this:
if [ $SCM_MODE = "mercurial" ]; then
......
FILELIST=`hg manifest | awk '{ print $3 }'`
......
fi
find_files() {
.......
lif [ "$SCM_MODE" = "mercurial" ]; then
for dir; do
if [ -d $codemgr_ws/$dir ]; then
cd $codemgr_ws
echo "$FILELIST" | grep "^$dir/.*/${pat#s.}\$"
cd - > /dev/null
fi
done
fi
.......
}
>> while read srcfile; do
>> if [ "$RELPATHS" != y -o $CURTREE = $codemgr_ws ]; then
>>
>
> Why the change of adding "-o $CURTREE = $codemgr_ws"?
>
>
This is because when I run "flg.flp -r" at $CODEMGR_WS($CURTREE is
$CODEMGR_WS),
In prpath() function:
reltree=${CURTREE##$codemgr_ws/}
reltree always $codemgr_ws
.......
tree=$reltree
while [ "${srcfile##$tree}" = "$srcfile" ]; do
the result always is $srcfile and it is always equal,and it will loop
stopless.
>> 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?
>
>
Yes,I shall change it.
>> 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).
>
yes,I think it may relate to mercurial.
If the SUBTREE donot in the "hg root",it will not run without fault.
Maybe I will change like it.
fail "$SUBTREE is not a subtree of $codemgr_ws"
What do you think?
> Thanks,
> Danek
>
Thanks
Jason
