On Mar 26 2014, at 14:52 , David Katleman <david.katle...@oracle.com> wrote:
> Hi Mike, > > A few minor comments, overall looks good. > > On 3/26/2014 2:11 PM, Mike Duigou wrote: >> Hello all; >> >> I introduced bug in JDK-8030681 which prevents non-clone commands from >> receiving parameters. >> >> I also slightly cleaned up checking for what is an acceptable root repo as I >> had encountered this in a problem with a local clone and reverted prior >> change which made the script a bash script. >> >> bug: https://bugs.openjdk.java.net/browse/JDK-8038435 >> >> webrev: http://cr.openjdk.java.net/~mduigou/JDK-8038435/0/webrev/ > >> 116 if [ "${command}" = "clone" -o "${command}" = "fclone" ] ; then >> 221 if [ "${command}" = "clone" -o "${command}" = "fclone" ] ; then > > Perhaps out of scope of this fix, but shouldn't we add -o "${command}" = > "tclone"? > The "fclone" & "tclone" are both cosmetic, neither extensions are used. > Can't remove "fclone" without potentionally breaking scripts. I can easily add the tclone. > >> 127 pull_default_tail=`echo ${pull_default} | sed -e >> 's@^.*://[^/]*/\(.*\)@\1@'` >> 128 if [ "x${pull_default}" = "x${pull_default_tail}" ] ; then >> 129 echo "ERROR: Need initial clone from non-local source" > >> ${status_output} >> 130 exit 1 >> 131 fi > > The above code looks right for what you want to do, but is it necessary to > restrict hgforest.sh to only operate vs non-local? The problem is that the default path is used to figure out the peer name in the extra (closed) repo. It's not possible to do this from a local path unless you make assumptions or we change the requirements for the extra path to include the appropriate root repo. > > Seems one should still be able to have their own forest, and clone from that > forest locally. > >> 238 echo "cd ${i} && hg${global_opts} ${command} ${command_args}" >> > ${status_output} >> 239 cd ${i} && (PYTHONUNBUFFERED=true hg${global_opts} ${command} >> ${command_args}; echo "$?" > ${tmp}/${repopidfile}.pid.rc ) 2>&1 & > > Add "PYTHONUNBUFFERED=true" to the echo line. I am not sure why this would be important. It would seem to be just clutter to me. Thanks! Mike
