search

cmderdev / cmder

Lovely console emulator package for Windows
https://cmder.app
MIT License
25.81k stars 2.02k forks source link

[Bug] Got ERROR: Clink initilization has failed with error code: 0 at every start of Cmder since v1.3.21 #2800

Closed BackSpace54 closed 1 year ago

BackSpace54 commented 1 year ago

Version Information

Cmder version: 1.3.21
Operating system: Windows 11 22H2 + december updates

Cmder Edition

Cmder Full (with Git)

Description of the issue

Hi,

since i've updated to the latest Cmder version to date (v1.3.21) and with Clink 1.4.4, i've an error at terminal startup:

Clink v1.4.4.7fe1ca Copyright (c) 2012-2018 Martin Ridgers Portions Copyright (c) 2020-2022 Christopher Antos https://github.com/chrisant996/clink ERROR: Clink initilization has failed with error code: 0

Before Cmder 1.3.21 and with the exact same configuration (clink 1.4.4 i mean) i've no troubles.

The error message is from init.bat line #201 (see here) Have you an idea of what is the problem? Can i delete the condition at line #200?

Btw there's a typo at line #201 it's initialization not initilization ;-)

Bye.

How to reproduce

Use Windows 11 22H2 Update/install Cmder 1.3.21 Install Clink 1.4.1 Set this run command in Windows Terminal cmd.exe /k %SYSTEMDRIVE%\<Cmder directory>\vendor\init.bat Run Cmder tab, at each start got the message ERROR: Clink initilization has failed with error code: 0

Additional context

Got a reply from Clink's author and it's not a Clink bug.

Checklist

chrisant996 commented 1 year ago

@DRSDavidSoft I see the root causes for the two problems:

1. The error level is mistakenly reported as 0.

This happens because line 201 is inside a block that started on line 168.

All environment variables within a block are expanded when the block starts.

So line 201 gets expanded with whatever %errorlevel% was at the moment line 168 was executed, not at the moment when line 201 was executed. The inject happens after line 168, so the errorlevel from it is not what gets shown.

2. Cmder reports that Clink failed to initialize.

This is due to new code that was added to init.bat, and the new code is incorrect.

On 2022/10/18 commit 9399cbdcd7f2b787938946de95b2e07e417fbc9d added the line.

The intent was to find when Clink isn't loaded. But the implementation was to find when Clink inject fails. They aren't equivalent.

The init.bat script assumes that a failed inject means Clink isn't loaded. But if Clink is installed for autorun, then by the time Cmder tries to inject Clink it is already loaded and initialized. So the inject fails, but Clink is in fact successfully initialized.

The short term fix is to delete the code that attempts to detect failure to initialize. There isn't a good long term fix at this time.

DRSDavidSoft commented 1 year ago

Thanks for the thorough investigation, I'll work on the short-term fix.

@chrisant996 Is it possible for Clink to either report a known error level if it's already injected, or set an environment variable like CLINK_INJECTED so that we can detect and bypass re-loading it?

Update: Added commit 9e55c4820006b15d91744e5b6f52c69782db084c to hopefully mitigate this issue.

@BackSpace54 Can you please:

  1. Open init.bat in your current installation, and remove line 200, 201 and 202.
  2. Open Cmder.exe and type set CMDER_ROOT to verify that the path to Cmder root is valid.
  3. Enter this: 
    curl "https://raw.githubusercontent.com/cmderdev/cmder/master/vendor/init.bat" > "%CMDER_ROOT%\vendor\init.bat"
  4. Exit all Cmder Windows, and re-open a Cmder.exe window

Please check if this fixes the issue. Thanks.

@chrisant996 If it's impossible or hard to implement something to tell if Clink is already injected, I'll change the error to a warning, so the execution steps are not stopped on such a failure, and maybe the plain cmd.exe PATH is used. If clink is indeed already loaded, this won't have an effect but displaying a warning message.

