On Thu, 18 Nov 2021 09:10:16 GMT, Andrew Leonard <aleon...@openjdk.org> wrote:

>> src/jdk.jartool/share/classes/sun/tools/jar/Main.java line 131:
>> 
>>> 129:     // There's also a files array per version
>>> 130:     // Use a LinkedHashMap to keep original insertion ordering
>>> 131:     Map<Integer,String[]> filesMap = new LinkedHashMap<>();
>> 
>> The entries of `filesMap` is sorted by the version.   Is the value of each 
>> entry already sorted?  Does that matter?   It's unclear to me since `expand` 
>> is called for each entry's value:
>> 
>> 
>>     private void expand() throws IOException {
>>         for (int version : filesMap.keySet()) {
>>             String[] files = filesMap.get(version);
>>             expand(null, files, pathsMap.get(version), version);
>>         }
>>     }
>
> @mlchung the way I looked at this was the filesMap is a HashMap, so the 
> filesMap.keySet() iteration order is not defined (or necessarily 
> deterministic), the expanded order of the file sets for each version could be 
> ver1, ver2, ver3 or ver3, ver2, ver1... the next time you run it with the 
> same parameters.
> LinkedHashMap will ensure the iterated order is as inserted (or as specified 
> in the jar args list..)
> 
> Although you say "filesMap is sorted by version", I don't see where that is 
> done?

I'm guessing Mandy asks not if the keys are sorted -- this is changed by your 
patch, but if the value retrieved -- the String[], is sorted. It's a valid 
question, I think. It's not apparent to me at least that this is the case. If 
it is not sorted when it is inserted it needs to be sorted when extracted.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6395

Reply via email to