RFR: CODETOOLS-7902478 - Add webrev flag to avoid printing workspace (Was: Ability to not display local filesystem path in the generated webrev?)

Thu Jun 13 10:23:06 UTC 2019

Ah, yes of course! Good catch!

The following chunk has been added to the patch:

@@ -27,7 +27,7 @@
 # Documentation is available via 'webrev -h'.
 #

-WEBREV_UPDATED=25.17-hg+openjdk.java.net
+WEBREV_UPDATED=25.18-hg+openjdk.java.net

 HTML='<?xml version="1.0"?>
 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"


Jon, I'll send you an hg export with the complete patch.

Thanks,
/Jesper

> On 13 Jun 2019, at 10:43, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> Hi Jesper,
> 
> I believe you're supposed to bump up the tool version as well:
> 
> http://hg.openjdk.java.net/code-tools/webrev/file/a6f5faf6bf32/webrev.ksh#l30
> 
> best regards,
> 
> -- daniel
> 
> On 12/06/2019 23:18, jesper.wilhelmsson at oracle.com wrote:
>>> On 12 Jun 2019, at 15:02, Jaikiran Pai <jai.forums2013 at gmail.com> wrote:
>>> 
>>> Hello,
>>> 
>>> First time user of the webrev tool - I happened to use it today for one
>>> of the patches I propose for a bug fix. In the generated webrev html
>>> index file, I see that there's a "Workspace:" whose value points to the
>>> local filesystem path of my system. I looked at the options available
>>> for the webrev.ksh script (and even the code in the script) but couldn't
>>> find a way to disable generating this (relatively private) information
>>> in the webrev.
>>> 
>>> Is there a reason to have this information exposed in the webrev? If
>>> not, can a option be added to disable generating this?
>>> 
>>> -Jaikiran
>> It seems to me that this is a relic from a time where all users of webrev was located on the same filesystem. Today I would expect most users to run webrev on a local filesystem and the path to the workspace (and also the path to webrev at the bottom of the page) are of less value. I filed CODETOOLS-7902478 to add a flag to disable this output. The patch below fixes it. Please review.
>> Thanks
>> /Jesper
>> diff --git a/webrev.ksh b/webrev.ksh
>> old mode 100644
>> new mode 100755
>> --- a/webrev.ksh
>> +++ b/webrev.ksh
>> @@ -1812,6 +1812,7 @@
>>          -u <username>: Use that username instead of 'guessing' one.
>>  	-m: Forces the use of Mercurial
>>  	-C: Don't show comments
>> +	-w: Don't display local paths to workspace or webrev installation
>>  Mercurial only options:
>>  	-r rev: Compare against a specified revision
>> @@ -1904,7 +1905,8 @@
>>  Nflag=
>>  Cflag=
>>  forestflag=
>> -while getopts "c:i:o:p:r:u:mONvfbC" opt
>> +print_paths=1
>> +while getopts "c:i:o:p:r:u:mONvfbCw" opt
>>  do
>>  	case $opt in
>>          b)      bflag=1;;
>> @@ -1942,6 +1944,7 @@
>>  	v)	print "$0 version: $WEBREV_UPDATED";;
>> +	w)	print_paths="";;
>>  	?)	usage;;
>>  	esac
>> @@ -2706,7 +2709,9 @@
>>  	print "<tr><th>Prepared by:</th><td>$username $date</td></tr>"
>>  fi
>> +if [[ -n "$print_paths" ]]; then
>>  print "<tr><th>Workspace:</th><td>$CWS</td></tr>"
>> +fi
>>  if [[ -n $parent_webrev ]]; then
>>          print "<tr><th>Compare against:</th><td>"
>>  	print "webrev at $parent_webrev"
>> @@ -2916,7 +2921,12 @@
>>  print
>>  print "<hr />"
>>  print "<p style=\"font-size: small\">"
>> -print "This code review page was prepared using <b>$0</b>"
>> +print "This code review page was prepared using "
>> +if [[ -n "$print_paths" ]]; then
>> +  print "<b>$0</b>"
>> +else
>> +  print "webrev"
>> +fi
>>  print "(vers $WEBREV_UPDATED)."
>>  print "</body>"
>>  print "</html>"
> 