BTW is there anything else that you would suggest might need edits, or any additions in init.bat to bring it in par with the scripts in the clink repository? I haven't had time to look at the official Clink loader scripts, so I'd appreciate your feedback.

chrisant996 commented 1 year ago

Clink itself already reports an error if injection fails due to a real error.

What is the motivation behind Cmder adding an additional error message? Offhand it seems like it may be redundant.

@chrisant996 Is it possible for Clink to either report a known error level if it's already injected, or set an environment variable like CLINK_INJECTED so that we can detect and bypass re-loading it?

Environment variables are inherited by every child process, so they are very unreliable for this sort of thing. E.g. once Clink is injected into one cmd.exe instance, all other processes and their children, and their grandchildren, etc all have the environment variable set.

The only thing Clink can do reliably is tell whether the Clink DLL is already loaded into the process. It can't tell whether Clink has been successfully initialized or whether Clink will "work as expected". There are many ways other programs can interfere (e.g. antivirus, or programs that hook APIs and interfere with Clink's API hooks).

Here's what I can do: I can explore the possibility of making inject return 2 for any fatal error that occurs, and only return 1 for non-fatal errors (such as "already loaded"). Then Cmder could use if errorlevel 2 and would interoperate with old and new versions of Clink without creating new problems. However, I need to be cautious about making such a change, because there are several ways it could potentially backfire.

@chrisant996 If it's impossible or hard to implement something to tell if Clink is already injected, I'll change the error to a warning, so the execution steps are not stopped on such a failure, and maybe the plain cmd.exe PATH is used. If clink is indeed already loaded, this won't have an effect but displaying a warning message.

Please don't. 🙃 If Cmder reports it as a warning (or error), then everyone who installed Clink for autorun will always see the warning. Is there a reason Clink's own error reporting is insufficient?

BTW is there anything else that you would suggest might need edits, or any additions in init.bat to bring it in par with the scripts in the clink repository? I haven't had time to look at the official Clink loader scripts, so I'd appreciate your feedback.

Sorry, I don't have the capacity available to do a broad review of the init.bat script.

BackSpace54 commented 1 year ago

  1. Open init.bat in your current installation, and remove line 200, 201 and 202.

    1. Open Cmder.exe and type set CMDER_ROOT to verify that the path to Cmder root is valid.

    2. Enter this:

curl "https://raw.githubusercontent.com/cmderdev/cmder/master/vendor/init.bat" > "%CMDER_ROOT%\vendor\init.bat"
4. Exit all Cmder Windows, and re-open a Cmder.exe window

Please check if this fixes the issue. Thanks.

Well, make this and now got ERROR: Clink initialization has failed with error code: 1 at each start of Cmder. Init.bat print line 209

See below ╭─ C:\Temp at 10:12:12 ╰─ echo %CMDER_ROOT% C:\Applications\Cmder

╭─ C:\Temp at 10:12:16 ╰─ echo %clink_architecture% x64

╭─ C:\Temp at 10:12:20 ╰─ echo %CMDER_CONFIG_DIR% C:\Applications\Cmder\config

╭─ C:\Temp at 10:12:27 ╰─ dir %CMDER_ROOT%\vendor\clink

Click to show... 
19/12/2022  09:35    DIR          .
21/12/2022  08:58    DIR          ..
19/12/2022  09:35                 7 .cmderver
14/12/2022  12:52           147 713 CHANGES
14/12/2022  12:52             1 499 clink.bat
14/12/2022  12:54           823 242 clink.html
14/12/2022  12:52           120 546 clink.ico
14/12/2022  12:52               247 clink.lua
14/12/2022  12:52            34 494 clink_blue.ico
14/12/2022  12:52            34 494 clink_cyan.ico
14/12/2022  12:53         1 831 936 clink_dll_x64.dll
14/12/2022  12:53         1 476 096 clink_dll_x86.dll
14/12/2022  12:52            34 494 clink_gold.ico
14/12/2022  12:52            34 494 clink_gray.ico
14/12/2022  12:52            34 494 clink_green.ico
14/12/2022  12:52            34 494 clink_orange.ico
14/12/2022  12:52            34 494 clink_purple.ico
14/12/2022  12:52            34 494 clink_red.ico
14/12/2022  12:53             4 608 clink_x64.exe
14/12/2022  12:53             4 096 clink_x86.exe
14/12/2022  12:52            35 823 LICENSE
14/12/2022  12:52               539 _default_inputrc
14/12/2022  12:52             1 581 _default_settings

