Skip to content

Use cache directory to store build files#39

Open
markfswe wants to merge 8 commits into
source-foundry:masterfrom
markfswe:patch-1
Open

Use cache directory to store build files#39
markfswe wants to merge 8 commits into
source-foundry:masterfrom
markfswe:patch-1

Conversation

@markfswe
Copy link
Copy Markdown

@markfswe markfswe commented Oct 26, 2021

$HOME/Library/Caches in macos (os x)
$XDG_CACHE_HOME/ttfautohint-build (with fallback) in linux

solves #38

@markfswe
Copy link
Copy Markdown
Author

I am not very good at bash so please test it. Especially I am not sure about OS X recognition

@markfswe markfswe changed the title Use cache directory to store build files in Use cache directory to store build files Oct 26, 2021
@chrissimpkins
Copy link
Copy Markdown
Member

Thank you!

Reviewing some background on your approach here and will document in the thread below.

@chrissimpkins
Copy link
Copy Markdown
Member

chrissimpkins commented Nov 6, 2021

From Apple docs on Library/Caches dir:

Contains cached data that can be regenerated as needed. Apps should never rely on the existence of cache files. Cache files should be placed in a directory whose name matches the bundle identifier of the app.

By convention, apps should store cache files in a subdirectory whose name matches the bundle identifier of the app. For example, if your app is named MyApp and has the bundle identifier com.example.MyApp, you would put user-specific cache files in the ~/Library/Caches/com.example.MyApp/ directory.

@chrissimpkins
Copy link
Copy Markdown
Member

Do you know how broadly accepted / implemented the XDG spec is across Linux distros? Is this a Debian-specific issue or do applications commonly use this approach everywhere in Linux env's?

@chrissimpkins
Copy link
Copy Markdown
Member

chrissimpkins commented Nov 6, 2021

From SO re portability of the $OSTYPE environment variable for platform detection:

The OSTYPE environment variable is not recognized by the original Bourne shell

It looks like this may limit our shell support? Perhaps we go with a uname approach instead?

@markfswe
Copy link
Copy Markdown
Author

markfswe commented Nov 15, 2021

From Apple docs on Library/Caches dir:

Yep, thanks for the docs. So we only store build cache files? If yes, this is the right place.

Do you know how broadly accepted / implemented the XDG spec is across Linux distros?

This thing should not be implemented by any distro. It it accepted by the most applications (and it's mostly accepted if it's specific linux only application). Here is a huge list of them: https://wiki.archlinux.org/title/XDG_Base_Directory (list is not full of course) -- here you can see programs and apps that do implement this spec, has workaround or has hardcoded path.
This xdg spec should be treated similar way as windows says to install programs to C:\Program Files and C:\Program Files(x86), (this should be the default installation path, but user can change it).

Is this a Debian-specific issue?

Not related to distro at all. I am using arch and void linux

Or do applications commonly use this approach everywhere in Linux env's?

Yep. I can provide example like alacritty
And even git changed default path from ~/.gitconfig to XDG_CONFIG_HOME/git/config. Both git and alacritty are cross platform of course

UPDATE: I did some changes to the path in a commit above, check it again please

@chrissimpkins
Copy link
Copy Markdown
Member

Thank you!

I updated our CI tests to use GitHub Actions and rebased your changes here on the master branch updates so that we can run the tests. If everything passes, let's merge this and cut a new release with your updates.

@chrissimpkins
Copy link
Copy Markdown
Member

chrissimpkins commented Dec 5, 2021

Mind updating the string check syntax fail in the shellcheck lint so that it passes POSIX sh tests?

https://github.com/koalaman/shellcheck/wiki/SC2039#testing-equality

See SO answer https://stackoverflow.com/a/1089852/2848172 for details.

I think that the following syntax change in your if / else block may do the trick:

if [ "$OS" = "Darwin"* ]; then

Copy link
Copy Markdown
Member

@chrissimpkins chrissimpkins left a comment

Choose a reason for hiding this comment

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

@markfswe markfswe requested a review from chrissimpkins March 23, 2022 08:26
@markfswe
Copy link
Copy Markdown
Author

markfswe commented Mar 23, 2022

Need some help converting from [[ "$OS" = "Darwin"* ]] to posix friendly grep as [[ ]] not supported.

stackoverflow.com/a/59553621

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants