RFR webrev: 7900311 webrev -r doesn't produce an exported changeset file

[Magnus Ihse Bursie]

On 2014-02-12 03:00, Stuart Marks wrote:
> Hi, is there someone who can push this changeset for me?
Sorry, I can't help you. :-( I don't have committer rights to webrev, 
since code-tools committer rights is required for that, which I don't have.

I'm cross-posting to code-tools-dev to see if someone there can step up.

/Magnus


>
> Thanks.
>
> s'marks
>
> On 1/30/14 1:28 PM, Stuart Marks wrote:
>> I've updated the version number and the copyright year as well. The 
>> patch has
>> now tripled in size. :-)
>>
>> Updated webrev:
>>
>> http://cr.openjdk.java.net/~smarks/reviews/7900311/webrev.1/
>>
>> s'marks
>>
>> On 1/30/14 1:03 PM, Mike Duigou wrote:
>>>
>>> On Jan 29 2014, at 17:47 , Stuart Marks <stuart.marks at oracle.com> 
>>> wrote:
>>>
>>>> I was frustrated today because webrev was producing a plain patch file
>>>> instead of an exported changeset file. Then I realized I had run 
>>>> across this
>>>> before. See message appended below. (Probably dropped because of 
>>>> the holidays.)
>>>>
>>>> I've filed
>>>>
>>>>     https://bugs.openjdk.java.net/browse/CODETOOLS-7900311
>>>>
>>>> for this. I've also run my modified webrev.ksh over this patch to 
>>>> webrev.ksh
>>>> and have uploaded a webrev here:
>>>>
>>>> http://cr.openjdk.java.net/~smarks/reviews/7900311/webrev.0/
>>>>
>>>> It's basically the same one-line change shown below. I don't think 
>>>> I have
>>>> commit rights to the webrev repo; can someone push this for me?
>>>
>>> Please bump the version number.
>>>
>>>>
>>>> Two observations, independent of this change:
>>>>
>>>> a) Even though this is a one-line change, the sdiff and frames 
>>>> views show a
>>>> lot of spurious segments of the file, with no apparent changes.
>>>
>>> Quite odd. The udiff is in contrast nicely compact as is the actual 
>>> changeset.
>>>
>>>>
>>>> b) The link in the changeset comment points to JDK-7900311, which 
>>>> doesn't
>>>> exist, since the bug is actually CODETOOLS-7900311.
>>>
>>> The -c option needs to be extended to understand full bug ids.
>>>
>>>>
>>>> Thanks,
>>>>
>>>> s'marks
>>>>
>>>>
>>>>
>>>> -------- Original Message --------
>>>> Subject: bug in webrev? export vs patch
>>>> Date: Sat, 21 Dec 2013 15:21:29 -0800
>>>> From: Stuart Marks <stuart.marks at oracle.com>
>>>> To: webrev-dev at openjdk.java.net
>>>> CC: Mike Duigou <mike.duigou at oracle.com>
>>>>
>>>> Hi all,
>>>>
>>>> My typical workflow is to keep the changes I'm working on in MQ, 
>>>> and then
>>>> generate a webrev based on the tipmost applied patch. Typically 
>>>> this is the only
>>>> applied patch, so I use a command like
>>>>
>>>>     webrev -N -r qparent
>>>>
>>>> to generate a webrev from this patch. Unfortunately, in this mode, 
>>>> webrev
>>>> doesn't export the hg changeset, instead it just generates a patch. 
>>>> There's an
>>>> actual changeset present, so it should export it, so I think that's 
>>>> a bug.
>>>>
>>>> (I guess I should file a CODETOOLS bug on this.)
>>>>
>>>> What to do about this? Well, I think there's a case in handling the 
>>>> -r flag
>>>> where a bit more of the global state needs to be set. It correctly 
>>>> picks up the
>>>> changeset comment but doesn't set the flag that eventually causes 
>>>> the changeset
>>>> to be exported instead of a patch to be generated.
>>>>
>>>> I've tried this:
>>>>
>>>>
>>>> diff -r 9eab6a0ae4b5 webrev.ksh
>>>> --- a/webrev.ksh    Fri Nov 08 09:36:55 2013 +0100
>>>> +++ b/webrev.ksh    Sat Dec 21 15:11:40 2013 -0800
>>>> @@ -2008,6 +2008,7 @@
>>>>              #
>>>>              FIRST_CREV=`hg log --rev $PARENT_REV --template '{rev}'`
>>>>              FIRST_CREV=`expr $FIRST_CREV + 1`
>>>> +            HG_LIST_FROM_COMMIT=1
>>>>          fi
>>>>      fi
>>>>      #Let's check if a merge is needed, if so, issue a warning
>>>>
>>>>
>>>> I haven't analyzed all the ways the various global variables are 
>>>> used though.
>>>> However, using this patch "works for me" ....
>>>>
>>>> s'marks
>>>>
>>>>
>>>