Add missing LANG=C for make sort - #766
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds LANG=C to two sort commands in the Makefile to ensure consistent, byte-wise sorting. This is a good change for build reproducibility.
However, I've found a critical issue with how the sorted files are updated. The current pattern of redirecting output to a temporary file and then moving it is not atomic. If the sort pipeline fails, the original file could be overwritten with an empty or incomplete one, leading to data loss. I've added suggestions to fix this for the changed lines by chaining commands with && and using unique temporary filenames.
This unsafe pattern is present in other parts of the sort target as well (lines 52-59). I strongly recommend applying the same fix to all of them to ensure the Makefile is robust.
Additionally, for consistency, you might want to check other sort commands in this file (e.g., in the check target on lines 65-66) and add LANG=C to them as well, to ensure all sorting operations behave identically.
There was a problem hiding this comment.
Pull request overview
This PR adds env LANG=C to the sort commands for phrase.occ and BPMFMappings.txt in the Makefile to ensure files are sorted consistently using C locale ordering, which is critical for the dictionary data files.
- Updates two sort commands to use
env LANG=Cto enforce C locale sorting - Aligns these commands with the existing pattern used for other file sorts in the same target (lines 52-59)
This makes sure that the files are indeed sorted according to the UTF-8 order.
fc59b70 to
7ccfa06
Compare
This makes sure that the files are indeed sorted according to the UTF-8 order.