╭─ C:\Temp at 10:12:51 ╰─ dir %CMDER_CONFIG_DIR%

Click to show... 
21/12/2022  10:12    DIR          .
19/12/2022  18:56    DIR          ..
21/12/2022  10:12             5 352 clink.log
21/12/2022  09:00            25 337 clink_history
19/12/2022  19:03             3 423 clink_settings
16/01/2022  22:05             1 959 cmder_prompt_config.lua
18/09/2022  10:11            72 281 flexprompt.lua
19/05/2022  20:42             1 137 flexprompt_autoconfig.lua
31/08/2022  10:27            61 894 flexprompt_modules.lua
20/09/2022  03:27            35 851 flexprompt_wizard.lua
19/12/2022  09:34    DIR          profile.d
19/12/2022  09:34               887 Readme.md
19/05/2022  11:19            54 781 user-ConEmu.xml
19/05/2022  14:07               437 user_aliases.cmd
16/01/2022  22:05               721 user_profile.cmd

╭─ C:\Temp at 10:13:06 ╰─ dir %CMDER_ROOT%\vendor

Click to show... 
21/12/2022  08:58    DIR          .
19/12/2022  18:56    DIR          ..
19/12/2022  09:34    DIR          bin
19/12/2022  09:35    DIR          clink
01/12/2022  22:09    DIR          clink-completions
19/12/2022  09:34            21 549 clink.lua
19/12/2022  09:34               726 clink_settings.default
19/12/2022  09:34             4 016 cmder_exinit
19/12/2022  09:34             2 477 cmder_prompt_config.lua.default
19/12/2022  09:35    DIR          conemu-maximus5
19/12/2022  09:34            56 225 ConEmu.xml.default
19/12/2022  18:41    DIR          git-for-windows
16/01/2022  22:05                29 git-for-windows_1.29.1
19/12/2022  09:34            16 597 init - Copie.bat
21/12/2022  09:05            16 313 init.bat
19/12/2022  09:34    DIR          lib
19/12/2022  09:34             9 725 profile.ps1
19/12/2022  09:34    DIR          psmodules
19/12/2022  09:34                49 Readme.md
19/12/2022  09:34               689 sources.json
19/12/2022  09:34               682 user_aliases.cmd.default
19/12/2022  09:34               740 user_profile.cmd.default
19/12/2022  09:34               902 user_profile.ps1.default
19/12/2022  09:34               301 user_profile.sh.default

╭─ C:\Temp at 10:13:39 ╰─ "%CMDERROOT%\vendor\clink\clink%clink_architecture%.exe" inject --quiet --profile "%CMDER_CONFIG_DIR%" --scripts "%CMDER_ROOT%\vendor

╭─ C:\Temp exit 1 at 10:13:40

clink.log from above command (at 10:13:39) is

Click to show... 
42b0 start_logger              160 ---- 2022/12/21 10:13:39.471 -------------------------------------------------
42b0 start_logger              166 Host process is 'cmd.exe' (pid 17072)
42b0 start_logger              170 DLL path is 'C:\Program Files (x86)\clink'
42b0 start_logger              185 Windows version 10.0.22621 (x64)
42b0 start_logger              187 Clink version 1.4.5.2e84bc (x64)
42b0 history_db::compact      1526 History:  703 active, 391 deleted
42b0 reset_handle              144 resetting mismatched stdout handle
42b0 reset_handle              144 resetting mismatched stderr handle
42b0 history_db::compact      1526 History:  703 active, 391 deleted
chrisant996 commented 1 year ago

