RFR : 8038435 : Some hgforest.sh commands don't receive parameters

David Katleman david.katleman at oracle.com
Wed Mar 26 23:23:02 UTC 2014

On 3/26/2014 3:39 PM, Mike Duigou wrote:
> I have updated the webrev to add the tclone.
> I also moved the initial clone from non-local source to only the closed repo case. The check isn't necessary for the case where all sub-repos will be relative to the root repo and not in a separate forest.
> http://cr.openjdk.java.net/~mduigou/JDK-8038435/1/webrev/
>> 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.

It's clutter only until you are trying to figure out why you get 
different results when you cut&paste the echo command.
That's just my reasoning for suggesting it, you can leave it out.

New changes look fine, approved.


On Mar 26 2014, at 15:15 , Mike Duigou <mike.duigou at oracle.com> wrote:
>> On Mar 26 2014, at 14:52 , David Katleman <david.katleman at 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

More information about the build-dev mailing list