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

Add dev directory during setup #255

Merged
merged 5 commits into from Sep 2, 2019
Merged

Add dev directory during setup #255

merged 5 commits into from Sep 2, 2019

Conversation

stepro
Copy link
Contributor

@stepro stepro commented Aug 30, 2019 •
edited

A minimal fix for git-for-windows/git#2291.

Note no change in mingit because that is just git.exe, and the missing /dev directory is only a problem for bash.exe.

Built and validated the regular installer, NuGet package and portable installers.

@stepro stepro mentioned this pull request Aug 30, 2019
Closed
1 task
stepro and others added 5 commits September 2, 2019 11:33
@stepro @dscho
installer/portable: add /dev/ directory during setup
b0c40ad 
In b0c033bf3 (mkdir: always check-for-existence, 2019-06-03), Cygwin
alters the behavior of `mkdir` such that it will no longer create an
entry in the Windows filesystem for Cygwin-only directories like
`/dev/`. This commit was introduced in Git for Windows as part of the
upgrade to by moving to msys2-runtime v3.0.7 patch 3.

Previously, `mkdir` succeeded, allowing the post-install script
`01-devices.post` to both create `/dev` and successfully add files like
`/dev/fd` to it. Now, it fails to create the directory with the error

	mkdir: cannot create directory '/dev': File exists

(although this error is not seen because it pipes stderr to
`/dev/null`), and the script proceeds because `/dev` does effectively
exist and passes the existence check. However, by not being an actual
directory in the Windows filesystem, Cygwin doesn't allow creation of
any files in that directory, and returns a "Read-only file system"
error. Therefore creation of the `shm` and `mqueue` directories fail,
and adding the various `/dev/fd` links also fails. When this is called
by `post-install.bat`, the output is:

	"running post-install"
	mkdir: cannot create directory '/dev/shm': Read-only file system

	Creating /dev/shm directory failed.
	POSIX semaphores and POSIX shared memory will not work

	mkdir: cannot create directory '/dev/mqueue': Read-only file system

	Creating /dev/mqueue directory failed.
	POSIX message queues will not work

	ln: failed to create symbolic link '/dev/stdin': Read-only file system
	ln: failed to create symbolic link '/dev/stdout': Read-only file system
	ln: failed to create symbolic link '/dev/stderr': Read-only file system
	ln: failed to create symbolic link '/dev/fd': Read-only file system
	'C:\windows\system32\drivers\etc\hosts' -> '/etc/hosts'
	'C:\windows\system32\drivers\etc\protocol' -> '/etc/protocols'
	'C:\windows\system32\drivers\etc\services' -> '/etc/services'
	'C:\windows\system32\drivers\etc\networks' -> '/etc/networks'

Because this script has no error handling, the Git for Windows installer
thinks that `post-install.bat` succeeded when really it should have
failed.

As per https://cygwin.com/ml/cygwin/2019-08/msg00339.html, Cygwin
recommends that we create that directory as part of the installation, so
now we do.

Signed-off-by: Stephen Provine <stephpr@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@stepro @dscho
nuget: add /dev/
db6dee5 
Just like the fix to the portable Git and the Git for Windows installer,
we now also create that directory in the NuGet package.

Signed-off-by: Stephen Provine <stephpr@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
installer/portable/nuget: ensure /dev/mqueue and /dev/shm exist
9a0af73 
Even if these directories are write-protected in `C:\Program Files\Git`,
they are actually visible in the virtualized `/dev/` filesystem, and
those directories need to exist for POSIX semaphores to be emulated
correctly by the Cygwin/MSYS2 runtime.

Rather than relying on the `01-devices.post` script to create them,
let's create them ourselves.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
installer: fix --debug mode of the release script
b253764 
When testing out pages via `./installer/release.sh -d <page-name>`, we
really need the script to work, which means that we have to give it that
`SetSystemConfigDefaults()` function provided by the `config.iss` file.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
installer: also create /dev/{fd,stdin,stdout,stderr}
02cb1e0 
These symbolic links are expected by at least some MSYS2 programs, and
are therefore generated via the `01-devices.post` script.

However, rather than relying on the post-install script to create them,
we can do this ourselves.

Note that these are the symlinks emulated by Cygwin: system files that
start with the special prefix `!<symlink>\xff\xfe` in all-ASCII,
followed by the symlink target in UTF-16 format including the trailing
(UTF-16) NUL.

As 7-Zip and .zip files do not retain the system bit (at least not
reliably), we cannot do much about this in the portable Git and the
NuGet edition of Git for Windows.

This fixes git-for-windows/git#2291

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho merged commit a15a4b2 into git-for-windows:master Sep 2, 2019
@dscho
Copy link
Member

dscho commented Sep 2, 2019

Thank you for your contribution!

I augmented the commit messages, spend a couple of hours to get the CreateCygwinSymlink() function right and to verify that everything works as intended.

dscho added a commit that referenced this pull request Sep 2, 2019
@dscho
Mention Bug Fix in release notes
f3ef5d1 
The shell construct `<(<command>)`, which was broken in
v2.23.0 (`/dev/fd/<n>: no such file or directory`), [was
fixed](#255).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@stepro
Copy link
Contributor Author

stepro commented Sep 3, 2019

Thanks @dscho. Are you thinking a patch release for this fix or wait until 2.24?

@dscho
Copy link
Member

dscho commented Sep 4, 2019

@stepro before I even think about that, I want to hear a couple of confirmations that the latest snapshot works as intended.

@stepro
Copy link
Contributor Author

stepro commented Sep 4, 2019

I tried the installer and it worked fine for me.

@jeremiahsnapp jeremiahsnapp mentioned this pull request Sep 12, 2019
Closed
dscho added a commit to dscho/build-extra that referenced this pull request Nov 5, 2019
@dscho
installer: use locale-independent way to create Cygwin symlinks
ec601d9 
Since git-for-windows#255, we create
the `/dev/{fd,stdin,stdout,stderr}` symlinks in the installer
explicitly, without relying on Git Bash's initial `post-install`
scripts.

However, the method chosen to create those (Cygwin-style) symlinks seems
to work only in US locales.

Let's jump through another hoop and make sure that it works also in
non-US locales.

The approach implemented in this patch was inspired by
https://stackoverflow.com/questions/38617829/writing-binary-file-in-inno-setup

This fixes git-for-windows/git#2388

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho mentioned this pull request Jan 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@stepro @dscho