Re: [patch] - GIT - version control for LyX documents not detected
> Am 19.01.2016 um 08:09 schrieb Stephan Witt : > > Am 19.01.2016 um 07:56 schrieb Peter Kümmel : >> >> Am 18. Januar 2016 23:25:03 MEZ, schrieb Pavel Sanda : >> Stephan Witt wrote: >> The attached patch fixes the broken detection of GIT version control. >> It seems so that Qt is caching the file meta data and fools the test >> of file emptiness. Perhaps this has changed with Qt5 and didn???t happen >> with Qt4??? >> >> Bad news, if your analysis is correct, we might encounter the same problem >> in other parts of the code as well. I don't have easy access to qt5 to test. >> >> Quick check says we use is FileEmpty also in: >> Buffer.cpp: enable = (d->preview_file_).exists() && >> !(d->preview_file_).isFileEmpty(); >> LaTeX.cpp:rerun = idxfile.exists() && idxfile.isFileEmpty(); >> LaTeX.cpp:if (head.haschanged(nlofile) || (nlofile.exists() && >> nlofile.isFileEmpty())) >> >> Pavel >> >> I did not follow this git stuff, but I assume current implementation tries >> to use system git calls via Qt classes, am I right? > > No. It is hand crafted stuff. The problem is the temporary file to collect > the output of system calls. > After creation it’s empty - of course. The system call changes this and the > next call to QFileInfo::size() returns 0 on my system. This is documented here: http://doc.qt.io/qt-5/qfileinfo.html I don’t know when this was changed and/or if this is/was platform dependent. Stephan > This I couldn’t debug further because of... I don’t know. In the past I’ve > been able to step into the Qt code. > After doing an upgrade of my OS and of the development system I have to get > it working again :( > > Stephan > >> I really could imagine this makes problems, because of Qt. >> >> Afaik QtCreator tries the same, is any code used from there in LyX? >> >> Had someone the idea to use libgit2 instead of the system git? Was it >> evaluated? > > No, this would add another external library to the list of dependencies…
Re: [patch] - GIT - version control for LyX documents not detected
Am 19.01.2016 um 07:56 schrieb Peter Kümmel : > > Am 18. Januar 2016 23:25:03 MEZ, schrieb Pavel Sanda : > Stephan Witt wrote: > The attached patch fixes the broken detection of GIT version control. > It seems so that Qt is caching the file meta data and fools the test > of file emptiness. Perhaps this has changed with Qt5 and didn???t happen > with Qt4??? > > Bad news, if your analysis is correct, we might encounter the same problem > in other parts of the code as well. I don't have easy access to qt5 to test. > > Quick check says we use is FileEmpty also in: > Buffer.cpp: enable = (d->preview_file_).exists() && > !(d->preview_file_).isFileEmpty(); > LaTeX.cpp:rerun = idxfile.exists() && idxfile.isFileEmpty(); > LaTeX.cpp:if (head.haschanged(nlofile) || (nlofile.exists() && > nlofile.isFileEmpty())) > > Pavel > > I did not follow this git stuff, but I assume current implementation tries to > use system git calls via Qt classes, am I right? No. It is hand crafted stuff. The problem is the temporary file to collect the output of system calls. After creation it’s empty - of course. The system call changes this and the next call to QFileInfo::size() returns 0 on my system. This I couldn’t debug further because of... I don’t know. In the past I’ve been able to step into the Qt code. After doing an upgrade of my OS and of the development system I have to get it working again :( Stephan > I really could imagine this makes problems, because of Qt. > > Afaik QtCreator tries the same, is any code used from there in LyX? > > Had someone the idea to use libgit2 instead of the system git? Was it > evaluated? No, this would add another external library to the list of dependencies…
Re: [patch] - GIT - version control for LyX documents not detected
Am 18. Januar 2016 23:25:03 MEZ, schrieb Pavel Sanda : >Stephan Witt wrote: >> The attached patch fixes the broken detection of GIT version control. >> It seems so that Qt is caching the file meta data and fools the test >> of file emptiness. Perhaps this has changed with Qt5 and didn???t >happen >> with Qt4??? > >Bad news, if your analysis is correct, we might encounter the same >problem >in other parts of the code as well. I don't have easy access to qt5 to >test. > >Quick check says we use is FileEmpty also in: >Buffer.cpp: enable = (d->preview_file_).exists() && >!(d->preview_file_).isFileEmpty(); >LaTeX.cpp:rerun = idxfile.exists() && idxfile.isFileEmpty(); >LaTeX.cpp:if (head.haschanged(nlofile) || (nlofile.exists() && >nlofile.isFileEmpty())) > >Pavel I did not follow this git stuff, but I assume current implementation tries to use system git calls via Qt classes, am I right? I really could imagine this makes problems, because of Qt. Afaik QtCreator tries the same, is any code used from there in LyX? Had someone the idea to use libgit2 instead of the system git? Was it evaluated? I assume then there we have much more control over the git files. Peter
Re: [patch] - GIT - version control for LyX documents not detected
Richard Heck wrote: > And, as I think is said there, we've seen issues with this, when using > filesystems with > significant latency, e.g., over NFS. It'd be worth adding a comment to > isFileEmpty() so > this question doesn't keep arising. Committed. P
Re: [patch] - GIT - version control for LyX documents not detected
Stephan Witt wrote: > The attached patch fixes the broken detection of GIT version control. > It seems so that Qt is caching the file meta data and fools the test > of file emptiness. Perhaps this has changed with Qt5 and didn???t happen > with Qt4??? Bad news, if your analysis is correct, we might encounter the same problem in other parts of the code as well. I don't have easy access to qt5 to test. Quick check says we use is FileEmpty also in: Buffer.cpp: enable = (d->preview_file_).exists() && !(d->preview_file_).isFileEmpty(); LaTeX.cpp:rerun = idxfile.exists() && idxfile.isFileEmpty(); LaTeX.cpp:if (head.haschanged(nlofile) || (nlofile.exists() && nlofile.isFileEmpty())) Pavel
Re: [patch] - GIT - version control for LyX documents not detected
On 01/16/2016 05:27 PM, Scott Kostyshak wrote: On Sat, Jan 16, 2016 at 11:13:23PM +0100, Stephan Witt wrote: Perhaps a better fix is to disable QFileInfo caching or to call fi.refresh() every time in FileName::isFileEmpty()? Is anybody aware of other places having this kind of problem? Yes, I fixed one here: 43ca05ea. See the discussion in that commit message. In particular, Pavel suggests that emptying the cache often would cause a significant performance decrease for some use cases: https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg188628.html And, as I think is said there, we've seen issues with this, when using filesystems with significant latency, e.g., over NFS. It'd be worth adding a comment to isFileEmpty() so this question doesn't keep arising. Richard
Re: [patch] - GIT - version control for LyX documents not detected
On Sat, Jan 16, 2016 at 11:13:23PM +0100, Stephan Witt wrote: > The attached patch fixes the broken detection of GIT version control. > It seems so that Qt is caching the file meta data and fools the test > of file emptiness. Perhaps this has changed with Qt5 and didn’t happen > with Qt4… A proper solution for VCS would move doVCCommandWithOutput > to VCS::doVCCommandWithOutput and use it where a temporary file for > output is used. I’ll do this if such a change is allowed. I'm fine with the change in your patch. I am familiar with this type of fix and the change is contained in VCBackend. I have not tested but if it fixes it for you then go ahead. +1 > Perhaps a better fix is to disable QFileInfo caching or to call > fi.refresh() every time in FileName::isFileEmpty()? > > Is anybody aware of other places having this kind of problem? Yes, I fixed one here: 43ca05ea. See the discussion in that commit message. In particular, Pavel suggests that emptying the cache often would cause a significant performance decrease for some use cases: https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg188628.html Scott signature.asc Description: PGP signature
[patch] - GIT - version control for LyX documents not detected
The attached patch fixes the broken detection of GIT version control. It seems so that Qt is caching the file meta data and fools the test of file emptiness. Perhaps this has changed with Qt5 and didn’t happen with Qt4… A proper solution for VCS would move doVCCommandWithOutput to VCS::doVCCommandWithOutput and use it where a temporary file for output is used. I’ll do this if such a change is allowed. Perhaps a better fix is to disable QFileInfo caching or to call fi.refresh() every time in FileName::isFileEmpty()? Is anybody aware of other places having this kind of problem? Stephan VCBackend-git-enabled.patch Description: Binary data
