RFR(xxs): 8227170: (.hg)Ignore the JTwork and JTreport directories generated at the root of the repo

[Erik Joelsson]

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.long at 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
>>>>>
>>>>>