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

Look for emcc in the path as a last resort #8

Closed
wants to merge 2 commits into from

Conversation

@bowling-allie
Copy link
Contributor

bowling-allie commented Sep 13, 2019

both toolchain files call 'find_file(_EMCC emcc)' and if '_EMCC'
exists on the file system, they use its parent folder as
'EMSCRIPTEN_PREFIX'

both toolchain files call 'find_file(_EMCC emcc)' and if  '_EMCC'
exists on the file system, they use its parent folder as
'EMSCRIPTEN_PREFIX'
@bowling-allie bowling-allie changed the title both emscripten toolchains look for emcc in the path as a last resort Look for emcc in the path as a last resort Sep 13, 2019
Copy link
Owner

mosra left a comment

Thank you, this is a very valuable addition as well! 👍

set(EMSCRIPTEN_PREFIX "/usr/lib/emscripten")
endif()
unset(_EMCC)
#find_file always sets a cache variable. we dont want this

This comment has been minimized.

Copy link
@mosra

mosra Sep 18, 2019

Owner

I think having this variable could be a valid use case, in fact -- imagine having two separate Emscripten installations (for whatever reason) and the toolchain picking the wrong one because its emcc leaked into PATH. Then, to debug this, one could spot the variable being set to an incorrect value (and use EMSCRIPTEN_PREFIX next time). A bit bad is that there's now two ways to do the same thing (either setting EMSCRIPTEN_PREFIX or this _EMCC), but I don't have an immediate idea how to make it better :)

So, alternatively, could the variable be named _EMSCRIPTEN_EMCC_EXECUTABLE and this called on it so it's hidden by default but "inspectable" when one needs to?

mark_as_advanced(_EMSCRIPTEN_EMCC_EXECUTABLE)

This comment has been minimized.

Copy link
@bowling-allie

bowling-allie Sep 18, 2019

Author Contributor

You're totally right. It slipped my mind that there might be a 'reason' for not being able to disable saving to the cache.

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Sep 20, 2019

Merged as 23cd3a4, thank you! :)

I'll be updating the toolchain submodules in all repos gradually with other changes (so not immediately), hope that's okay.

@mosra mosra closed this Sep 20, 2019
@bowling-allie

This comment has been minimized.

Copy link
Contributor Author

bowling-allie commented Sep 20, 2019

Of course, thank you for your hard work on these libraries!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.