Make sure LC_ALL=C is set in all shell scripts #13454

pull practicalswift wants to merge 2 commits into bitcoin:master from practicalswift:avoid-locale-dependent-range-expressions changing 27 files +58 −1
  1. practicalswift commented at 5:55 am on June 13, 2018: contributor

    Make sure LC_ALL=C is set when using grep range expressions.

    Make sure LC_ALL=C is set in all shell scripts.

    From the grep(1) documentation:

    Within a bracket expression, a range expression consists of two characters separated by a hyphen. It matches any single character that sorts between the two characters, inclusive, using the locale’s collating sequence and character set. For example, in the default C locale, [a-d] is equivalent to [abcd]. Many locales sort characters in dictionary order, and in these locales [a-d] is typically not equivalent to [abcd]; it might be equivalent to [aBbCcDd], for example. To obtain the traditional interpretation of bracket expressions, you can use the C locale by setting the LC_ALL environment variable to the value C.

    Context: [Locale issue found when reviewing #13450](https://github.com/bitcoin/bitcoin/pull/13450/files#r194877736)

  2. practicalswift force-pushed on Jun 13, 2018
  3. fanquake added the label Scripts and tools on Jun 13, 2018
  4. laanwj commented at 10:46 am on June 13, 2018: member
    Maybe export LC_ALL=C at the beginning of these scripts, to rule out unexpected locale dependency in other commands too. (this also avoids making the code look even more cluttered)
  5. practicalswift force-pushed on Jun 13, 2018
  6. MarcoFalke renamed this:
    Add linter: Make sure LC_ALL=C is set when using grep range expressions
    Make sure LC_ALL=C is set when using grep range expressions
    on Jun 13, 2018
  7. MarcoFalke added the label Build system on Jun 13, 2018
  8. practicalswift commented at 2:54 pm on June 13, 2018: contributor
    @laanwj Good idea. Now enforcing export LC_ALL=C. Please re-review :-)
  9. practicalswift renamed this:
    Make sure LC_ALL=C is set when using grep range expressions
    Make sure LC_ALL=C is set in all shell scripts
    on Jun 13, 2018
  10. MarcoFalke commented at 2:57 pm on June 13, 2018: member

    How would shellcheck segfault on this? :face_with_head_bandage:

    0test/lint/lint-shell.sh: line 29:  8402 Segmentation fault      (core dumped) shellcheck -e SC2001,SC2004,SC2005,SC2006,SC2016,SC2028,SC2046,SC2048,SC2066,SC2086,SC2116,SC2148,SC2162,SC2166,SC2181 $(git ls-files -- "*.sh" | grep -vE 'src/(secp256k1|univalue)/')
  11. practicalswift commented at 3:10 pm on June 13, 2018: contributor
    @MarcoFalke Oh, that’s weird! I’m unable to reproduce locally. Will try to find time to investigate soon!
  12. MarcoFalke commented at 6:10 pm on June 13, 2018: member
    Presumably caused by the non-ascii char in https://github.com/practicalswift/bitcoin/blob/0f556d2e6d19e5fd86712bb5f4b0506924740ae3/test/lint/lint-python.sh#L65
  13. practicalswift force-pushed on Jun 13, 2018
  14. practicalswift force-pushed on Jun 13, 2018
  15. practicalswift commented at 9:25 pm on June 13, 2018: contributor
    @MarcoFalke Tried removing that but same issue :-\
  16. laanwj commented at 7:08 am on June 14, 2018: member
    Seems like an upstream bug then, with the version installed by Travis, if you can’t reproduce it locally?
  17. ken2812221 commented at 7:29 am on June 14, 2018: contributor

    Looks like travis has already installed shellcheck v0.4.6, but we reinstall v0.3.3 from ubuntu repo.

    Edit: Tried not to downgrade shellcheck, not going to work.

  18. practicalswift force-pushed on Jun 14, 2018
  19. practicalswift force-pushed on Jun 14, 2018
  20. practicalswift commented at 10:07 am on June 14, 2018: contributor

    Worked around the shellcheck issue by adding an “opt in to locale dependence” mechanism which is now in used for test/lint/lint-shell.sh.

    Please re-review :-)

  21. laanwj commented at 11:28 am on June 14, 2018: member
    Still segfaults?!?
  22. practicalswift force-pushed on Jun 14, 2018
  23. practicalswift commented at 1:24 pm on June 14, 2018: contributor
    @laanwj LC_ALL=C was exported by the lint script runner test/lint/lint-all.sh and thus inherited also by test/lint/lint-shell.sh. Now “opting in to locale dependence” also in lint-all.sh to allow for the linting scripts to opt in/out out of locale dependences themselves :-)
  24. Add "export LC_ALL=C" to all shell scripts 3352da8da1
  25. Add linter: Make sure all shell scripts opt out of locale dependence using "export LC_ALL=C" 47776a958b
  26. practicalswift force-pushed on Jun 14, 2018
  27. laanwj commented at 2:16 pm on June 14, 2018: member
    Good. We should not forget to remove this workaround when we’ll be able to use a newer shellcheck, it’s a bit yucky, but checking all scripts but two is much better than nothing. utACK 47776a958b08382d76d69b5df7beed807af168b3
  28. MarcoFalke deleted a comment on Jun 15, 2018
  29. ken2812221 commented at 4:57 pm on June 16, 2018: contributor
    utACK 47776a958b08382d76d69b5df7beed807af168b3
  30. laanwj merged this on Jun 18, 2018
  31. laanwj closed this on Jun 18, 2018
  32. laanwj referenced this in commit 45c00f8416 on Jun 18, 2018
  33. MarcoFalke commented at 11:24 am on June 18, 2018: member

    @practicalswift This fails travis with

    0test/lint/lint-all.sh
