-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
FSMonitor fixes #3263
FSMonitor fixes #3263
Conversation
…emon via IPC In FSMonitor v1, we made sure to only use a valid `since_token` when querying the FSMonitor. This condition was accidentally lost in v2, and caused segmentation faults uncovered by Scalar's Functional Tests. I had tried to fix this in https://github.com/git-for-windows/pull/3241, but the fix was incomplete, and I had to follow up with https://github.com/git-for-windows/pull/3258. However, it turns out that both of them were actually only work-arounds; I should have dug deeper to figure out _why_ the `since_token` was no longer guaranteed not to be `NULL`, and I finally did. Signed-off-by: Johannes Schindelin <[email protected]>
Now that we have a correct fix where we guarantee again (just like v1 of the built-in FSMonitor) that `since_token` is not `NULL`, we can revert the work-arounds introduced by these two PRs: - https://github.com/git-for-windows/pull/3241 - https://github.com/git-for-windows/pull/3258 Signed-off-by: Johannes Schindelin <[email protected]>
When flushing the FSMonitor data, we do not actually need to wait for a cookie file. In the worst case, we will over-report a bit. If we _do_ wait for a cookie file, in the worst case we cause a hang because the FSMonitor daemon will wait and wait even though the `.git/` directory might be gone already. Signed-off-by: Johannes Schindelin <[email protected]>
When the built-in FSMonitor receives an unexpected token, we do not actually need to wait for a cookie file. There simply is no use for waiting in this instance. It's not like the client will all of a sudden realize that it sent an incorrect token and somehow make up a correct token from thin air in a follow-up query. Signed-off-by: Johannes Schindelin <[email protected]>
When the built-in FSMonitor receives an invalid v2 token, we do not actually need to wait for a cookie file. There simply is no use for waiting in this instance. It's not like the client will all of a sudden realize that it sent an incorrect token and somehow make up a correct token from thin air in a follow-up query. Signed-off-by: Johannes Schindelin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Checks out as the same commits from microsoft/git
.
The built-in file system watcher could hang in some scenarios. [This was fixed](git-for-windows/git#3263). Signed-off-by: Johannes Schindelin <[email protected]>
when i start my "bash" it show cd: too many arguments |
how to solve this |
@jeffhostetler sounds like maybe an issue with spaces in arguments. While I don't think that is from this PR, perhaps you know where to look for such a possibility? |
@kashi1729 while it would be better to open a new ticket rather than tacking on a question to an unrelated Pull Request, I will give you this pointer: you can trace the startup commands as described here. |
Are you getting the error opening a new terminal window or when running a command within it? If it is when open a terminal window, please look and see if you have a bad command line or something in your startup ~/.profile or ~/.bashrc that might be causing the error. |
no there is no bad command line and ~/.bashrc not working |
While Git's test suite did not report any regressions, I had to hunt for a few days to figure out why Scalar's Functional Tests were still unhappy with the new FSMonitor patch series.
This PR is the culmination of those efforts, and a sibling to microsoft#371 (which not only integrates these patches, but makes sure that Scalar's Functional Tests are run in
microsoft/git
as part of the CI).