cbucher / console

This is a modified version of Console 2 for a better experience under Windows Vista/7/8/10 and a better visual rendering.
https://github.com/cbucher/console/wiki
GNU General Public License v2.0
2.93k stars 231 forks source link

Please expand environment variables in shell path #225

Closed alexandrul closed 8 years ago

alexandrul commented 9 years ago

At this point I can't specify the shell as %FOLDER%\abc.exe

cbucher commented 9 years ago

https://github.com/cbucher/console/wiki/How-to-Report-an-Issue

What do you mean by I can't specify?

alexandrul commented 9 years ago

I have a shell.cmd file that defines some environment variables (including %PROJECT-ROOT%) based on its location, then it should launch ConsoleZ with this shell path: %PROJECT-ROOT%\PyCmd\PyCmd.exe

At this moment I get this error: consolez

I was able to check that the ConsoleZ process has access to this environment variable, but so far I couldn't figure it out from the documentation how to:

alexandrul commented 9 years ago

I would like to avoid defining an explicit value in the Tab - Environment settings (main goal being the portability), but it would be absolutely fine if the value specified would be expanded using the parent env vars, something like this: <env var="PROJECT-ROOT1" value="%PROJECT-ROOT%" check="1"/>

where PROJECT-ROOT1 would be the new env var accessible for launching the shell and for the new shell itself, and %PROJECT-ROOT% would be expanded based on the parent env vars.

cbucher commented 9 years ago

https://github.com/cbucher/console/wiki/How-to-Report-an-Issue I'm waiting for the diagnostic report.

If the environment variable is not replaced in the shell command line, then the variable is absent from ConsoleZ's environment block therefore the variable was undefined when ConsoleZ has been launched ...

cbucher commented 9 years ago

Environment variables defined in the tab settings are added to the shell process environment block. I can specify a specific environment block when I create a process. Unfortunately, there is no system API to expand a string with a specific environment block. Therefore these variables cannot be expanded from ConsoleZ process (using Windows API).

If you can configure an environment variable in settings tab, you can copy the value in shell command line. So this is no big deal.

alexandrul commented 9 years ago

Please verify the shell__*.cmd scripts included in this archive: https://bitbucket.org/alexandrul/bin/downloads/project.7z

You can extract the archive content in any folder on your PC (without spaces), e.g. d:\diag_225

Steps for each script:

At least on my PC, ConsoleZ is the only one that is unable to change the current working folder.

alexandrul commented 9 years ago

As mentioned before, you can use Process Explorer (or any other tool) to verify that the %PROJECT-ROOT% env var is present for the ConsoleZ.exe process, but is missing for the launched cmd.exe.

cbucher commented 9 years ago

Yes, you right (sorry for my mistake). ConsoleZ loads the user's environment block, and don't use the current one. Only variables defined in Tab's settings and in Windows system settings are valid.

\ first solution ** Don't create a cmd that configures env and then calls ConsoleZ, but configure ConsoleZ for calling a cmd that configures env.

Just configure the shell in tab's settings like this: image

And launch console.exe -t PyCmd Since it uses relative paths, ConsoleZ should be launched from its directory. So your shell.cmd must be like that: cd "%~dp0\ConsoleZ" console.exe -t PyCmd

\ second solution ** You can create the environment variable PROJECT-ROOT in Windows system settings.

alexandrul commented 9 years ago

Thank you very much for your explanation, your first solution solves my issue.

Is there any chance to add an option to choose between the user's environment block and the current one?

cbucher commented 9 years ago

The current environment block could be used only for shell with current user credentials and only if ConsoleZ is configured with multiple instances. This would require a complex documentation and nobody reads the documentation :wink:

Perhaps these specific environment variables could be defined in ConsoleZ's command line. In this case, these environment variables would be reachable from all user contexts..

If you think this idea is relevant, you can create a feature request.

alexandrul commented 9 years ago

Speaking only for myself:

Since the current behavior in ConsoleZ is not a bug, but a decision, please ignore this issue, given that I'm the only one affected and I have multiple workarounds.

Thanks again for your help.

alexandrul commented 9 years ago

A process inherits its environment from its parent, and the consequences of this simple statement: http://blogs.msdn.com/b/oldnewthing/archive/2015/09/15/10641604.aspx

ankostis commented 9 years ago

I'm also affected by this behavior, and I cannot substitute ConsoleZ for Console2 extactly because of this.

Please @cbucher reconsider this decision (and thank you for your time).

cbucher commented 9 years ago

I offered tips, workarounds and even coding a new command line option.

If none of these methods is suitable, explain why.

I chose consistency between all the usages. Using the inherited environment block works only with these restrictions:

ankostis commented 9 years ago

Thank you for you clarifying answer. I will try to reply to your points as better as i can.

POSIX processes ALWAYS inherit the FULL environment of their parent process, allowing for all kinds of customizations.

So the central answer to the "why" question is "for consistency!". But for more technical details, read the example use-case, further below.

