Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Pass LANG=C.UTF-8 to environment #1476

Merged
merged 3 commits into from
Dec 6, 2022
Merged

Conversation

bric3
Copy link
Contributor

@bric3 bric3 commented Nov 28, 2022

In order to use act with workflows where any tools, like build tools, depends on the locale being UTF-8, one must pass via command line via the --env option.

However the Github Actions runner already passes LANG=C.UTF-8 by default, which makes the need to explicitly pass this variable via command line un-intuitive.

The PR here aims to make act pass that environment variable by default.

Fixes: #1308
Possibly #639

@bric3 bric3 requested a review from a team as a code owner November 28, 2022 09:12
@bric3
Copy link
Contributor Author

bric3 commented Nov 28, 2022

Out of scope question, what tool is used to label PR by size ?
It's just above 🤦‍♂️

@ChristopherHX
Copy link
Contributor

The PR here aims to make act pass that environment variable by default.

I want to let you know:
This PR partially enforces LANG=C.UTF-8 and overrides an env defined in the workflow file. This now also defines LANG on windows, I'm not shure that github actions does this on windows.

on: push
env:
  LANG: de_DE.utf8
jobs:
  _:
    runs-on: ubuntu-latest
    steps:
    - run: env

@bric3
Copy link
Contributor Author

bric3 commented Nov 28, 2022

Hi thanks for the feedback !

I want to let you know:
This PR partially enforces LANG=C.UTF-8 and overrides an env defined in the workflow file.

Ha ok, I thought the env map was later merged, and workflow envs were overidden.
Where do you think declaring the LANG should happen ?
I was also looking at this location

rc.Env["ACT"] = "true"

Could you point me to a better place for doing that.

This now also defines LANG on windows, I'm not shure that github actions does this on windows.

Ah good point ! I have no idea about that. I don't know how windows runners operate. What would be the best course of action if Windows runner don't.


I'm a novice in Go, so I'm happy for any advice in the current matter.

@ChristopherHX
Copy link
Contributor

ChristopherHX commented Nov 28, 2022

I think you can add a line here:

envList = append(envList, fmt.Sprintf("%s=%s", "RUNNER_TEMP", "/tmp"))

this code only runs if you use a linux docker container and not running directly on windows.
This should also allow overriding it via cli/workflow, but this has to be tested manually.

@bric3
Copy link
Contributor Author

bric3 commented Nov 30, 2022

Thanks @ChristopherHX

@codecov
Copy link

codecov bot commented Nov 30, 2022

Codecov Report

Merging #1476 (3b663ac) into master (4f8da0a) will increase coverage by 2.93%.
The diff coverage is 66.67%.

@@            Coverage Diff             @@
##           master    #1476      +/-   ##
==========================================
+ Coverage   57.50%   60.44%   +2.93%     
==========================================
  Files          32       44      +12     
  Lines        4594     7000    +2406     
==========================================
+ Hits         2642     4231    +1589     
- Misses       1729     2463     +734     
- Partials      223      306      +83     
Impacted Files Coverage Δ
pkg/common/file.go 0.00% <0.00%> (ø)
pkg/container/docker_logger.go 52.08% <ø> (ø)
pkg/container/host_environment.go 0.00% <0.00%> (ø)
pkg/container/util.go 0.00% <0.00%> (ø)
pkg/model/action.go 0.00% <0.00%> (ø)
pkg/model/step_result.go 0.00% <ø> (ø)
pkg/container/docker_run.go 12.82% <11.53%> (+7.27%) ⬆️
pkg/model/workflow.go 45.65% <23.58%> (-5.26%) ⬇️
...ontainer/linux_container_environment_extensions.go 24.32% <24.32%> (ø)
pkg/container/docker_pull.go 33.33% <33.33%> (ø)
... and 37 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@ChristopherHX ChristopherHX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works now as expected from my side. Both overriding the default LANG from cli and workflow file works.

@mergify mergify bot merged commit 1797775 into nektos:master Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider passing LANG=C.UTF-8
3 participants