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 MYST_HOME environment variable for configuring load dirs. #194

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

faultyserver
Copy link
Member

The old MYST_PATH that allowed multiple paths similar to $PATH has been removed in favor of MYST_HOME, which should always point to the install location of the currently-active version of Myst. This is both simpler in terms of the implementation, and more intuitive to reason about

This should correspond with changes coming in mtenv that will set MYST_HOME in the shim before running myst.

For now, users who do not use mtenv will have to set this manually, though we can probably check and ensure it is set as part of the VM setup. I think this should probably emit a warning and notify the user what the VM will use in its place, since it might not always be accurate (e.g., if a user wrongly installed myst directly into /usr/local/bin without moving the standard library as well).

Perhaps a better solution would be to disallow execution if MYST_HOME is not set?

The old `MYST_PATH` that allows multiple paths similar to `$PATH` has been removed in favor of `MYST_HOME`, which _should_ always point to the install location of the currently-active version of Myst.

This should correspond with changes coming in `mtenv` that will set `MYST_HOME` in the shim before running `myst`. For now, users who do not use `mtenv` will have to set this manually, though we can probably check and ensure it is set as part of the VM setup.
@faultyserver faultyserver added this to the Next milestone May 28, 2018
end.as(Array(String))
@load_dirs ||= [
Dir.current,
ENV["MYST_HOME"]?
Copy link
Member

Choose a reason for hiding this comment

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

What if MYST_HOME is empty? We just let it go? Who needs the stdlib anyways?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it definitely needs more discussion. Ideally load_dirs wouldn't even live here and we could have a more authoritative place for configurable things like this (probably with dependency injection as well? for embedded versions).

@faultyserver faultyserver modified the milestones: v0.6.1, Next May 29, 2018
@faultyserver
Copy link
Member Author

One other thing to consider with this is that running scripts without a login shell would probably end up not setting MYST_HOME for a lot of users, at least ones who aren't using mtenv.

With mtenv, /usr/local/bin/myst is a shim that automatically sets MYST_HOME, rather than relying on the shell profile or something else to set it, so this isn't an issue.

On manual installs, even if a user sets MYST_HOME in something like ~/.bash_profile, it won't be loaded for non-login shells. Realistically, this probably won't ever be an issue, and it can be solved by editing /etc/profile or ~/.bashrc or whatever with the variable as well. The only case I can really think of is background processes/daemons, and we can always come back to address this later.

@Jens0512
Copy link
Member

Jens0512 commented Jun 8, 2018

A myst daemon sounds like a terrible idea IMO. It'd be more like a myst demon 😛

As the primary goal of myst is to support users as best as possible, I think a strong competitor for how to set MYST_HOME in manual installations is the Makefile – I mean, the purpose of Makefiles is very much to aid installations.

A manual installation should be done with the Makefile, any kind of other manual installation would be little, but meaningless. This is how I imagine a manual installation:

  • make install MYST_HOME=/opt/myst/%v (or wherever you want it – and of course with a default value)

Where %v gets substituted with version just because its nice. This install a myst executable to /opt/myst/<version>/bin/myst, the stdlib to /opt/myst/<version>/stdlib, and installs a shell script /usr/local/bin/myst (unless otherwise specified) that sets env MYST_HOME to what specified in the Makefile installation, and executes myst.

If not: every OS should have some way for environment variables to be set (even when not running login shells) – so worst case scenario is to force manual setting.

EDIT: Although for embedded Myst, this would not be sufficient though.

@faultyserver
Copy link
Member Author

That sounds like a pretty solid plan to me. I certainly think it's fair to say "if you don't use mtenv or make install, it's your responsibility to set MYST_HOME before running Myst".

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