For the workarounds that you have tirelessly implemented:

Now, for the your last objection, over consistency, i fully agree, it is important.

And one last point: We DO read the docs, thank you again for your efforts in keeping them in-sync.

alexandrul commented 9 years ago

@ankostis beautiful explanation. Since my initial comments, I have found ConEmu which behaves just like Console2. However, it's not perfect:

In the end, I'm using both ConEmu and Console2, while waiting for a future release of ConsoleZ to replace them both.

cbucher commented 9 years ago

POSIX processes ALWAYS inherit the FULL environment of their parent process, allowing for all kinds of customizations.

  1. Windows is not a POSIX OS.
  2. http://linux.die.net/man/2/execve Hopefully, you're wrong, in POSIX you can choose the environment variables you want use when you create a new process.

I'm not sure I understand the "multiple instance" case; you mean when you "clone tab"?

See -reuse option. (But you show me one more technical restriction.)

And one last point: We DO read the docs, thank you again for your efforts in keeping them in-sync.

Developers can believe in Santa Claus or unicorns, but not that ... :wink:

I offered [...] coding a new command line option.

A mini specification: -useenv will use the current environment...

ankostis commented 9 years ago

If i understood it correctly, --useenv is a foreseen enhancement, correct? I would say that tab-settings envs SHOULD override parent envs. I would also suggest this to be the default behavior, and add a --clearenv for the current behavior. But anyhow, the important thing is to be able to inherit envs.

Gracie!

[edit]: To "inherit env by default" provides, among others, for double-clicking ConsoleZ in some file-manager, e.g. TotalCommander, and inherit its env without needing an extra .bat file just for the --useenv option.]

cbucher commented 8 years ago

[edit]: To "inherit env by default" provides, among others, for double-clicking ConsoleZ in some file-manager, e.g. TotalCommander, and inherit its env without needing an extra .bat file just for the --useenv option.]

You suggest a rollback, ignoring consistency, ... (or worst: only for a list of applications)

I will reexplain the consistency problem with pictures. We use a mono instance of ConsoleZ (version 1.11) (I think that is a major usage).

image

image

image

image

At this time, I can only see two clever solutions:

rgfsteed commented 8 years ago

@cbucher

Firstly, I don't want to appear inconsiderate, as I greatly appreciate the work you are contributing and don't expect you to do anything on my behalf. However, I must concur with ankostis, that the default behaviour should be to inherit the parent's environment.

I ran into this issue (and found this discussion) when trying to launch a new console from another application. That application sets a large number of variables relating to the version which is running, licensing, etc. While I could write a custom tab for each application, it seems unnecessarily dificult and invites all sorts of issues.

Launching with the user's default environment is a nice feature, but I feel this should be something one can force with a flag (rather than the other way around). As such, I would suggest the following:

  1. Default behavior. Launching "console" inherits parent.
  2. If the user specifies a tab, the console inherits the parent's environment and the environment is modified based on variables specified for the tab.
  3. A launch flag will specify the default block to be used (or more explicitly a flag to "not" inherit the parent's environment).

Kind regards, Robin

cbucher commented 8 years ago

image

In mono instance, if the shell inherits parent then it inherits environment of the mono instance application ConsoleZ. And mono instance application ConsoleZ inherits environment of the process that launches ConsoleZ for the first time!

ankostis commented 8 years ago

So @cbucher you say that if ConsoleZ has already started, it is not possible to copy env?

cbucher commented 8 years ago

@ankostis I said that cannot be done by inheritance. The only way to propagate the environment block is to dump it and to send it in WM_COPYDATA message. I will try this.

cbucher commented 8 years ago

@ankostis please try beta version

ankostis commented 8 years ago

Yes, it works just fine. Thank you.

Is this beta ready for "distribution" ?

cbucher commented 8 years ago

I do'nt know. I expect feedbacks before considering this version as stable version. By the way, you can help by testing features (issues are marked with a yellow tag "help wanted").

adamsd5 commented 8 years ago

This change is exactly what I was looking for today, thanks! I'll upgrade to 1.16 immediately.

I thought I would add a use case that needs this. We have a git repo that includes "launch_consoleZ.cmd". This script will set an environment variable OUR_ROOT with the path to the repo's location on disk (%~dp0 in batch), then launch consoleZ. Then, many of the commands we've written for the user to run in bash, cmd, powershell rely on %OUR_ROOT% for the base location of various configuration files or scripts. Allowing OUR_ROOT to pass to the tab processes means users can move the location of their git repo, or have many clones in different places, and simply double-click the "launch_consoleZ.cmd" script that is in the repo they want as the active one. It would be pretty inconvenient to define a new tab type with each cloned location they want.

Thanks again, and I hope this helps you understand one potential use case.

Also, I read the manual before searching on google for my answer. The google search led to this patch request. In my experience, those who don't read the manuals are not the ones likely to post here.

One final thought. I'm indifferent to whether the inheritance is the default or not.