Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Improve sed detection in update-locales.sh #23254

Merged
merged 6 commits into from
Mar 4, 2023
Merged

Improve sed detection in update-locales.sh #23254

merged 6 commits into from
Mar 4, 2023

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 2, 2023 •
edited
Loading

  • Make scripts work from any directory
  • Detect sed version just like Makefile does

@silverwind
Improve build scripts
26c39e5 
- Make scripts work from any directory
- Detect sed version just like Makefile does
@silverwind silverwind added type/enhancement An improvement of existing functionality topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile labels Mar 2, 2023
@silverwind silverwind mentioned this pull request Mar 2, 2023
Merged
@silverwind
Copy link
Member Author

silverwind commented Mar 2, 2023 •
edited
Loading

Had to temporarily disable -e because BSD sed will exit non-zero for a --version argument, in fact it has no way to show its version:

$ /usr/bin/sed --version; echo $?
/usr/bin/sed: illegal option -- -
usage: sed script [-Ealnru] [-i extension] [file ...]
  sed [-Ealnu] [-i extension] [-e script] ... [-f script_file] ... [file ...]
1

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

Removed the cd related changes. The scripts run under /bin/sh so bash variables were not there and I'm not 100% sure I can switch these scripts to bash because it's not present in alpine for example. So this is only the sed change now.

@silverwind silverwind changed the title Improve build scripts Improve sed detection in update-locales.sh Mar 2, 2023
@silverwind silverwind added skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. and removed type/enhancement An improvement of existing functionality labels Mar 3, 2023
@wxiaoguang
Copy link
Contributor

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.

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 ....

@lunny
Copy link
Member

lunny commented Mar 3, 2023

I think it's copied from Makefile and it works well before with no gsed installation requirement.

@silverwind
Copy link
Member Author

Alpine is pretty much the only environment in use today that does not have bash unless manually installed. I also don't know where/how this script runs. Maybe @techknowlogick has some insight.

wxiaoguang
wxiaoguang reviewed Mar 3, 2023
View reviewed changes
@wxiaoguang
Copy link
Contributor

I think we need to take this fix with the shebang line fix .... 😂

@lunny @wxiaoguang
Apply suggestions from code review
3218c08 
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot 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 3, 2023
@silverwind
Copy link
Member Author

As I suspected, this step is running on alpine:3.17, so it needs sh (busybox). I hope the script is sh-compatible.

@silverwind
Copy link
Member Author

Need to merge this to un-break the translation updates.

@GiteaBot 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 4, 2023
@zeripath zeripath merged commit b6d2c94 into go-gitea:main Mar 4, 2023
@silverwind silverwind deleted the buildscripts branch March 4, 2023 14:32
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 6, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
306ba96 
* giteaofficial/main: (28 commits)
  Update hacking-on-gitea-zh_cn documentation (go-gitea#23315)
  Fix broken code editor diff preview (go-gitea#23307)
  [skip ci] Updated translations via Crowdin
  Add context when rendering labels or emojis (go-gitea#23281)
  Change interactiveBorder to fix popup preview  (go-gitea#23169)
  Improve the frontend guideline (go-gitea#23298)
  Scoped labels: set aria-disabled on muted Exclusive option for a11y (go-gitea#23306)
  Add basic documentation for labels, including scoped labels (go-gitea#23304)
  [skip ci] Updated translations via Crowdin
  Re-add accidentally removed `hacking-on-gitea.zh-cn.md` (go-gitea#23297)
  Add default owner team to privated_org and limited_org in unit test (go-gitea#23109)
  Improve sed detection in update-locales.sh (go-gitea#23254)
  Support sanitising the URL by removing extra slashes in the URL (go-gitea#21333)
  Make Ctrl+Enter submit a pending comment (starting review) instead of submitting a single comment (go-gitea#23245)
  Avoid panic caused by broken payload when creating commit status (go-gitea#23216)
  Add run status in action view page (go-gitea#23212)
  update to mermaid v10 (go-gitea#23178)
  Fix code wrap for unbroken lines (go-gitea#23268)
  Fix stray backticks appearing in pull request timeline (go-gitea#23282)
  Fill head commit to in payload when notifying push commits for mirroring (go-gitea#23215)
  ...
@delvh delvh added this to the 1.20.0 milestone Apr 3, 2023
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

@lunny lunny lunny approved these changes

@jolheiser jolheiser jolheiser approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.
7 participants
@silverwind @wxiaoguang @lunny @jolheiser @zeripath @GiteaBot @delvh