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.
Thanks
Dave
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