Tieske / luawinmulti

Lua build and installation script for Windows, building multiple Lua versions in parallel, including LuaRocks.
Other
57 stars 5 forks source link

Persist path changes #17

Closed Sharparam closed 3 years ago

Sharparam commented 5 years ago

The setlua helper seems to only update the environment variables for the current shell session. Since the paths don't seem to change when switching the unversioned Lua interpreters, could these paths not be persisted to the machine on installation?

I can add them manually, but then if I invoke setlua to change to a certain version it will add the paths again, so for that shell session I will have duplicated paths, which is not optimal.

Seems it might be better (or as an option) if the paths could be set on install, and setlua just changes the unversioned executables around?

Tieske commented 5 years ago

Unfortunately there is no easy way to get this right.

I can add them manually, but then if I invoke setlua to change to a certain version it will add the paths again, so for that shell session I will have duplicated paths, which is not optimal.

Maybe we can add a small Lua script that will take care of de-duplication? (very hard to do with a batch file)

That seems the cleanest, since it would allow users to choose between adding them on startup, or only when invoking.

PR's are welcome!

Sharparam commented 5 years ago

Maybe an option on install to generate a setlua that doesn't set the environment variables, and instead prompt the user to update their system with the proper values?

I agree batch programming is needlessly difficult for anything not super simple :P

Tieske commented 5 years ago

we tried that with LuaRocks, also hard to do due to system priviledges etc. Also requires extra steps for novices, making it harder.

I think a small Lua script could actually do the deduplication, when setlua runs we already know the Lua environment, so shouldn't be too hard.

Tieske commented 4 years ago

closing this, as it is most likely not going to happen. And also; easy to workaround.

Note that setlua.bat itself protects from adding the same paths over and over again.

Tieske commented 3 years ago

@Sharparam I created a PR that adds this. Mind giving it some test runs?

Sharparam commented 3 years ago

This seems to indeed work, with the caveat that setx breaks if PATH is longer than 1024 characters (the GUI in Windows to edit environment variables has the same limitation). It also seems to copy my system (machine-wide) path into my user-specific path. I have different values in my user-scoped PATH and the machine one.

The .NET API Environment.SetEnvironmentVariable has a much higher limit (32 767 characters).

Would it be feasible to invoke a powershell script to edit the variable when -x is provided?

So it seems a few things would be desired:

Tieske commented 3 years ago

Do not overwrite the user-scoped PATH variable with the system one. The script should only add the variables needed by the Lua environment to it.

How to access those independent values? seems there is only the path variable? how to tell which entry came from which source?

Sledmine commented 3 years ago

Do not overwrite the user-scoped PATH variable with the system one. The script should only add the variables needed by the Lua environment to it.

How to access those independent values? seems there is only the path variable? how to tell which entry came from which source?

Hope to give a little help over here, there is a way to set and get env variables from different scopes machine or user, using powershell via NET.API

API Docs: https://docs.microsoft.com/en-us/dotnet/api/system.environment.setenvironmentvariable?view=net-5.0

Some PowerShell examples: http://xahlee.info/powershell/environment_variables.html

Tieske commented 3 years ago

Down the rabbit hole ....

What I get from this so far;

so the logic would have to be:

  1. get the user_path
  2. get current environment variable value for path (which the system bases of system paths + user paths)
  3. append new paths to user_path and write to persist (for future command shells)
  4. append new paths to path for use in the current command shell

alternatively do 3 only when -x is set, and 4 only without -x.

Is that correct?

Sharparam commented 3 years ago

That sounds like it should work.

Can powershell scripts be executed from a batch script? I remember at least a while ago the default on Windows is to not trust powershell scripts downloaded from external sources so you either needed to trust it explicitly or change a policy.

On Wed, 23 Dec 2020, 12:27 Thijs Schreijer, notifications@github.com wrote:

Down the rabbit hole ....

What I get from this so far;

  • So the path in a command shell is constructed as: user_path; system_path
  • Using setx overwrites the user_path (hence we're currently copying system_path values in there, which we do not want)

so the logic would have to be:

  1. get the user_path
  2. get current environment variable value for path (which the system bases of system paths + user paths)
  3. append new paths to user_path and write to persist (for future command shells)
  4. append new paths to path for use in the current command shell

alternatively do 3 only when -x is set, and 4 only without -x.

Is that correct?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Tieske/luawinmulti/issues/17#issuecomment-750176121, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEGRNUUGJKZKRCHV2LVHM3SWHHYLANCNFSM4IWTCMLA .

Tieske commented 3 years ago

The rabbit hole gets deeper and deeper...

I think we should not be adding this. This will end up being a support nightmare for all the different configs out there. Thoughts?

Sharparam commented 3 years ago

Getting it to work within the limitations of Batch would probably be frustrating. Having the notice be printed to the user about manually updating the paths might be enough, and is similar to how other tools like rbenv function.

As setlua should detect if paths exist already and not re-add them, then if the user adds the paths manually after installation it should work sufficiently.

Tieske commented 3 years ago

23 adds the ability to cleanup duplicate entries in paths. So the user can add them manually without them getting polluted.

closing this.