1Missing "export LC_ALL=C" (to avoid locale dependence) as first non-comment non-empty line in test/lint/lint-python-utf8-encoding.sh
2^---- failure generated from test/lint/lint-shell-locale.sh
  34. practicalswift referenced this in commit 7b23e6e13f on Jun 18, 2018
  35. practicalswift commented at 12:21 pm on June 18, 2018: contributor
    @MarcoFalke Oh, thanks! Fixed by #13494.
  36. MarcoFalke referenced this in commit d67eff8002 on Jun 18, 2018
  37. stamhe referenced this in commit 925f396f0d on Jun 27, 2018
  38. Longyan-Wu referenced this in commit 8857da2a60 on Jul 1, 2018
  39. HashUnlimited referenced this in commit 42da5db28f on Sep 11, 2018
  40. HashUnlimited referenced this in commit a5e9a06a04 on Sep 11, 2018
  41. joemphilips referenced this in commit 76013b639e on Nov 9, 2018
  42. PastaPastaPasta referenced this in commit 65a6976d31 on Jul 29, 2020
  43. PastaPastaPasta referenced this in commit dad6e669b8 on Jul 29, 2020
  44. PastaPastaPasta referenced this in commit 86c9819fc4 on Jul 29, 2020
  45. PastaPastaPasta referenced this in commit e597662f77 on Jul 29, 2020
  46. PastaPastaPasta referenced this in commit cb5d0d8b99 on Jul 29, 2020
  47. PastaPastaPasta referenced this in commit 9a6c8277b6 on Jul 29, 2020
  48. zkbot referenced this in commit 311a079dd5 on Oct 27, 2020
  49. zkbot referenced this in commit 43ac2062f9 on Oct 28, 2020
  50. zkbot referenced this in commit 84a5830aaa on Nov 9, 2020
  51. practicalswift deleted the branch on Apr 10, 2021
  52. gades referenced this in commit 34f2d9dfb7 on Mar 8, 2022
  53. DrahtBot locked this on Aug 18, 2022


practicalswift laanwj MarcoFalke ken2812221

Labels
Build system Scripts and tools

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-11-20 21:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me