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

Use apple's native ObjC APIs to produce the right paths #6

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jfro
Copy link

@jfro jfro commented Jul 12, 2020

  • Updated to 2018 edition, though doesn't change much currently
  • Brings in objc on mac/ios side to use apple's APIs for acquiring various directories
    • This should fix systems that are on non-english native locales (some folders are localized on the filesystem)
    • This should fix using any rust libraries using dirs-sys-rs within a sandbox mac/ios app

@soc
Copy link
Collaborator

soc commented Jul 14, 2020

@jfro Thanks! I will look into it shortly!

@soc
Copy link
Collaborator

soc commented Jan 13, 2021

@jfro Thanks for your contribution – do you think it makes sense to expose SearchPathDomainMask here, if we will only ever call it with UserDomain?

@soc
Copy link
Collaborator

soc commented Jan 14, 2021

@jfro Thanks! I'll merge this relatively soon.

For future contributions, I'd like to kindly ask to avoid mixing code reformattings into PRs, it makes it harder to review and see what's going on.

Cheers, Simon!

@jfro
Copy link
Author

jfro commented Jan 14, 2021

re SearchPathDomainMask yeah you're right, for this crate we never use but the user one

@soc
Copy link
Collaborator

soc commented Jan 15, 2021

Ok, thanks!

@jfro
Copy link
Author

jfro commented Jan 18, 2021

Looking back at it, I guess I lean more towards leaving it public since this is more of a sys wrapper around the system API so it's exposing it's functionality, though the higher level crate will unlikely use system domain at this time.

If we really want we could hide it for now, just make the mask private & make path_for_dir hardcoded to user domain.

@soc
Copy link
Collaborator

soc commented Jan 19, 2021

Ok, thanks, I'll keep it public then!

@soc
Copy link
Collaborator

soc commented Feb 6, 2021

This should fix systems that are on non-english native locales (some folders are localized on the filesystem)

@jfro After some investigation, it appears that the translation of folders happens in the UI, so e. g. ~/Downloads works on every install, regardless of locale on macOS. Do you have an example to the contratry?

This should fix using any rust libraries using dirs-sys-rs within a sandbox mac/ios app

At this point, this would be the remaining reason for this change ... could you expand on this? Is there some official documentation on the behavior?

@jfro
Copy link
Author

jfro commented Feb 8, 2021

Yeah so iOS apps & macOS apps (macOS is optional or store apps) get a container folder that the app is allowed to read/write, where it can't access system & user folders without special permission first. So accessing say $HOME in a traditional manner will be rejected if not using say an open/save GUI panel to allow user to provide that permission.

So typically for these apps to store various data like logs, config, etc they use the APIs I used in this PR. They've been around before sandbox, so they work in & outside of sandbox, returning the correct paths accordingly. So asking for a data dir you'd get ~/Library/Application Support on regular macOS app, or ~/Library/Containers/MyAppId/Library/Application Support

Docs about the sandbox container found here:
https://developer.apple.com/library/archive/documentation/Security/Conceptual/AppSandboxDesignGuide/AppSandboxInDepth/AppSandboxInDepth.html#//apple_ref/doc/uid/TP40011183-CH3-SW6

I've also wanted to introduce similar changes for Windows and its containers, but that API is harder to reach from Rust, so far seems to require winrt or the new windows crate which are heavy, specially with build times.

@soc
Copy link
Collaborator

soc commented Feb 8, 2021

@jfro Thank you, I'll read up on that!

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

Successfully merging this pull request may close these issues.

2 participants