Replace home-env patches with the upstreamed ones
#63
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I should probably rebase (rewording the "Revert" commits into "fixup!" ones) and merge this before rebasing onto MSYS2 runtime v3.5.0, then do a clean rebase onto v3.4.10, as I should most likely branch off a |
In preparation for integrating the patches that made it upstream via https://inbox.sourceware.org/cygwin-patches/[email protected]/, let's revert the original patches from Git for Windows' branch thicket. This reverts commit 81a0e5d. Signed-off-by: Johannes Schindelin <[email protected]>
In preparation for integrating the patches that made it upstream via https://inbox.sourceware.org/cygwin-patches/[email protected]/, let's revert the original patches from Git for Windows' branch thicket. This reverts commit 22854f6. Signed-off-by: Johannes Schindelin <[email protected]>
In preparation for integrating the patches that made it upstream via https://inbox.sourceware.org/cygwin-patches/[email protected]/, let's revert the original patches from Git for Windows' branch thicket. This reverts commit f0d5641. Signed-off-by: Johannes Schindelin <[email protected]>
This patch hails from Git for Windows (where the Cygwin runtime is used in the form of a slightly modified MSYS2 runtime), where it is a well-established technique to let the `$HOME` variable define where the current user's home directory is, falling back to `$HOMEDRIVE$HOMEPATH` and `$USERPROFILE`. The idea is that we want to share user-specific settings between programs, whether they be Cygwin, MSYS2 or not. Unfortunately, we cannot blindly activate the "db_home: windows" setting because in some setups, the user's home directory is set to a hidden directory via an UNC path (\\share\some\hidden\folder$) -- something many programs cannot handle correctly, e.g. `cmd.exe` and other native Windows applications that users want to employ as Git helpers. The established technique is to allow setting the user's home directory via the environment variables mentioned above: `$HOMEDRIVE$HOMEPATH` or `$USERPROFILE`. This has the additional advantage that it is much faster than querying the Windows user database. Of course this scheme needs to be opt-in. For that reason, it needs to be activated explicitly via `db_home: env` in `/etc/nsswitch.conf`. Signed-off-by: Johannes Schindelin <[email protected]>
We should not blindly set the home directory of the SYSTEM account (or of Microsoft accounts) to `/home/<name>`, especially `/etc/nsswitch.conf` defines `db_home: env`, in which case we want to respect the `HOME` variable. Signed-off-by: Johannes Schindelin <[email protected]>
The account under which Azure Web Apps run is an IIS APPOOL account that is generated on the fly. These are special because the virtual machines on which thes Apps run are not domain-joined, yet the accounts are domain accounts. To support the use case where such a Web App needs to call `ssh` (e.g. to deploy from a Git repository that is accessible only via SSH), we do need OpenSSH's `getpwuid (getuid ())` invocation to work. But currently it does not. Concretely, `getuid ()` returns -1 for these accounts, and OpenSSH fails to find the correct home directory (_especially_ when that home directory was overridden via a `db_home: env` line in `/etc/nsswitch.conf`). This can be verified e.g. in a Kudu console (for details about Kudu consoles, see https://github.com/projectkudu/kudu/wiki/Kudu-console): the domain is `IIS APPPOOL`, the account name is the name of the Azure Web App, the SID starts with 'S-1-5-82-`, and `pwdgrp::fetch_account_from_windows()` runs into the code path where "[...] the domain returned by LookupAccountSid is not our machine name, and if our machine is no domain member, we lose. We have nobody to ask for the POSIX offset." Since these IIS APPPOOL accounts are relatively similar to AzureAD accounts in this scenario, let's imitate the latter to support also the former. Reported-by: David Ebbo <[email protected]> Helped-by: Corinna Vinschen <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
In the very early code path where `dll_crt0_1 ()` calls `user_shared->initialize ()`, the Cygwin runtime calls `internal_pwsid ()` to initialize the user name in preparation for reading the `fstab` file. In case `db_home: env` is defined in `/etc/nsswitch.conf`, we need to look at the environment variable `HOME` and use it, if set. When all of this happens, though, the `pinfo_init ()` function has had no chance to run yet (and therefore, `environ_init ()`). At this stage, therefore, `getenv ()`'s `findenv_func ()` call still finds `getearly ()` and we get the _verbatim_ value of `HOME`. That is, the Windows form. But we need the "POSIX" form. To add insult to injury, later calls to `getpwuid (getuid ())` will receive a cached version of the home directory via `cygheap->pg.pwd_cache.win.find_user ()` thanks to the first `internal_pwsid ()` call caching the result via `add_user_from_cygserver ()`, read: we will never receive the converted `HOME` but always the Windows variant. So, contrary to the assumptions made in 27376c6 (Allow deriving the current user's home directory via the HOME variable, 2023-03-28), we cannot assume that `getenv ("HOME")` returned a "POSIX" path. This is a real problem. Even setting aside that common callers of `getpwuid ()` (such as OpenSSH) are unable to handle Windows paths in the `pw_dir` attribute, the Windows path never makes it back to the caller unscathed. The value returned from `fetch_home_env ()` is not actually used as-is. Instead, the `fetch_account_from_windows ()` method uses it to write a pseudo `/etc/passwd`-formatted line that is _then_ parsed via the `pwdgrp::parse_passwd ()` method which sees no problem with misinterpreting the colon after the drive letter as a field separator of that `/etc/passwd`-formatted line, and instead of a Windows path, we now have a mere drive letter. Let's detect when the `HOME` value is still in Windows format in `fetch_home_env ()`, and convert it in that case. For good measure, interpret this "Windows format" not only to include absolute paths with drive prefixes, but also UNC paths. Signed-off-by: Johannes Schindelin <[email protected]>
dscho
force-pushed
the
clean-up-home-env-patches
branch
from
20b50f0
to
1b8d886
Compare
Merged
|
/open pr The workflow run was started |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In a pretty long effort, I managed to upstream these patches in https://inbox.sourceware.org/cygwin-patches/[email protected]/#t. They do look quite a bit different than I originally wrote them, so let's swap the old versions out in favor of the new versions.