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

Mount /etc/localtime on macOS with Orbstack #196

Merged
merged 6 commits into from
Aug 11, 2024
Merged

Conversation

cHJlaXpoZXI
Copy link
Contributor

Description

This PR is to delete line that block Orbstack to mount /etc/localtime. Orbstack on macOS can mount this folder proprely now. I have tested several time (no option, VPN and Desktop) and no issue, the time is sync with the host !

Related issues

No related issue

Point of attention

Hope that my modification is correct :)

Delete line that block Orbstack to mount /etc/localtime
@cHJlaXpoZXI cHJlaXpoZXI changed the title Update ContainerConfig.py Mount /etc/localtime on macOS with Orbstack Dec 19, 2023
@ShutdownRepo
Copy link
Member

@QU35T-code can you confirm?

@ShutdownRepo ShutdownRepo added enhancement New feature or request macOS macOS support is involved labels Dec 19, 2023
@Dramelac
Copy link
Member

Thank you for the PR. We need to do some testing before merging.

Regarding your code, my only concern is because of the docker-desktop patch:
path_match = path_match.replace("/etc/", "/private/etc/")

Not sure if it's necessary when using orbstack or not

@cHJlaXpoZXI
Copy link
Contributor Author

Thank you for the PR. We need to do some testing before merging.

Regarding your code, my only concern is because of the docker-desktop patch: path_match = path_match.replace("/etc/", "/private/etc/")

Not sure if it's necessary when using orbstack or not

Orbstack mount the /etc/localtime by default. So I guess no. I haven't delete it because I don't know if this was used somewhere else or not.

If not, we can remove everything then.

And sure for testing no problem ;) if I can contribute with test, tell me ;)

@@ -963,8 +963,6 @@ def addVolume(self,
logger.critical(msg)
raise CancelOperation(msg)
if path_match.startswith("/etc/"):
Copy link
Member

Choose a reason for hiding this comment

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

Change to if path_match.startswith("/etc/") and EnvInfo.isDockerDesktop():

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I make the change :)

@QU35T-code QU35T-code added the waiting for additional changes Further changes are requested label Jul 2, 2024
@Dramelac
Copy link
Member

Dramelac commented Jul 8, 2024

@ShutdownRepo or @QU35T-code Can you confirm Orbstack can bind volume from /etc ?
Can you also test with Docker Desktop if it works or not ?

@Dramelac Dramelac added waiting for additional info Further information is requested and removed waiting for additional changes Further changes are requested labels Jul 8, 2024
Copy link
Member

@Dramelac Dramelac left a comment

Choose a reason for hiding this comment

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

Thank you for the testing and the PR !

@Dramelac Dramelac merged commit 5e5b1bc into ThePorgs:dev Aug 11, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request macOS macOS support is involved waiting for additional info Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants