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

Error on fresh mini install #1726

Closed
Stanzilla opened this issue Mar 23, 2018 · 34 comments
Closed

Error on fresh mini install #1726

Stanzilla opened this issue Mar 23, 2018 · 34 comments
Assignees

Comments

@Stanzilla
Copy link
Member

Just writing it down, was testing something and it errored on a fresh zip:

ERROR: C:\Users\Benjamin\Desktop\cmder_mini\vendor\git-for-windows\cmd\git.exe
CMDER Shell Initialization has Failed
Creating initial user-aliases store in "C:\Users\Benjamin\Desktop\cmder_mini\config\user-aliases.cmd"...
        1 file(s) copied.
@daxgames
Copy link
Member

@Stanzilla - What build was this?

@Stanzilla
Copy link
Member Author

Stanzilla commented Mar 23, 2018

@daxgames https://ci.appveyor.com/project/MartiUK/cmder/build/1.0.605-master/artifacts

note it's the cmder_mini package, so no git on board.

@Stanzilla
Copy link
Member Author

Ah, this maybe? #1497

@daxgames
Copy link
Member

daxgames commented Mar 24, 2018

@Stanzilla agree its probably #1497 but I'm not sure my C++ skills are up to adding git.exe detection to cmder.exe in order to get %GIT_INSTALL_ROOT% set in time for the tasks to be able to use it. At least without a TON of time fumbling to figure out how to do it.

Maybe @DRSDavidSoft has some cycles since I believe he has more C++ experience and he contributed the git detection code to init.bat?

Another option would be to add another cmder.exe command line arg to point to a user installed git and set the env var before launching conemu if specified and found and thus skipping init script git .exe detection.

@Stanzilla
Copy link
Member Author

Can we not run init.bat in the task before we want to open the actual console?

@daxgames
Copy link
Member

daxgames commented Mar 24, 2018

I have thought of that in the past as an option just never tried it. Might work, init.bat might need some re-work, but it should work. If it does work it would solve a lot of issues inhale been thinking about tackling. For other shells.

@DRSDavidSoft
Copy link
Contributor

@daxgames I believe the best practice is to move as much as possible from the C++ launcher to init.bat, that way it would be more customizable.

My best best is to use the launcher as an interface for init.bat that only receives the arguments, and pass it directly to the init.bat.

@daxgames
Copy link
Member

@DRSDavidSoft. It doesn't work that way. The only way the launcher can pass info to the init.bat is through env vatlriables because the launcher does not directly launch the task conemu does and init.bat is currently only for cmd sessions not bash or PowerShell. @Stanzilla suggestion of running init.bet in each task before calling the shell might be a solution to set env variables before launching other shells.

@DRSDavidSoft
Copy link
Contributor

