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

Default bash tasks in ConEmu don't work in mini installations #1497

Closed
jordancoll opened this issue Sep 23, 2017 · 28 comments
Closed

Default bash tasks in ConEmu don't work in mini installations #1497

jordancoll opened this issue Sep 23, 2017 · 28 comments
Labels
👀 Awaiting Response Waiting to hear back from the issue reporter. 🎨 Type: Enhancement

Comments

@jordancoll
Copy link

The default bash::* tasks try to launch %ConEmuDir%\..\git-for-windows\bin\bash

<value name="Cmd1" type="string" data="cmd /c &quot;%ConEmuDir%\..\git-for-windows\bin\bash&quot; --login -i -new_console:d:%USERPROFILE%"/>

I suggest changing the default tasks to use GIT_INSTALL_ROOT as it should point to the user's preferred git installation.

@Stanzilla
Copy link
Member

%GIT_INSTALL_ROOT% is not set at that time though :/

@i-s-o
Copy link

i-s-o commented Mar 24, 2018

@Stanzilla is %GIT_INSTALL_ROOT% a cmder-specific env var? I always get confused by this one because it sounds like it is set by git-for-windows. I expected it to be called something like %cmder_git_root% if it was set by cmder.

@Stanzilla
Copy link
Member

Yeah, we set it in init.bat

@DRSDavidSoft
Copy link
Contributor

@Stanzilla @daxgames How about not specifying anything, and launch bash from user's %PATH%?

I mean should the user have installed any other bash (WSL, msys2, Cygwin) without a Git, it should run that too.

@daxgames
Copy link
Member

daxgames commented Apr 7, 2018

The default bash task is designed to work with vendored git. So the path is correct and required. We can't launch a get that we have not discovered yet and so far the only Discovery we have is in the Annette. Bat for command sessions so I don't know what to say.

@daxgames
Copy link
Member

daxgames commented Apr 7, 2018

why don't the users that want to use mini just edit the task to point to their own Bash

@DRSDavidSoft
Copy link
Contributor

@daxgames We should devise a plan on Cmder first launch:

  1. Cmder full: use the vendored git unchanged
  2. Cmder mini:
    a) Git exists on any directory in path: modify ConEmu.xml to use that
    b) No Git or Bash exist at all: remove the option from ConEmu.xml

How about it?

@Stanzilla
Copy link
Member

That would mean we have to change the file during the build process and make that way more complex, no?

@DRSDavidSoft
Copy link
Contributor

@Stanzilla not necessary on build time, it can be done on runtime. On each startup, if bash is not found in the predefined path, the item should disappear.

I personally prefer not to have unnecessary changes between the full and minimal version, and keep the differences to a minimal.

@Stanzilla
Copy link
Member

Yeah agree

@daxgames
Copy link
Member

daxgames commented Apr 7, 2018

@drdavidsoft I think that all sounds great but how do intend on making this first launch magic happen? And it does not have to be done each time under starts, just the first time.

@DRSDavidSoft
Copy link
Contributor

@daxgames It can easily be made from a sample file, if the user file does not exist then make a user config from the sample. However, I do strongly recommend checking before each startup, or at least giving the user an option to rebuild it manually. Think of a scenario when user installs Cmder mini then proceeds to install WSL or msys2.

(P.S. my username is DR S David Soft)

@daxgames
Copy link
Member

daxgames commented Apr 7, 2018

@DRSDavidSoft That means maintaining two different 'default' conemu.xml files. One for cmder and one for cmder mini. It's already a pain to maintain one copy of a change is needed.

I think we just need to add a section to the readme.md explaining the task is there for the full cmder but can be altered or added to in the case a user has a prior installed bash shell. I think were trying to fix something that does not need fixing but might need documenting. Another option is another command line art for the launcher where a user can specify a git install root but I'm not real hip on the idea because as you have said there are many different bash shells that can be installed and not all Will launch exactly the same way.

