This looks good to me. I can sponsor the change.
/Erik
On 2019-07-29 04:12, Jaikiran Pai wrote:
I got some more inputs from Stuart Marks on the linked JBS issue[1] and
I've incorporated those in the latest updated webrev which is at [2].
I've tested this latest change locally on mercurial version 5.0.1 and it
has worked as expected (now ignores JTreport and JTwork from the root as
well as sub directories). Plus, with this change I haven't seen any
files/directories that are now being unexpectedly not ignored.
Anyone willing to review and sponsor this please?
[1] https://bugs.openjdk.java.net/browse/JDK-8227170
[2] http://cr.openjdk.java.net/~jpai/webrev/8227170/3/webrev/
-Jaikiran
On 12/07/19 4:04 PM, Jaikiran Pai wrote:
Hello Dean,
Thank you for testing this. I have now updated the webrev to use the
JTreport/* syntax and have also verified that it works for me too. It
correctly ignores both root level JTreport and JTwork directories and
any such sub-directories. I am on a macOS with:
hg --version
Mercurial Distributed SCM (version 5.0.1)
(see https://mercurial-scm.org for more information)
The updated webrev is at
http://cr.openjdk.java.net/~jpai/webrev/8227170/2/webrev/
-Jaikiran
On 11/07/19 1:16 AM, dean.l...@oracle.com wrote:
The ** syntax isn't working for me in Mercurial 2.9, but the following:
syntax: glob
JTreport/*
JTwork/*
seems to work in both 2.9 and 4.6.1. This also seems to work:
syntax: glob
JTreport
JTwork
but it matches files and not just directories.
dl
On 7/10/19 4:48 AM, Jaikiran Pai wrote:
Ping. Anyone willing to review and sponsor this change?
-Jaikiran
On 03/07/19 5:43 PM, Jaikiran Pai wrote:
Hello,
Can I please get a review and a sponsor for the patch for the
enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8227170?
The webrev containing the patch is here
http://cr.openjdk.java.net/~jpai/webrev/8227170/00/webrev/
For some context about this change - there's been some discussion on
this in the jdk-dev list here
https://mail.openjdk.java.net/pipermail/jdk-dev/2019-June/003076.html.
The patch in the webrev, does the following things:
- In addition to ignoring the JTwork and JTreport sub-directories, this
now also ignores these directories at the root of the repo. This patch
uses "syntax: glob" to easily ignore such directories, instead of the
(default) "regex" syntax. In the jdk-dev mailing list discussion,
Gustavo suggested to continue using the regex syntax and listing the
following in the .hgignore:
.*JTreport/.*
.*JTwork/.*
But that would then end up ignoring directories of the form
"foobarJTreport" and files under it like "foobarJTreport/blah.txt". I
guess we can still stick with a regex syntax and come up with a regex
which ignores these directories both at the root as well as
sub-directories, maybe something like:
(.*/JTreport/.*|^JTreport/.*)
(.*/JTwork/.*|^JTwork/.*)
but IMO, that appears a bit too complex when compared to the glob
syntax
that's used in the webrev.
- The other change in this webrev is to ignore the
src/utils/hsdis/build/ directory which gets generated when
src/utils/hsdis is built. I included this in this patch based on
Gustavo's suggestion in that linked thread. I verified that this is
indeed the "build" location used by hsdis tool, by checking the
src/utils/hsdis/Makefile as well src/utils/hsdis/README.
Gustavo also had a suggestion that we ignore the "oprofile_data"
directory/files. But I don't have knowledge of what those files are and
didn't have a way to ascertain the right location/rule to add to the
.hgignore. So I have not included that in the current version of this
webrev. I'm however open to including this in the webrev if Gustavo and
others feel that's OK/needed.
-Jaikiran
