Review request for JMC-6548: Duplicated Flags rule somewhat naive

[Florian David]

On Tue, Aug 6, 2019 at 5:53 PM Henrik Dafgård <hdafgard at gmail.com> wrote:

> Hi Florian,
>
> The split needs to have that limit there due to the experimental arguments
> with options, i.e. -XX:FlightRecorderOptions=stackdepth=128. With this
> change we would get inconsistent results there instead as the argument
> would be seen as FlightRecorderOptions=stackdepth=128 instead of
> FlightRecorderOptions.
>
> Oh right, I did not think about these ones.


> Also, if an argument matches one in the VERBATIM array it should only be
> compared using the full argument and we won't even run that split on that
> argument. If we want to compare multiple -javaagent commands that share
> jarpaths but might have different options that could be contradictory I
> think we need to have a more detailed solution, i.e. have a check for
> "-javaagent" as a prefix instead of adding it to the VERBATIM array and
> handling those arguments specially.
>
>
After an offline discussion with Henrick, we think it's better to display a
message in case there is duplicate agents ("duplicate" here meaning agents
with the same jar) even if given options are different. This is something
the current patch does not catch, I'll update it accordingly.


>
> Cheers,
> Henrik Dafgård
>
>
> On Tue, 6 Aug 2019 at 14:41, Florian David <florian.david at datadoghq.com>
> wrote:
>
>> Hi,
>>
>> Please find attached a webrev to solve JMC-6548.
>>
>> I have also corrected a small bug on line 123 (String[] split =
>> fullArgument.split("[=:]",
>> 2);). In case fullArgument contains more than one ':' (for instance
>> "-javaagent:C:/mypath"), split[1] does not contain the exact option
>> argument (in this case only "C"), which could give inconsistent results.
>>
>> JIRA: https://bugs.openjdk.java.net/browse/JMC-6548
>> Webrev: http://cr.openjdk.java.net/~fdavid/JMC-6548/webrev.0/
>>
>> Thanks,
>> Florian
>>
>