Improve update-locales script and fix locale processing bug #23240

wxiaoguang · 2023-03-02T10:49:21Z

The locales of Gitea has been broken for long time, till now, it's still not fully fixed.

One of the root problems is that the ini library is quite quirky and the update-locales script doesn't work well for all cases.

This PR fixes the update-locales script to make it satisfy ini library and the crowdin.

See the comments for more details.

The locale_zh-CN.ini is an example, it comes from crowdin and is processed by the new update-locales.sh. Especially see the feed_of: https://github.com/go-gitea/gitea/pull/23240/files#diff-321f6ca4eae1096eba230e93c4740f9903708afe8d79cf2e57f4299786c4528bR268

codecov-commenter · 2023-03-02T11:41:25Z

Codecov Report

Merging #23240 (5a1e6de) into main (33e556e) will increase coverage by 0.12%.
The diff coverage is 54.44%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main   #23240      +/-   ##
==========================================
+ Coverage   47.44%   47.56%   +0.12%     
==========================================
  Files        1140     1143       +3     
  Lines      150804   151076     +272     
==========================================
+ Hits        71549    71866     +317     
+ Misses      70788    70705      -83     
- Partials     8467     8505      +38

Impacted Files	Coverage Δ
cmd/serv.go	`2.59% <0.00%> (-0.06%)`	⬇️
models/actions/run.go	`1.72% <0.00%> (+0.01%)`	⬆️
models/actions/run_list.go	`0.00% <0.00%> (ø)`
models/actions/status.go	`22.85% <0.00%> (-1.39%)`	⬇️
models/actions/task.go	`1.69% <0.00%> (-0.01%)`	⬇️
models/auth/oauth2.go	`60.52% <ø> (ø)`
models/auth/twofactor.go	`19.73% <ø> (ø)`
models/db/engine.go	`44.80% <ø> (ø)`
models/unittest/testdb.go	`12.92% <0.00%> (ø)`
modules/auth/password/hash/pbkdf2.go	`69.69% <ø> (ø)`
... and 115 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

build/update-locales.sh

zeripath

Thanks @wxiaoguang

I kept meaning to get round to looking at this as I suspected that something was going wrong to do with the ini output preparation.

This to me indicates again that we either need to get on with fixing go-ini or get off it.

zeripath · 2023-03-02T14:59:42Z

I'd also say we might want to consider making this script some go code instead

wxiaoguang · 2023-03-02T15:04:43Z

I'd also say we might want to consider making this script some go code instead

Agree, and I prefer to get rid of the ini more 😁 if there is no buggy ini package, we do not need these tricks.

I think this PR is the last fix by modifying the script. Next time we should decide whether use Go to rewrite or totally replace the ini package.

build/update-locales.sh

lunny · 2023-03-02T15:12:01Z

replace #23239

jolheiser · 2023-03-02T18:33:26Z

🎺 🤖

silverwind · 2023-03-02T19:07:18Z

build/update-locales.sh

+if [[ $OSTYPE == 'darwin'* ]]; then
+  # for macOS developers, use "brew install gnu-sed"
+  SED=gsed
+fi


Could just do the same detection that Makefile does and adapt sed command syntax based on whether it is GNU or BSD sed. Better than to require the installation of additional software :)

I'll check it out.

Thank you. Actually I have thought about this problem, but I didn't choose the sed -i'' for BSD sed, because the script has been here for long time, nobody really needs it, I just want to avoid any inconsistency. And I think this PR is the last fix by modifying the script 😂 in the future we should decide whether use Go to rewrite or totally replace the ini package.

The sed -i'' might also work, just like it in Makefile, but I haven't tested it.

Some more explanations about my "worry about inconsistency": this script is hard to debug, and not covered by any test (unlike the sed in Makefile, any error can be found in the first time and gets fixed quickly). If there is any inconsistency between GNU sed and BSD sed, the problem can not be found until next crowdin sync runs, and it's difficult for macOS users to debug the inconsistency if they are using a different version.

If there is no inconsistency, then using sed -i'' is fine for BSD sed.

Just share some of my thoughts, for your information, no objection nor block. 😁

wxiaoguang · 2023-03-03T01:24:23Z

I didn't know where this script runs in. If it runs in the environment without bash (really? it's 2023 now ...), I think we should also revert the shebang line to #!/bin/sh and remove the bash related if [[.

ps: I didn't see today's crowdin sync commit, does it mean that this script failed because I used bash in last PR?
I want to mention some maintainers but I don't know who knows the crwodin sync infrastructure ....