@DRSDavidSoft the new commit seems to still have both of the problems I noted earlier.

Using if errorlevel 1 after invoking clink inject isn't ok.

Also, the %errorlevel% is still within a parenthesis grouping, so it will never report an errorlevel accurately.

schittli commented 1 year ago

Good evening

Just some maybe important informations:

If you absolutely want to stay with .bat, then please work that way:

Old:

  if errorlevel 1 (
    …

New:

  IF %errorlevel% GEQ 1 ( 
    …

The new solution compares the real int value. If errorlevel is missing because of the way .cmd handles it, then it still works 😄

Thanks a lot, kind regards, Thomas

chrisant996 commented 1 year ago
  • Maybe it is worth changing from .bat to .cmd. .bat still works on the basis of 16bit MS-DOS with command.com. Actually, it is a miracle that it still works 😄

Huh?

Both .bat and .cmd files are run by cmd.exe on Windows.

  • Additionally, .bat handles ErrorLevel differently: .bat sets ErrorLevel only in case of errors. (Source: StackOverflow)

That is not a general difference -- it applies only to the commands PATH/APPEND/PROMPT/SET/ASSOC, and only when Command Extensions are enabled. And that's the only difference between how cmd.exe runs .bat files versus .cmd files.

Using .bat and .cmd are interchangeable for practical purposes.

If you absolutely want to stay with .bat, then please work that way:

Old:

  if errorlevel 1 (
    …

New:

  IF %errorlevel% GEQ 1 ( 
    …

The new solution compares the real int value. If errorlevel is missing because of the way .cmd handles it, then it still works 😄

I'm sorry, but that is dangerous advice:

Using if errorlevel 1 looks at the exit code from the previous command/program.

Using if %errorlevel% geq 1 looks for an environment variable named "errorlevel". If one exists, then the named environment variable is used and it behaves very differently than if errorlevel 1. If one doesn't exist, then cmd.exe uses the exit code from the previous command/program and it behaves identically to if errorlevel 1.

The suggested approach is unreliable and creates the opportunity for weird malfunctions.

It's safer to use the if errorlevel syntax. It exists for good reasons.

Also: please don't check errorlevel after Clink inject, for the reasons I've shared earlier.

chrisant996 commented 1 year ago

@DRSDavidSoft Clink v1.4.6 was just published and includes returning exit code 2 from clink inject when a fatal error occurs.

Please change the if errorlevel 1 to if errorlevel 2. That way with an updated Clink, the Cmder error message will work as intended, and with older Clink then Cmder will avoid reporting a bogus error.

And there's little point in showing the %errorlevel% in the error message, since it will always be the same value; it is not an error code that can be looked up anywhere for diagnostic purposes, it is simply a "yes/no" value. So, please remove the %errorlevel% mentioned in the error message.

(That way you can sidestep needing to restructure the code to report the actual errorlevel -- since the way it's currently written it prints whatever %errorlevel% was at line 168, which is completely unrelated to the error at line 201. This is because of how variable expansion works in parenthesis groupings. For more info, refer to "Delayed Expansion" for cmd.exe.)

tan-wei commented 1 year ago

Just met the same issue in the latest version of cmder.

baicaisir commented 1 year ago

刚刚在最新版本的cmder中遇到了同样的问题。

+1

DRSDavidSoft commented 1 year ago

🔥 This issue will be fixed soon, I apologize to everyone for taking too long to work on it.

BackSpace54 commented 1 year ago

fire This issue will be fixed soon, I apologize to everyone for taking too long to work on it.

No problem 😺

chrisant996 commented 1 year ago

@DRSDavidSoft just a reminder, this needs a fix.

Why were these lines added?

Options:

  1. If the %printerror% is important for logging purposes, then a simple functional fix is to change if errorlevel 1 ... to if errorlevel 2 ....
  2. Or if the `%printerror% isn't important, then it would be best to just remove the lines.

(Also, see this reply about why some other recent feedback is incorrect and should not be applied.)

DRSDavidSoft commented 1 year ago

@chrisant996 Hi, I'm so sorry for taking so long as I was busy with a couple of IRL things.

Thanks for the explanation, I added this commit to hopefully fix the issue: https://github.com/cmderdev/cmder/commit/e9750ab73d30f6dd60ffe6acfbbe2d74d4096788

Basically, what I understood from your description is that if clink is executed and then is ran into a problem, it will display its own messages and then exit with the appropriate error message, thus it's unnecessary for Cmder (and ill-advised) to print a second message to the screen. The most suitable action it can do is to log the error somewhere, and maybe change its course of action.

My initial intention was to detect if the injection of Clink failed, show some proper error messages to the user, and then revert back to the vanilla Cmd shell with Cmder's prompt (see code in SKIP_CLINK section). Please note that this failure might also arise when the Clink executable is not launched, for example if an anti-virus software prevented Clink from being executed, or if somehow clink.exe was missing on the target machine's Cmder installation, or if the CPU instruction set doesn't match, etc.

For now, I have removed the error message since it was causing more problems and used the error level of 2 to jump to the SKIP_CLINK label. Ideally, I believe I should account for any other error levels (besides 0 and 1) as well.

If the above seems right to you, please let me know so that I can implement it.

Additionally, please review the changes at https://github.com/cmderdev/cmder/commit/e9750ab73d30f6dd60ffe6acfbbe2d74d4096788 and let me know if it fixes this issue.

Tests by the OP @BackSpace54 and also @pmsobrado will be appreciated

pmsobrado commented 1 year ago

Hey @DRSDavidSoft I've been testing the build and the error is not present anymore, but I would like to point out a few things I've noticed about the clink injection behaviour (also present in past builds, not only this new one):

1 - For people like my that already have clink injected on cmd, the history is not available if using cmder. 2 - If I comment line 207 of vendor\init.bat (or 41 of \config\user_init.cmd) "%CMDER_ROOT%\vendor\clink\clink_%clink_architecture%.exe" inject --quiet --profile "%CMDER_CONFIG_DIR%" --scripts "%CMDER_ROOT%\vendor" clink still works, since it is already in the system, and also the command history, but then I lose the lambda symbol in the prompt, and I think colors too.

If I change the --profile "%CMDER_CONFIG_DIR%" to --profile "MY_SYSTEM_CLINK_DIR" then I have the lambda, colors AND my command history.

Running clink autorun show gives me the system autorun config, and I can take from there the profile path so I can see my history on cmder.

Now, why is clink and the lambda/colors of the command window related? Aren't we injecting clink twice, one from the system and one from cmder?

Thanks!

daxgames commented 1 year ago

@DRSDavidSoft TIL what IRL means! :-)

DRSDavidSoft commented 1 year ago

Hi @pmsobrado, since my comment turned out to be so long, I posted it over discussions here: https://github.com/cmderdev/cmder/discussions/2856 as it already was off-topic regarding this issue.

I think @chrisant996 might also have a say, so I invite all to reply over the new discussions page.

@daxgames Cool! BTW I'm planning to next review & merge your amazing speed PR. I have some ideas on how to improve it as well. Sorry it's taking this long, I had multiple projects & problems to deal with. Hopefully I'll be free soon. Thanks! 👍🏻

chrisant996 commented 1 year ago

@pmsobrado @DRSDavidSoft I'll respond in #2856.

pmsobrado commented 1 year ago

@DRSDavidSoft @chrisant996 see you there!

xmedeko commented 1 year ago

Workaround is to add set CMDER_CLINK=0 to the Settings > Environment.

If clink autodetection is difficult, then maybe just add a setting "Do not init clink" or something like that.