Then running init.bat in each session should be considered (although it makes each session slightly slower, but I'm all for it.) Clink could be avoided for bash shells by I imagine it would be useful from the poweshell shells.

As discussed in #62, the launcher script was originally meant as a fix to #39, to be able to be pinned to the taskbar. So, I think doing a complete rewrite of it should be in order, while focusing more on the init.bat.

Also, setting the environmental variables are not the only way to pass arguments to init.bat (though it may be the best.) We could also use a separate config file as discussed in #1695 – I imagine the variables won't change that often and init.bat can generate the rest (I do know that some should be generated on fly.)

I can work on the launcher script, even use some outline from @Maximus5's source code for ConEmu launcher. (I would also like to add DPI-awareness and Win32's TaskDialog API to the launcher's code, like what Maximus5 has currently written for ConEmu.)

In any case, we should consider first whether including init.bat in each session should be done or not. I vote for it to be done.

@DRSDavidSoft
Copy link
Contributor

@daxgames We could also switch to C# as it's more versatile, though I'm not sure if that would be acceptable by adding yet another dependency (.NET) just for the sake of the launcher.

@daxgames
Copy link
Member

daxgames commented Mar 25, 2018

👎 on C# because of the extra dependencies. I'm not sure a rewrite of the launcher is necessary I certainly don't have time to do it. Kinda like killng a fly with a sledgehammer.

Maybe we do not run the entire init.bat in all shell tasks because as you said clink is not needed neither are the Doskey macros nor are the other cmd init scripts. The sub routines are now libs so we could write a smaller more targeted bash_init.bat that sets env vars that would then be available to the bash tasks and shell. If one is necessary for powershell maybe it gets written too.

@Stanzilla
Copy link
Member Author

The sub routines are now libs so we could write a smaller more targeted bash_init.bat that sets env vars that would then be available to the bash tasks and shell. If one is necessary for powershell maybe it gets written too.

yeah that was my thinking

@DRSDavidSoft
Copy link
Contributor

Yeah, I personally hate the extra dependencies as well. Since init.bat is now more modular, let's focus on targeting it for more shells than Cmd. However, let's make it all in one file, and not break it to bash_init.bat, ps_init.bat, etc. It could be elegantly done in one file using recent libs.

I'll focus on a much cleaner launcher code, like you said it's not a priority but there are lots that can be improved in it.

@Stanzilla Please keep this issue open for now until both points are addressed in future issues/PRs.

@Stanzilla
Copy link
Member Author

I haven't found out how to run a bat in a task yet though

@daxgames
Copy link
Member

@Stanzilla - Just an idea but we could add a command line arg to init.bat of shell and have it launch PowerShell or bash. So each task would call init.bat as cmder tasks do today then launch the respective bash or PowerShell shell as each task does today based on the shell command line arg provided in the task. That is how I envisioned it working when I considered it as an option a while ago.

@Stanzilla
Copy link
Member Author

Yeah that makes more sense, I was trying to chain bat calling and the actual task but that did not work out well.

@DRSDavidSoft
Copy link
Contributor

@daxgames 👍 something like init.bat -shell ps or init.bat -shell bash and by default, -shell cmd would be used

@jods4
Copy link

jods4 commented Mar 26, 2018

I upgraded my cmder with today's Appveyor build: 1.3.6-pre1.

It was working fine before but now when starting up I get the same error message:

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

I guess it's the same bug as this one. It's a regression from 1.3.5.
I have the mini install of cmder, so no vendor git, as I have a frequently updated git installed globally (on %PATH%).

@daxgames
Copy link
Member

There's also a bug in the bash cmder.sh init script that might over write an existing user-profile.sh in the nightly. Please do not use it until I fix it.

@jods4
Copy link

jods4 commented Mar 26, 2018

Just to clarify: my default task is cmd, not bash or powershell.

@BrianAllred
Copy link

👎 on C# because of the extra dependencies.

I don't have a dog in this fight, I just wanted to point out CoreRT.

@DRSDavidSoft
Copy link
Contributor

@BrianAllred, thanks for pointing this out, can CoreRT meet the following requirements IYO?

  • small footprint & fast to load and execute
  • easy syntax to develop (is it the same as C#?)
  • have GUI support as well
  • zero to no dependencies for the final user
  • simple compilation toolchain to be integrated into AppVeyor ci

If so, it could be a nice replacement for the current C++ launcher.
(Though I still prefer the native Win32 builds with Visual Studio.)

On another hand, (and sorry ahead for the long comment) as suggested here, it still needs the visual studio redistribute DLLs. Although I'm sure everyone has them (or could easily install them), someone has created a fork of Cmder embedding them, which is not recommendded.

It's still a valid reason to use compiled C++ launcher, because ConEmu (the underlying terminal emulator that Cmder uses) is built also with VS flavor of C++ with Win32 API.

The best scenario I think of is to always use rhe same build tools that ConEmu use to avoid multiple DLL required, and move as much as appreciate logic into init.bat, which makes Cmder universal with other underlying emulators.

The launcher works as a really nice way to add switches, pinned taskbar tasks and other windows functions that can not or should not be emulated with batch files.

@Stanzilla
Copy link
Member Author

Stanzilla commented Mar 28, 2018

I'd rather not switch from c++ at this point.

@DRSDavidSoft
Copy link
Contributor

@Stanzilla yes, me too.

@BrianAllred
Copy link

  • small footprint & fast to load and execute

It integrates any referenced code into the executable, so I can't see it being slow. As fast as C++, maybe, maybe not.

  • easy syntax to develop (is it the same as C#?)

It should be C# 7 compatible, although I'm sure there are unimplemented APIs since it's a relatively new project.

  • have GUI support as well

Doubtful, at least not natively. Decent C# native GUIs are non-existent right now; probably my biggest gripe with such an otherwise awesome language.

  • zero to no dependencies for the final user

The CoreRT compiled code is native to the targeted platform, so there should be zero user dependencies.

  • simple compilation toolchain to be integrated into AppVeyor ci

I'm not very familiar with AppVeyor, but CoreRT should be supported by msbuild, which is an easy to use command line compiler for C#.

So all of that being said, it's probably not a viable replacement right now. Might be something to watch as it matures, though.

@Stanzilla
Copy link
Member Author

Still happening with #1739 merged btw

@daxgames
Copy link
Member

daxgames commented Apr 5, 2018

I don't know if we can fix this. Nothin it tried worked to my satisfaction. I think the solution would be to add fit detection to the launcher but I'm non up to trying that one. Launching init.bat with a shell art would work but had some realot bad side effects.

@Stanzilla
Copy link
Member Author

What exactly is the problem?

@daxgames
Copy link
Member

daxgames commented Apr 7, 2018

it may have been a problem that was always there and we shouldn't fix it

the problem is is that the bash task calls Bash from the fit for Windows vendored and if you have cmder mini there is no any vensored git

@Stanzilla
Copy link
Member Author

Yeah that's what I figured but there is no way to fix that if we don't change something during our build process so the package in _mini is actually different from the full one.

@Stanzilla
Copy link
Member Author

Also I guess this is a duplicate of #1497

@DRSDavidSoft
Copy link
Contributor

@Stanzilla Was something that errored bash task on Cmder mini? just to be sure.

@Stanzilla
Copy link
Member Author

@DRSDavidSoft No, it was git not being there on cmder_mini which is basically the same problem

@DRSDavidSoft
Copy link
Contributor

@Stanzilla could you rename the issue title to "Error on fresh mini install"?

@Stanzilla Stanzilla changed the title Error on fresh install Error on fresh mini install Apr 8, 2018
@daxgames daxgames mentioned this issue Apr 13, 2018
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

No branches or pull requests

5 participants