* giteaofficial/main: Use async await to fix empty quote reply at first time (go-gitea#23168) Fix switched citation format (go-gitea#23250) Improve update-locales script and fix locale processing bug (go-gitea#23240) Refactor `ctx` in templates (go-gitea#23105) Improve frontend guideline (go-gitea#23252) Close the temp file when dumping database to make the temp file can be deleted on Windows (go-gitea#23249) # Conflicts: # templates/repo/issue/view_content/context_menu.tmpl

wxiaoguang · 2023-03-03T13:25:42Z

Sorry, I think it's incorrect to use bash in this script.

The last run fails:

https://drone.gitea.io/go-gitea/gitea/68658/1/3

3.17: Pulling from library/alpine
Digest: sha256:69665d02cb32192e52e07644d76bc6f25abeb5410edc1c7a81a10ba3f0efb90a
Status: Image is up to date for alpine:3.17
+ ./build/update-locales.sh
/bin/sh: ./build/update-locales.sh: not found

…x some strings with semicolons (#23819) Follow #23633 and #23240 Close #23814 Now we almost have a complete test set for Gitea's LocalStore. This PR is still a quick fix for the legacy locale system (see the TODOs), to resolve the problems fundamentally, it needs more work in the future.

wxiaoguang force-pushed the fix-update-locales branch from a6238d3 to aaaf267 Compare March 2, 2023 10:50

fix

5a1e6de

wxiaoguang force-pushed the fix-update-locales branch from aaaf267 to 5a1e6de Compare March 2, 2023 10:54

wxiaoguang mentioned this pull request Mar 2, 2023

[skip ci] update zh-cn because of crowdin's bug #23239

Closed

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 2, 2023

wolfogre approved these changes Mar 2, 2023

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 2, 2023

wxiaoguang commented Mar 2, 2023

View reviewed changes

build/update-locales.sh Outdated Show resolved Hide resolved

add comment

c338fb9

wxiaoguang force-pushed the fix-update-locales branch from 2508af5 to c338fb9 Compare March 2, 2023 14:55

zeripath approved these changes Mar 2, 2023

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 2, 2023

delvh approved these changes Mar 2, 2023

View reviewed changes

lunny reviewed Mar 2, 2023

View reviewed changes

build/update-locales.sh Outdated Show resolved Hide resolved

fix typo in comment

9b73983

lunny added modifies/translation topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Mar 2, 2023

Merge branch 'main' into fix-update-locales

8d48775

jolheiser merged commit d72462d into go-gitea:main Mar 2, 2023

jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 2, 2023

wxiaoguang deleted the fix-update-locales branch March 2, 2023 18:33

silverwind reviewed Mar 2, 2023

View reviewed changes

wxiaoguang mentioned this pull request Mar 30, 2023

Clarify Gitea/Crowdin locale behaviors, add tests for LocaleStore, fix some strings with semicolons #23819

Merged

delvh added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Apr 3, 2023

delvh added this to the 1.20.0 milestone Apr 3, 2023

go-gitea locked and limited conversation to collaborators May 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve update-locales script and fix locale processing bug #23240

Improve update-locales script and fix locale processing bug #23240

wxiaoguang commented Mar 2, 2023 •

edited

Loading

codecov-commenter commented Mar 2, 2023

zeripath left a comment

zeripath commented Mar 2, 2023

wxiaoguang commented Mar 2, 2023

lunny commented Mar 2, 2023

jolheiser commented Mar 2, 2023

silverwind Mar 2, 2023 •

edited

Loading

silverwind Mar 2, 2023

silverwind Mar 2, 2023

wxiaoguang Mar 3, 2023

wxiaoguang Mar 3, 2023

wxiaoguang commented Mar 3, 2023

wxiaoguang commented Mar 3, 2023

Improve update-locales script and fix locale processing bug #23240

Improve update-locales script and fix locale processing bug #23240

Conversation

wxiaoguang commented Mar 2, 2023 • edited Loading

codecov-commenter commented Mar 2, 2023

Codecov Report

zeripath left a comment

Choose a reason for hiding this comment

zeripath commented Mar 2, 2023

wxiaoguang commented Mar 2, 2023

lunny commented Mar 2, 2023

jolheiser commented Mar 2, 2023

silverwind Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

silverwind Mar 2, 2023

Choose a reason for hiding this comment

silverwind Mar 2, 2023

Choose a reason for hiding this comment

wxiaoguang Mar 3, 2023

Choose a reason for hiding this comment

wxiaoguang Mar 3, 2023

Choose a reason for hiding this comment

wxiaoguang commented Mar 3, 2023

wxiaoguang commented Mar 3, 2023

wxiaoguang commented Mar 2, 2023 •

edited

Loading

silverwind Mar 2, 2023 •

edited

Loading