This is not a commit to be merged, but rather to test on a much..

[c-blake]

..broader scale if any tests break with the new string hash.

[c-blake]

Sorry for the delay. I only just learned a few hours ago on Nim Internals that a release might be "imminent". I thought we had maybe the whole Summer.

As shown by that benchmark prog over on the initial Issue thread, all 3 hashes (aboriginal hashData, murmur, new Farm) peacefully co-exist in the ever larger std/hashes.nim. So, the worst back.incompat. thing will be "for Nim-2.2, if you rely on hash order (you should try not to when possible!) then you can add --define:xyz to your nim.cfg" or words to that effect. { Well, depending on the breakage. ;-) }

[Araq]

Well almost everything is red. ;-)

[c-blake]

Across all the deployments, things seem mostly ok to me. I saw one chronos failure obviously related to explicit hash-order test. A DataMancer and a zippy. But I am not as practiced at reading these failure reports as you, to be sure.

[c-blake]

Oh, and I should have said that DataMancer thing might have to do with const Table vs. run-time Table. Maybe we also need to patch compiler/nim.cfg?

[Vindaar]

A DataMancer

Fixed in new version I just tagged (only related to comparison of table keys, where the order changed compared to what the test assumed).

[c-blake]

FWIW, I raised an issue over at the chronos repo.

[c-blake]

Oh, also, I should have said - I cloned zippy locally. nimble test fails with |without nimPreviewHashFarm on both master and 0.10.12. So, it seems unrelated/a flaky test suite, and I might guess it is failing for other PRs around now. So, I think that chronos one which is just an obvious hash-order dependent test is the only blocker.

[c-blake]

So, I did #23791 which should fix one failure, Vindaar fixed his, I opened an issue with chronos (& a PR fixing it to be merged before its next release). I don't see why the zippy one would fail, but studying its large test suite seemed hard. I did reproduce it locally { after tracking down another test failure related to an LC_ALL=C I had set } and raised a zippy issue. Cross-linking all that here on this test-PR for convenience.

A lingering question I see is what to do about --jsbigint64:off tests. @ringabout sez --jsbigint64:on has long been the default. I don't know when/if there is a plan to retire support for --jsbigint64:off or even what level of compatibility support is promised in that mode. E.g., one solution might be to test for --jsbigint64:off and use the old string hash in that case (not matching hashes across that mode switch).

[guzba]

Oh, also, I should have said - I cloned zippy locally. nimble test fails with|without nimPreviewHashFarm on both master and 0.10.12.

Is this on Nim devel? This test passes locally and in CI across many Nim releases and OSes: https://github.com/guzba/zippy/actions/runs/9782117226/job/27007969654 (ignore the annoying jiro4989/setup-nim-action@v1 issue I now need to investigate)

[c-blake]

Apologies - it only fails for me (yes, on nim-devel) locally (EDIT: and in the CI here) with nimPreviewHashFarm defined once I have a UTF8 locale. The locale-sensitive failure temporarily confused me.