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

fix(linux): correct permissions and paths in Linux docker image #13008

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

ermshiperete
Copy link
Contributor

This change fixes two issues in the Linux docker image that showed up when running tests:

  • some directories had wrong permissions
  • we have to build keyman-system-service in a docker-specific directory as well so that the location of the service executable can be found when running the tests in docker

@keymanapp-test-bot skip

This change fixes two issues in the Linux docker image that showed up
when running tests:
- some directories had wrong permissions
- we have to build keyman-system-service in a docker-specific directory
  as well so that the location of the service executable can be found
  when running the tests in docker
@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S20 milestone Jan 23, 2025
@darcywong00
Copy link
Contributor

nit on the PR description (docker-build is more a pseudo-platform and not an active verb 😄 )

chown build:build /run/user/1000 && \
chmod 700 /run/user/1000 && \
mkdir -p /tmp/.X11-unix && \
chmod 1777 /tmp/.X11-unix && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this not needed?

  chown build:build /tmp/.X11-unix && \

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it should stay owned by root.

@mcdurdin mcdurdin changed the title chore(linux): docker-build: fix Linux docker image fix(linux): correct permissions and paths in Linux docker image Jan 24, 2025
@mcdurdin
Copy link
Member

nit on the PR description (docker-build is more a pseudo-platform and not an active verb 😄 )

I have changed the PR title to:

fix(linux): correct permissions and paths in Linux docker image

This is a little more specific on what the PR is actually changing -- it's a fix not a chore, and tries to state what the fixes are.

This changes the test script: when running in Docker it doesn't make
sense to restart ibus when exiting the tests. This works around a
problem where the `ibus start` command would hang.
Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

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

lgtm

@ermshiperete ermshiperete merged commit 0177abf into master Jan 24, 2025
28 checks passed
@ermshiperete ermshiperete deleted the chore/linux/docker branch January 24, 2025 16:51
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 18.0.177-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants