Bug 379219 - kdevelop interferes with the correct operation of git
Summary: kdevelop interferes with the correct operation of git
Status: RESOLVED FIXED
Alias: None
Product: kdevelop
Classification: Applications
Component: VCS: Git (show other bugs)
Version: 5.5.0
Platform: Other Linux
: NOR grave
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords:
: 382850 404515 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-04-25 19:23 UTC by mrnuke
Modified: 2023-03-29 13:46 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
stdio output of Kdevelop while problem manifests (1.35 KB, text/plain)
2017-05-13 03:25 UTC, mrnuke
Details
strace, attached to already running kdevelop, then the rebase boogieman strikes (586.14 KB, application/gzip)
2017-05-16 15:41 UTC, mrnuke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description mrnuke 2017-04-25 19:23:46 UTC
KDevelop consistently does things behind the hood that interfere with the operation of git commands run from a terminal.most notably, during git rebase, git will often stop with the following error:

fatal: Unable to create '/home/mrnuke/src/linux/.git/index.lock': File exists.

Another git process seems to be running in this repository, e.g.
an editor opened by 'git commit'. Please make sure all processes
are terminated then try again. If it still fails, a git process
may have crashed in this repository earlier:
remove the file manually to continue.
Could not apply ...

This is extremely annoying, especially on rebases with a dozen or more patched, where git is interrupted repeatedly.

To test:
1. Open provect in kdevelop
2. Leave Kdevelop open
3. From a separate terminal, git rebase (the more patches in the rebase chain, the better)
4. Save rebase-todo, continue

Expected results:
(if there is no conflict) git rebase continues without issue.

Actual results:
git is constantly interrupted.

I am marking this as "Severity: grave" as it presents a great health risk to git users. I have seen reports of several strokes caused by combining kdevelop with git, with one reported user being rushed to the emergency room. The gas resulting from mixing these two chemicals is known to the state of california to cause cancer, dementia, and other reproductive harm.
Comment 1 Wei-Cheng Pan 2017-04-28 06:40:53 UTC
Maybe we need to rewrite the git plugin to use libgit2, instead of command line.
Comment 2 Kevin Funk 2017-04-28 07:40:40 UTC
Note: I've never seen this. I use the command-line Git regularly, to do branch switches / rebases / etc. pp..

Could you check what KDevelop is doing while you are performing the Git rebase from the command line? I.e. by checking the output KDevelop gives in the terminal?
Comment 3 Sven Brauch 2017-04-28 15:48:15 UTC
(In reply to Wei-Cheng Pan from comment #1)
> Maybe we need to rewrite the git plugin to use libgit2, instead of command
> line.

I don't think that would even help in this situation. The problem is that KDevelop does $thing (e.g. git status, I guess), which makes total sense, and that interrupts git rebase because KDevelop can't know that is running.

Maybe the "proper"™ solution to this is making git rebase not release the lock while it is running? I don't really see a different option.
Comment 4 Sven Brauch 2017-04-28 15:49:47 UTC
Also I'm not super excited about libgit2, it's another incredibly complex piece of software running in-process and potentially crashing (ask the kate people). Out-of-process looks messy but tends to be quite robust ...
Comment 5 Kevin Funk 2017-04-28 16:30:25 UTC
+1, btw,

I'm not looking forward to another potentially crashy library incorporated into KDevelop either. We should find a different solution (but first we need more information).
Comment 6 mrnuke 2017-05-13 03:25:09 UTC
Created attachment 105500 [details]
stdio output of Kdevelop while problem manifests

I finally remembered to start Kdevelop from the terminal, so I got a stdio trace. It doesn't seem particularly useful to me.

The frequency of the problem increases exponentially with the size of the codebase (i.e. easy to see on linux tree), and with the number of files that are open in Kdevelop. The more open files that the debase modifies, the higher the insanity level.

DISCLAIMER: No git users were harmed in the making of this stdio trace.
Comment 7 Milian Wolff 2017-05-16 09:38:20 UTC
you could try to strace kdevelop to see the git commands it executes and files it creates/locks, something like the following could do the trick:

strace -e open,execve,flock -f -s 1000 kdevelop -s test |& gzip2 > strace.log.gz

then upload the strace here, it will probably be large and require us to look at in-depth

cheers
Comment 8 mrnuke 2017-05-16 15:41:12 UTC
Created attachment 105593 [details]
strace, attached to already running kdevelop, then the rebase boogieman strikes

I think it's much easier for someone familiar with the kdevelop code to add some useful debug info, start kdevelop and do a rebase on the linux tree.

That being said, here it goes
Comment 9 Wei-Cheng Pan 2017-05-25 03:25:44 UTC
I think it's because `git status --porcelain`, it will do something like `open("/repo/.git/index.lock", O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0666)`, according to strace.
Comment 10 Wei-Cheng Pan 2017-05-26 08:05:23 UTC
BTW, this is what I get in the log:

kdevplatform.vcs: Execute dvcs command: "git status --porcelain -- /***/firefox/widget/windows/JumpListBuilder.cpp"
kdevplatform.vcs: Execute dvcs command: "git ls-files -c -- /***/firefox/widget/windows/JumpListBuilder.cpp"
kdevplatform.vcs: Execute dvcs command: "git ls-files -- JumpListBuilder.cpp"
kdevplatform.vcs: Execute dvcs command: "git status --porcelain -- /***/firefox/widget/windows/JumpListBuilder.cpp"
kdevplatform.vcs: Execute dvcs command: "git ls-files -c -- /***/firefox/widget/windows/JumpListBuilder.cpp"
kdevplatform.vcs: Execute dvcs command: "git symbolic-ref -q --short HEAD"
kdevplatform.vcs: Execute dvcs command: "git symbolic-ref -q --short HEAD"
kdevplatform.vcs: Execute dvcs command: "git status --porcelain -- /***/firefox"
kdevplatform.vcs: Execute dvcs command: "git ls-files -c -- /***/firefox"
kdevplatform.vcs: Execute dvcs command: "git symbolic-ref -q --short HEAD"
kdevplatform.vcs: Execute dvcs command: "git symbolic-ref -q --short HEAD"
kdevplatform.vcs: Execute dvcs command: "git status --porcelain -- /***/firefox"
kdevplatform.vcs: Execute dvcs command: "git ls-files -c -- /***/firefox"
kdevplatform.vcs: Execute dvcs command: "git symbolic-ref -q --short HEAD"
kdevplatform.vcs: Execute dvcs command: "git symbolic-ref -q --short HEAD"
kdevplatform.vcs: Execute dvcs command: "git status --porcelain -- /***/firefox"
kdevplatform.vcs: Execute dvcs command: "git ls-files -c -- /***/firefox"
kdevplatform.vcs: Execute dvcs command: "git symbolic-ref -q --short HEAD"
kdevplatform.vcs: Execute dvcs command: "git symbolic-ref -q --short HEAD"
kdevplatform.vcs: Execute dvcs command: "git status --porcelain -- /***/firefox"
kdevplatform.vcs: Execute dvcs command: "git ls-files -c -- /***/firefox"
Comment 11 Sven Brauch 2017-05-26 11:11:56 UTC
I still think this needs to be fixed in git, if you run a rebase and then "git status" in a different terminal, you will have the same issue ...
Comment 12 mrnuke 2017-05-29 18:50:17 UTC
NAK. That's not how git is designed to work. I can take a Ferrarri and drive it on a dirt road. It coming back all dinged and scratched is not something Ferrari has to fix.

The interaction issue here is that operations on the HEAD are locking and non-atomic. If someone keeps bugging me when I use a torque wrench, I will get the wrong torque. It doesn't mean the wrench is out of calibration. The same applies to git.

When the problem is tightening a nut, and the tool is a torque wrench. In our case, the problem is source control, and the to tool is git. Generally, when someone switches to a terminal to do git stuff, kdevelop loses the focus. now I'm not saying go ahead and implement a silly "suspend git commands when we lose focus" heuristic, but something does have to be done.

Note that this issue did not happen with Kdevelop 4.x (or somewhere around the late 4.x releases), so this is most likely a regression that was introduced within the past year.

As far as a user is concerned, recalibrating the torque wrench won't solve the problem. What will is firing the employee who bugs me. I don't think that telling git users to stop using Kdevelop is a sane approach, as I am pretty sure the git guys will rightfully say "this is not our problem".
Comment 13 Sven Brauch 2017-05-29 19:21:24 UTC
You created half a page of analogies but none of them has an actual argument which  explains why it makes sense for git to release the lock during rebasing and then fail when something else acquires it.
Comment 14 mrnuke 2017-05-29 19:24:26 UTC
Git needs to release the lock in order for operations such as conflict resolution to not deadlock, or rebase with edit. Or a number of other things. I'm sorry for assuming this was obvious.
Comment 15 Sven Brauch 2017-05-29 19:27:22 UTC
It wasn't, but that makes sense. It could still wait for a bit for the lock to become available though.
Comment 16 mrnuke 2017-05-29 19:45:46 UTC
In that case, you'd have to determine if the process holding the lock is still active in order to avoid an infinite loop. Or timeout after a while, increasing complexity for a very niche use case.

Another approach is to introduce producer-consumer relationships between commands, where 'git status' is a consumer, but that introduces a complex hierarchy between commands. I'm not involved in git development, but if I were maintaining git, I would not consider running 'git status' in parallel with other operations to be a valid reason to add this level complexity.
Comment 17 Sven Brauch 2017-05-29 20:10:31 UTC
Well programming is complicated sometimes ...

"don't run any git operations unless the window has focus" certainly isn't an acceptable solution either way.
Comment 18 Milian Wolff 2017-05-30 13:54:04 UTC
@mrnuke: how long does `git status --porcelain -- path/to/linux` take on your machine? On mine it finishes within ~200ms, according to perf stat:

 Performance counter stats for 'git status --porcelain -- .' (5 runs):

        268.574322      task-clock (msec)         #    1.822 CPUs utilized            ( +- 20.51% )
               101      context-switches          #    0.375 K/sec                    ( +- 37.75% )
                17      cpu-migrations            #    0.063 K/sec                    ( +- 22.93% )
             3,723      page-faults               #    0.014 M/sec                    ( +-  0.07% )
       985,826,062      cycles                    #    3.671 GHz                      ( +- 20.79% )
       724,647,803      instructions              #    0.74  insn per cycle           ( +- 13.41% )
       136,599,722      branches                  #  508.611 M/sec                    ( +- 14.38% )
         1,823,707      branch-misses             #    1.34% of all branches          ( +- 13.35% )

       0.147384364 seconds time elapsed                                          ( +- 18.04% )

What kind of disk and filesystem are you using? Maybe not yet upgraded to SSD? Note that I actually spent quite some time working on perf from within KDevelop recently, and I never saw this issue crop up. I do use git more or less exclusively from the command line, kdevelop only tracks the edit status every now and then - but so far I never noticed this issue at all.
Comment 19 Wei-Cheng Pan 2017-06-02 08:13:20 UTC
For what it's worth, `git status --porcelain` usually takes 0.3s for a clean work copy on my machine.
Comment 20 Sven Brauch 2017-07-28 10:53:59 UTC
*** Bug 382850 has been marked as a duplicate of this bug. ***
Comment 21 Sven Brauch 2017-07-28 10:54:48 UTC
I'll set this to open -- it's not really waiting for info, and it's clearly a problem. I just don't see how we can solve it here downstream.
Comment 22 Daniel Santos 2017-07-28 14:07:13 UTC
Sorry about my duplicate bug.

This really is a serious problem, you just *can't* interfere with source control.  I would even suggest working with the git team to see if git can add (or already has) a good mechanism to help.  At this time, the very LEAST that KDevelop can do is to check if there's a .git/rebase-merge directory in existence and don't run any git commands when it exists unless it's interactive and the file time is at least 10 seconds old.
Comment 23 Sven Brauch 2017-07-28 14:09:32 UTC
I don't even think this issue is specific to KDevelop. For example I have a shell which does "git status" and shows a prompt depending on that -- lots of people have that. If you press Enter while a rebase is in progress in another shell, you will have the same problem ...

I still think this should, and needs to be fixed on the git side. But yes, maybe we should add a workaround like you suggested in the meantime ...
Comment 24 mrnuke 2017-08-04 17:18:59 UTC
I think the comment that this issue isn't specific to KDevelop is misleading. Yes, it's possible to type 'git status' while a rebase is in progress, the same way it's possible to type 'rm -rf ~'. If KDevelop exec'd 'rm -rf ~', you'd treat that as a bug and fix it, correct?
Comment 25 Sven Brauch 2017-08-04 17:34:15 UTC
I don't think that is a good comparison.
Comment 26 Daniel Santos 2017-08-05 00:07:46 UTC
(In reply to Sven Brauch from comment #23)

I understand your position that git may want to look at this.  Has there been a bug report filed with them yet?  I also think that I understand git's design decision to implement rebases as individual atomic transactions.

None the less, KDevelop 4 did not have this problem, so something changed in KDevelop 5 that is leading to this.  I learned long ago that if I don't put my build directory in an "exclude" filter and I try to run make on my project that my machine comes to a stands still because KDevelop 4 demanded all available CPU to continually rescan the file system, update the tree, etc., especially when I ran nice -n20 make.

Further, when we're in the middle of a rebase, we don't even want to update the status in KDevelop for each commit, that will unnecessarily hog CPU cycles, and cause rebase operations to be significantly slowed down for no good reason -- this is especially true when source parsing is triggered!

A long time ago I wrote an algo for saved game preview functionality for a video game.  Producing the preview took some I/O and a fair amount of CPU.  When the user tried to arrow down through the list, there was a 400-1000-ish millisecond pause in between each movement of the cursor.  Even moving the saved game preview into another thread (where it belonged anyway) was still clunky because of the CPU and I/O.  So my algo kept track of the recent frequency of changes to the currently selected file and when that was high, it delayed the onset of the saved game preview generation so that the user could quickly scroll without having the UI bog down.  Then when they stopped moving the cursor through the saved game list, there was a reasonable 250-ish millisecond delay before the preview generation started.

Maybe KDevelop needs something similar, so that it has a base delay before updating due to (unfiltered) file system changes (something small like 100mS), and as the frequency of such changes increases over a short time is high, that delay is increased and if the delay has not expired when another change occurs, the counter is reset.  But KDevelop should still probably check if there is a rebase in progress.

As far as git is concerned, I wonder if compound operations like "rebase" and "am" should have some mechanism to indicate that such an operation is occurring so that other operations (like "status") can pass a flag that tells git block until such a compound operation has either completed or is suspended (in the case of a merge conflict, edit, squash, etc.).  Like "git status --dont-break-the-goddam-rebase-operations" :)
Comment 27 Daniel Santos 2017-09-22 08:44:43 UTC
There is some hope coming around the corner: https://public-inbox.org/git/20170921043214.pyhdsrpy4omy54rm@sigill.intra.peff.net

It's not yet committed, but this bug may end up either getting me committed or a trip to the cardiologist.  Any chance we can get the check for .git/rebase-merge directory work-around in the mean time.  I'm sorry I can't get to it myself right now.

Thanks
Comment 28 Sven Brauch 2017-09-30 10:27:27 UTC
Good to know somebody is considering this problem upstream ...
Yes, possibly we should provide a workaround on our side until then :/
So long, what you can do is pkill -STOP kdevelop and then pkill -CONT it when done with the rebase ... :D
Comment 29 mrnuke 2018-03-16 16:14:53 UTC
Can I have my checkbox to disable git integration completely? Please?

I'll do my own git management rather than have sensitive operations, like rebase fail. Thank you!
Comment 30 Sven Brauch 2018-03-16 16:34:50 UTC
You can turn off the git plugin in the options, no?
Comment 31 mrnuke 2018-03-16 17:39:26 UTC
I had totally missed that. Thank you for the suggestion!
Comment 32 Lassi Väätämöinen 2021-01-21 12:40:51 UTC
(In reply to Sven Brauch from comment #30)
> You can turn off the git plugin in the options, no?

This is a viable workaround, but still inconvenient, as for example using the 'Annotation'/git blame -functionality in KDevelop would be useful.

So disabling Git plugin loses all the functionality.
Comment 33 Milian Wolff 2021-05-03 12:55:53 UTC
Git commit 266dc9761bdee6dbe21fc2567caac3eb2e99a318 by Milian Wolff.
Committed on 03/05/2021 at 12:49.
Pushed by mwolff into branch 'master'.

Set GIT_OPTIONAL_LOCKS=0 for all git subprocesses

This should prevent kdevelop from interferring with other git
processes that are run manually by the user in e.g. a separate
terminal. Most notably, it should fix the pesky errors of this kind:

    fatal: Unable to create '....git/index.lock': File exists.

Support for this environment variable was added upstream in
version 2.15.0 from what I can see in [1].

[1]: https://github.com/git/git/commit/27344d6a6c8056664966e11acf674e5da6dd7ee3

M  +1    -1    plugins/git/gitclonejob.cpp
M  +2    -2    plugins/git/gitclonejob.h
M  +4    -0    plugins/git/gitjob.cpp
M  +24   -24   plugins/git/gitplugin.cpp

https://invent.kde.org/kdevelop/kdevelop/commit/266dc9761bdee6dbe21fc2567caac3eb2e99a318
Comment 34 Lassi Väätämöinen 2023-03-29 13:46:26 UTC
*** Bug 404515 has been marked as a duplicate of this bug. ***