-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Cannot create a new VM on git bash or cygwin #4996
Comments
Probably related to the emergency fix for #4983, don't think it was tested too much on Windows... |
Interesting! Did this behavior change between v1.2.0 and v1.3.0? We're still using the same progressbar library, just a slightly different version of it. I suspect the error is raised here: @cvila84 - I currently don't have a way to duplicate this. My naive solution is to turn off the progressbar if the library returns an error. Do you mind trying to see if returning after we log the error solves the issue for you? After this line: minikube/pkg/util/progressbar.go Line 52 in 40a1a98
Add: return &readCloser{Reader: reader, close: func() error { return nil }} |
On minikube 1.2, it is working with the following display:
On minikube 1.3 with winpty, it is working with the following display, which is quite different:
@tstromberg I will test later your proposal and will let you know |
@cvila84 - Just curious but is there a reason why you are using Cygwin/Git for Bash and not PowerShell or CMD? +1 . Can reproduce this on my machine in Git for Bash. |
Thanks for the update! I took a look at the old progress bar code, and nothing jumped out to me as to how it implemented a fallback from a full progress bar to the smaller version you saw. https://github.com/jimmidyson/go-download/blob/master/download.go It suspect the difference may come down to either the version of the pb library, or our use of pb.NewPool() rather than pb.NewProxyReader() |
@tstromberg -- I tried running after putting the line (which fails the compilation though so I returned without reader) and it is failing there -
|
Apparently this is "normal", as in a difference between winpty and mintty (that MSYS2/Cygwin uses)
(says mintty/mintty#482) So I think you need to file a bug with github.com/cheggaaa/pb, to check for a MinTTY on Windows ? |
This also seems to be happening on Command Prompt (With Admin). I was trying to run integration tests and facing this same issue -
On PowerShell, the tests are not running but on CMD, they seem to be running but then this is happening. |
The |
@tstromberg, I confirm it is related to NewPool instead of NewProxyReader. I tested with v2 and v3, and NewProxyReader is working well. I guess the pool is used because we now want to download in // 2 files for the ISO (the SHA and the actual ISO content). I suspect using a pool requires to lock the console differently as we need to write progress characters on several lines instead of only one with NewProxyReader. A potential fix could be to use 2 "simple" bars instead of a pool of 2. That said, the workaround is fine for me, less for my team mates who rely on a minikube wrapper for their day 2 day work (see our gokube project) which is not able yet to install and run winpty in front of each minikube command. @blueelvis, we are using git bash/cygwin because some of our internal tools are bash scripts, and we don't want to rely on docker to run them, neither port them for the moment in powershell. |
@cvila84 Thanks for the extra context. I propose we:
If someone wants to look at this, feel free to I'd like this to be fixed ASAP and ship as part of v1.3.1 this week. |
/assign @tstromberg - I did some testing today with v3 which doesn't have the Pools but it was causing weird display issues. Let me tinker some more with this. -Pranav |
Any luck? |
Hello,
When using GIT bash or cygwin, it is no more possible to create a new VM the first time as it fails with the following error (and hangs):
Using winpty works but is not very convenient...
The text was updated successfully, but these errors were encountered: