Support OneDrive better #69
Merged
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
Chances are high that Cygwin recalls offline files from remote storage, even if the file is only accessed during stat(2) or readdir(3). To avoid this - make sure Cygwin is placeholder-aware, - open files in path_conv handling, as well as in stat(2)/readdir(3) scenarios with FILE_OPEN_NO_RECALL, and - during symlink checking or testing for executablility, don't even try to open the file if one of the OFFLINE attributes is set. Reported-by: Marcin Wisnicki <mwisnicki@gmail.com> Signed-off-by: Corinna Vinschen <corinna@vinschen.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Add FILE_OPEN_NO_RECALL to NtOpenFile calls trying to fetch or write file security descriptors so as not to recall them from offline storage inadvertently. Signed-off-by: Corinna Vinschen <corinna@vinschen.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
force-pushed
the
better-onedrive-support
branch
from
d805f80 to
6380bea
Compare
If FILE_DIRECTORY_FILE is given, FILE_OPEN_NO_RECALL is not allowed,
otherwise NtCreateFile returns STATUS_INVALID_PARAMETER.
Drop FILE_OPEN_NO_RECALL where FILE_DIRECTORY_FILE is specified.
Fixes: f6b56abec186 ("Cygwin: try to avoid recalling offline files")
Reported-by: Bruce Jerrick <bmj001@gmail.com>
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
w32api 12.0.0 adds the returns_twice attribute to RtlCaptureContext(). There's some data-flow interaction with using it inside a while loop which causes a maybe-uninitialized warning. ../../../../winsup/cygwin/exceptions.cc: In member function 'int _cygtls::call_signal_handler()': ../../../../winsup/cygwin/exceptions.cc:1720:33: error: '<anonymous>' may be used uninitialized in this function [-Werror=maybe-uninitialized] Backported-from: 7e3c833592 (Cygwin: suppress a warning generated with w32api >= 12.0.0, 2024-06-07) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
rimrul
approved these changes
Jul 9, 2024
In preparation for integrating the patches that made it upstream via https://inbox.sourceware.org/cygwin-patches/cover.1684753872.git.johannes.schindelin@gmx.de/, let's revert the original patches from Git for Windows' branch thicket. This reverts commit 81a0e5d. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In preparation for integrating the patches that made it upstream via https://inbox.sourceware.org/cygwin-patches/cover.1684753872.git.johannes.schindelin@gmx.de/, let's revert the original patches from Git for Windows' branch thicket. This reverts commit 22854f6. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In preparation for integrating the patches that made it upstream via https://inbox.sourceware.org/cygwin-patches/cover.1684753872.git.johannes.schindelin@gmx.de/, let's revert the original patches from Git for Windows' branch thicket. This reverts commit f0d5641. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
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 <david.ebbo@gmail.com> Helped-by: Corinna Vinschen <corinna@vinschen.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
After 7 or so years, I got the original `home-env` patch series landed in Cygwin. This topic branch reverts my original patches and replaces them with the ones that were backported from Cygwin. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We've upgraded our cross compiler to gcc 13 now, so the workaround can be removed This is a backport from msys2/msys2-runtime. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
It is used to pass strings/paths to the preprocessor and breaks for example the CPython build. For example -DPREFIX='"/ucrt64"'. Fixes msys2/msys2-runtime/#190 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
If non-cygwin process is executed in console, the exit code is not set correctly. This is because the stub process for non-cygwin app crashes in fhandler_console::set_disable_master_thread() due to NULL pointer dereference. This bug was introduced by the commit: 3721a75 ("Cygwin: console: Make the console accessible from other terminals."), that the pointer cons is accessed before fixing when it is NULL. This patch fixes the issue. Backported-from: aa73e11 (Cygwin: console: Fix exit code for non-cygwin process., 2024-02-02) Fixes: 3721a75 ("Cygwin: console: Make the console accessible from other terminals.") Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
If disable_master_thread flag is set between the code checking that flag not be set and the code acquiring input_mutex, input record is processed once after setting disable_master_thread flag. This patch prevents that. Backported-from: 9bcfd06 (Cygwin: console: Avoid slipping past disable_master_thread check., 2024-02-03) Fixes: d4aacd5 ("Cygwin: console: Add missing input_mutex guard.") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Non-cygwin app may call ReadFile() for empty pipe, which makes NtQueryObject() for ObjectNameInformation block in fhandler_pipe:: get_query_hdl_per_process. Therefore, do not to try to get query_hdl for non-cygwin apps. Addresses: msys2/msys2-runtime#202 Backported-from: f6be372 (Cygwin: pipe: Give up to use query_hdl for non-cygwin apps., 2024-03-03) Fixes: b531d6b ("Cygwin: pipe: Introduce temporary query_hdl.") Reported-by: Alisa Sireneva, Johannes Schindelin <Johannes.Schindelin@gmx.de> Reviewed-by: Corinna Vinschen <corinna@vinschen.de> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
If pipe reader is a non-cygwin app first, and cygwin process reads the same pipe after that, the pipe has been set to bclocking mode for the cygwin app. However, the commit 9e4d308 assumes the pipe for cygwin process always is non-blocking mode. With this patch, the pipe mode is reset to non-blocking when cygwin app is started. Backported-from: 55431b4 (Cygwin: pipe: Make sure to set read pipe non-blocking for cygwin apps., 2024-03-11) Addresses: https://cygwin.com/pipermail/cygwin/2024-March/255644.html Fixes: 9e4d308 ("Cygwin: pipe: Adopt FILE_SYNCHRONOUS_IO_NONALERT flag for read pipe.") Reported-by: wh <wh9692@protonmail.com> Reviewed-by: Corinna Vinschen <corinna@vinschen.de> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This topic branch brings a couple of fixes that only made it into Cygwin's v3.5 branch but not into the v3.4 branch. However, Git for Windows is still based on v3.4 (at least until Windows 7 and Windows 8 support is dropped after Git for Windows v2.46). So let's backport the fixes. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Technically, we don't. Practically, I'd like to have them, also to exercise the new |
This topic branch backports the improvements of Cygwin where it no longer tries to download file contents from OneDrive just to list the directory contents including faked executable permissions. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
/open pr The workflow run was started |
github-actions bot
pushed a commit
to git-for-windows/build-extra
that referenced
this pull request
Jul 9, 2024
Git Bash's `ls` command [can now be used in OneDrive-managed folders](git-for-windows/msys2-runtime#69) without having to hydrate all the files. Signed-off-by: gitforwindowshelper[bot] <gitforwindowshelper-bot@users.noreply.github.com>
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.
This backports patches that avoid hydrating files on OneDrive just to
stat()them.See also msys2/msys2-runtime#206.