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.

Sign up for GitHub

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

Jump to bottom

Avoid access violations when running Bash with TERM=msys #222

Closed
dscho opened this issue Jun 23, 2015 · 13 comments · Fixed by msys2/MSYS2-packages#275
Closed

Avoid access violations when running Bash with TERM=msys #222

dscho opened this issue Jun 23, 2015 · 13 comments · Fixed by msys2/MSYS2-packages#275
Assignees
@dscho
Labels
bug msys2
Milestone
First release of ...

Comments

@dscho
Copy link
Member

dscho commented Jun 23, 2015

We used to set the environment variable TERM=msys in Git for Windows 1.x, so we should handle it gracefully with Git for Windows 2.x, too. Certainly an access violation is the wrong thing to do, ideally we handle msys as a synonym to cygwin.
@dscho dscho self-assigned this Jun 23, 2015
@Alexpux
Copy link

Alexpux commented Jun 25, 2015

ncurses don't have msys terminal so need always use cygwin

@dscho
Copy link
Member Author

dscho commented Jun 25, 2015

It does not crash when TERM=xterm. In fact, it seems it only crashes whenever TERM is set to a value that does not correspond to a directory in /usr/share/terminfo/??/. That is probably a problem we want to fix in any case.

@dscho
Copy link
Member Author

dscho commented Jun 25, 2015

@Alexpux BTW why is ncurses-devel a build dependency, but ncurses not a runtime dependency?

@Alexpux
Copy link

Alexpux commented Jun 25, 2015

Dependency of what?

@Alexpux
Copy link

Alexpux commented Jun 25, 2015

Package ncurses itself is in group base so it always present in MSYS2 from the installer.

@dscho
Copy link
Member Author

dscho commented Jun 25, 2015

Dependency of what?

Whoops, sorry: bash (I still think of this ticket as "Avoid access violations when running Bash").

@Alexpux
Copy link

Alexpux commented Jun 25, 2015

bash is linked fully static, so it depends only from msys-2.0.dll

@dscho
Copy link
Member Author

dscho commented Jun 25, 2015

Package ncurses itself is in group base so it always present in MSYS2 from the installer.

Okay. I generally prefer the dependencies in a package management system to be set up correctly because it is as much documentation as it is necessary for proper process of Pacman.

@dscho
Copy link
Member Author

dscho commented Jun 25, 2015

bash is linked fully static, so it depends only from msys-2.0.dll

That explains it.

@Alexpux
Copy link

Alexpux commented Jun 25, 2015

I'm create bash and pacman packages statically linked to simplify process of update core packages.

@KindDragon KindDragon mentioned this issue Jun 25, 2015
Closed
@KindDragon
Copy link

JFYI we use TERM=msys because of gitextensions/gitextensions#1092

@dscho dscho mentioned this issue Jun 26, 2015
Closed
dscho added a commit to git-for-windows/MSYS2-packages that referenced this issue Jun 26, 2015
@dscho
ncurses: fix crash when TERM refers to an unknown terminal
61838a3 
A but that was recently introduced into the 6.x development branch of
ncurses causes free()d data to be reused by mistake, leading to a crash.

You can verify this by setting TERM to, say, `msys` and then running
`bash.exe --login -i`.

This fixes git-for-windows/git#222

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho mentioned this issue Jun 26, 2015
Merged
@dscho
Copy link
Member Author

dscho commented Jun 27, 2015

@KindDragon good point. I recall that we fixed other, similar issues by setting TERM=msys in the past.

@dscho
Copy link
Member Author

dscho commented Jun 28, 2015

Fixed with msys2/MSYS2-packages#275

@dscho dscho closed this as completed Jun 28, 2015
jeffhostetler pushed a commit to jeffhostetler/git that referenced this issue Nov 13, 2019
@derrickstolee
Merge pull request git-for-windows#222: Merge features/sparse-checkou…
ef9f45f 
…t-2.24.0 into vfs-2.24.0

The `features/sparse-checkout-*` branches were helpful to keep separate from the `vfs-*` branches while the sparse-checkout feature was getting feedback upstream. After the `v2.24.0` update, the feature branch has a clean history and can be merged.

microsoft/VFSForGit#1582 confirmed that we get passing builds using the `-sc` builds. This is not surprising: the features are independent and use different config values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dscho dscho

Labels
bug msys2
Projects
None yet
Milestone
First release of Git for Windows gene...
Development

Successfully merging a pull request may close this issue.

ncurses: fix crash when TERM refers to an unknown terminal git-for-windows/MSYS2-packages
3 participants
@dscho @KindDragon @Alexpux