Maybe we even rename the vendored menu option to cmder bash to denote the built in nature of the censored git-for-windows.

@DRSDavidSoft
Copy link
Contributor

@daxgames While documenting this issue in README is an acceptable alternative to fixing a problem that we're sure not a lot of people might occur, I think it's still a better solution to try and fix it IF a) the fix is easy and b) it doesn't break other parts of Cmder.

Regarding in naming it cmder bash I'd say it would be an appropriate name because the way cmder currently handles it is by only calling vendored Git's bash.

This is acceptable behavior to many users of Cmder, and this issue can be closed. However, I'd like to ask to write a fix for it, by changing a small portion of Cmder logic: creating the ConEmu.xml based on user machine's configuration. That way a better (an easier) integration between Cmder and external shells installed on user's machine is guaranteed.

@daxgames
Copy link
Member

daxgames commented Apr 8, 2018

@DRSDavidSoft. PRs are always welcome and no permission required. Just understand the only time the default conemu.xml is used or references is on initial install. Just don't break the current handling of the conemu.xml after that initial install.

@Stanzilla
Copy link
Member

Why not build the problem into the error message? If it says "CMDER Shell Initialization has Failed, are you trying to use bash in cmder_mini?" Or something like that, it should be pretty clear?

@DRSDavidSoft
Copy link
Contributor

@Stanzilla that'd also make sense,

  • if Git is not found don't initialize it
  • if bash is selected but not found, show sensible error
  • on first launch if mini version, remove bash from tasks

That way we'd avoid this issue on fresh installs, and won't introduce breaking changes to previous installs by displaying an appropriate message.

@vBm
Copy link

vBm commented Apr 11, 2018

I have exactly the same issue.
I agree with @DRSDavidSoft that issue should be fixed rather than explaining in the README about possible workaround being that this wasn't an issue before the last set of changes (thus I see this as an regression which broke the previous behavior).

@Stanzilla
Copy link
Member

How exactly did 1.3.5 do this? Because it's actually a regression

@jods4
Copy link

jods4 commented Apr 11, 2018

This is a regression indeed.
I had 1.3.5 and it worked fine. I upgraded to 1.3.6-pre1, did not change my config at all and now I have this error.

It's not exclusively related to bash. I have the same error in all my tasks: cmd, powershell, bash, doesn't matter.

@daxgames
Copy link
Member

@jods4. What error?

@jods4
Copy link

jods4 commented Apr 12, 2018

@daxgames

ERROR: C:\ProgramData\Cmder\vendor\git-for-windows\cmd\git.exe
CMDER Shell Initialization has Failed

Also, see #1726. This is where I come from and it was closed as a duplicate of this one.

@daxgames
Copy link
Member

@jods4. You say you get this error in all shells not just a cmder shell?

@daxgames daxgames mentioned this issue Apr 13, 2018
@jods4
Copy link

jods4 commented Apr 13, 2018

@daxgames I mean in all kind of tasks inside Cmder.

@vBm
Copy link

vBm commented Apr 16, 2018

@daxgames
#1745 does fix this issue for me, thank you for that.

@Stanzilla
Copy link
Member

Stanzilla commented Apr 19, 2018

The latest ConEmu build (https://conemu.github.io/blog/2018/04/16/Build-180416.html) has an interesting bug fix:

conemu#1513: Fix erroneously trimmed leading " in commands  cmd /c "-new_console script.bat argument".

maybe that was the reason why I could not make it work before. Edit: nope.

@stale
Copy link

stale bot commented May 25, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contribution.

@stale stale bot added the 👀 Awaiting Response Waiting to hear back from the issue reporter. label May 25, 2019
@stale
Copy link

stale bot commented Jun 1, 2019

This issue has been automatically closed due to it not having any activity since it was marked as stale. Thank you for your contribution.

@stale stale bot closed this as completed Jun 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 Awaiting Response Waiting to hear back from the issue reporter. 🎨 Type: Enhancement
Projects
None yet
Development

No branches or pull requests

7 participants