Skip to content

Allow cache directory to be configured via config file#11091

Merged
sbc100 merged 3 commits into
masterfrom
cache_config
May 7, 2020
Merged

Allow cache directory to be configured via config file#11091
sbc100 merged 3 commits into
masterfrom
cache_config

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented May 7, 2020

Honor CACHE setting in config file as an alternative to EM_CACHE
environment variable or --cache commandline flag.

Specifically this is useful for emsdk which wants to set the cache directory
and currently has has to export both EM_CONFIG and EM_CACHE whereas
it should be enough to do just the one.

Hopefully we can remove the command line flag if it has no more
users.

@sbc100 sbc100 requested a review from kripken May 7, 2020 01:13
@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented May 7, 2020

The --cache command line flag as was added back in #2816. We currently don't have any tests for it though. @waywardmonkeys .. are you still using this flag? Would this config setting work for you in its place?

@waywardmonkeys
Copy link
Copy Markdown
Contributor

We no longer use the command flag but pass it again in the environment. That said, setting it in the config file would be superior.

Is there value in doing the same for EMCC_TEMP_DIR?

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented May 7, 2020

Perhaps yes, regarding EMCC_TEMP_DIR. My first question though would be, why would want to set this explicitly like that? Why not rely on the OS-convesion for setting temp dir across all applications? We already default to using tempfile.gettempdir() which in UNIX is controlled via $TMPDIR and defaults to /tmp.. is there a reason that this mechanism that is shared by all applications is not what you want here too?

@waywardmonkeys
Copy link
Copy Markdown
Contributor

That's a good question. I don't recall. I know that we added this as some of our CI machines kept running out of space in /tmp randomly and I wanted to isolate the emscripten temp stuff. But setting $TMPDIR should be sufficient there ... Maybe a separate bug to consider removing EMCC_TEMP_DIR ? :)

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented May 7, 2020

Yes, I think removing EMCC_TEMP_DIR would be great if we can.

@waywardmonkeys
Copy link
Copy Markdown
Contributor

I've updated our build system to no longer use EMCC_TEMP_DIR. :)

sbc100 added 3 commits May 6, 2020 23:07
Honor `CACHE` setting in config file as an alternative to `EM_CACHE`
environment variable or `--cache` commandline flag.

Hopefully we can remove the command line flag if it has no more
users.
Comment thread tools/shared.py
JS_ENGINES = [listify(engine) for engine in JS_ENGINES]
WASM_ENGINES = [listify(engine) for engine in WASM_ENGINES]
if not CACHE:
CACHE = os.path.expanduser(os.path.join('~', '.emscripten_cache'))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we put a default value in tools/settings_template.py?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No.. we don't normal users to be setting this. Its mostly for the benefit of emsdk. See emscripten-core/emsdk#495.

Right now emsdk has to use the environment variable which I'd like to change.

sbc100 added a commit to emscripten-core/emsdk that referenced this pull request May 7, 2020
@sbc100 sbc100 merged commit 8a3ac89 into master May 7, 2020
@sbc100 sbc100 deleted the cache_config branch May 7, 2020 16:30
sbc100 added a commit that referenced this pull request May 7, 2020
This is an ancient and untested command line argument.  As discussed
in #11091 it seems pretty safe for remove.  We have `CACHE` config
file setting as well as EM_CACHE environment variable.
sbc100 added a commit that referenced this pull request May 7, 2020
This is an ancient and (hopefully) unused command line argument.  As
discussed in #11091 it seems pretty safe for remove.  We have `CACHE`
config file setting as well as EM_CACHE environment variable to
replace it.
sbc100 added a commit to emscripten-core/emsdk that referenced this pull request May 7, 2020
sbc100 added a commit to emscripten-core/emsdk that referenced this pull request May 7, 2020
sbc100 added a commit that referenced this pull request May 7, 2020
This is an ancient and (hopefully) unused command line argument.  As
discussed in #11091 it seems pretty safe for remove.  We have `CACHE`
config file setting as well as EM_CACHE environment variable to
replace it.
wtu87830895-svg pushed a commit to wtu87830895-svg/emsdk2 that referenced this pull request Dec 20, 2025
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.

3 participants