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

Daniel Fuchs daniel.fuchs at oracle.com
Thu Jun 13 08:43:25 UTC 2019


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>"
> 
> 



More information about the webrev-dev mailing list