[prev in list] [next in list] [prev in thread] [next in thread]
List: kwrite-devel
Subject: Re: Broken Git repos scanning in project plugin
From: Valentin Rouet <v.rouet () gmail ! com>
Date: 2015-10-31 17:29:14
Message-ID: CACAhqFWLObyVu1nFACtS0JbGsjv+vU4nzrwA8wk6E3QmsfHvKA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/related)]
[Attachment #4 (multipart/alternative)]
Here is a patch that fixes the bug and adds some comments to the calls of
the libGit2 functions. It should still work for people who didn't have the
problem.
Can someone try it out and review the code to include it in the repo?
On Sat, Oct 31, 2015 at 6:28 PM, Valentin Rouet <v.rouet@gmail.com> wrote:
> OK, I've been investigating the source code, and have narrowed down the
> place where it happens. It does seem to come from some change in Qt.
>
> The problem is in file addons/project/kateprojectworker.cpp at line 358:
>
>> if (relpathUtf8.isEmpty() ) { // git_object_lookup_bypath is not able to
>> resolv "." as path
>>
> It seems this line relies on the fact that Qdir::relativeFilePath() would
> return an empty string when called with the current working dir as
> argument, but what it actually returns is the string "."
>
> When replacing that test with the following, the content loads:
>
>> if (relpathUtf8.isEmpty() || relpathUtf8 == ".") {
>
> But it's not a full solution, because then what you get is this:
>
> As you can see, there is a "." folder, instead of the files at the
> project's root being displayed directly. This makes me think that this
> change of Qt functions from returning an empty string to returning "." is
> affecting other places of the code. I also wonder if it really is
> Qt-related or if it has to do with an underlying OS-dependent mechanism...
> Anyways I haven't been able to find any reference to such change in Qt's
> changelogs.
>
> On Sat, Oct 31, 2015 at 6:03 PM, Valentin Rouet <v.rouet@gmail.com> wrote:
>
>> OK, I've been investigating the source code, and have narrowed down the
>> place where it happens. It does seem to come from some change in Qt.
>>
>> The problem is in file addons/project/kateprojectworker.cpp at line 358:
>>
>>> if (relpathUtf8.isEmpty() ) { // git_object_lookup_bypath is not able to
>>> resolv "." as path
>>>
>> It seems this line relies on the fact that Qdir::relativeFilePath() would
>> return an empty string when called with the current working dir as
>> argument, but what it actually returns is the string "."
>>
>> When replacing that test with the following, the content loads:
>>
>>> if (relpathUtf8.isEmpty() || relpathUtf8 == ".") {
>>
>> But it's not a full solution, because then what you get is this:
>>
>> As you can see, there is a "." folder, instead of the files at the
>> project's root being displayed directly. This makes me think that this
>> change of Qt functions from returning an empty string to returning "." is
>> affecting other places of the code. I also wonder if it really is
>> Qt-related or if it has to do with an underlying OS-dependent mechanism...
>> Anyways I haven't been able to find any reference to such change in Qt's
>> changelogs.
>>
>> On Sat, Oct 31, 2015 at 3:13 PM, Michal Humpula <
>> michal.humpula@hudrydum.cz> wrote:
>>
>>> Are you capable of downgrading to Qt5.4, just for the fun of testing?:)
>>>
>>> On Saturday 31 of October 2015 15:11:12 Valentin Rouet wrote:
>>> > Hi,
>>> >
>>> > I'm not completely sure after which update this started to happen, but
>>> from
>>> > what I see in my packages cache, I was on Qt 5.5.0 since august and it
>>> was
>>> > working.
>>> >
>>> > 2 updates I see are:
>>> > - Kate 15.08.1 to 15.08.2 on october 13th
>>> > - Qt 5.5.0 to 5.5.1 on october 16th
>>> >
>>> >
>>> > The problem must have been introduced by one of these 2 updates, but I
>>> > don't know which.
>>> >
>>> > On Sat, Oct 31, 2015 at 3:00 PM, Michal Humpula <
>>> michal.humpula@hudrydum.cz>
>>> > wrote:
>>> > > Hi Valentin,
>>> > >
>>> > > can you pinpoint the time when it started to happen? Could it be
>>> after
>>> > > upgrade
>>> > > to Qt5.5?:) I have a very similar issue to yours, which is
>>> reproducible on
>>> > > Qt5.5 but not on Qt5.4:)
>>> > >
>>> > > Cheers
>>> > > Michal
>>> > >
>>> > > On Saturday 31 of October 2015 13:51:07 Valentin Rouet wrote:
>>> > > > Dear Kate developers,
>>> > > >
>>> > > > First of all, before talking about problems, let me thank you for
>>> the
>>> > >
>>> > > great
>>> > >
>>> > > > pieces of software that Kate/Kwrite are. I've been using them for
>>> about
>>> > >
>>> > > 10
>>> > >
>>> > > > years, and they've always been fulfilling my text-editing and
>>> > > > programming
>>> > > > needs.
>>> > > >
>>> > > > I have already reported the bug I'm having on the bugtracker (
>>> > > > https://bugs.kde.org/show_bug.cgi?id=354494), but didn't get any
>>> > >
>>> > > feedback
>>> > >
>>> > > > from any dev, so I'd just like to be sure you're aware of its
>>> existence,
>>> > >
>>> > > as
>>> > >
>>> > > > it seems to me that one major feature is involved. Someone did
>>> confirm
>>> > >
>>> > > thay
>>> > >
>>> > > > had the same problem though, on a different distro but with the
>>> same
>>> > > > package version.
>>> > > >
>>> > > > The Git repo scanning feature is great and absolutely necessary.
>>> I've
>>> > >
>>> > > been
>>> > >
>>> > > > working without it for a few days, using the files browser plugin
>>> > >
>>> > > instead,
>>> > >
>>> > > > but it's really not filling the void...
>>> > > >
>>> > > > If you need any additional information to track down the bug,
>>> please let
>>> > >
>>> > > me
>>> > >
>>> > > > know.
>>> > > >
>>> > > > Thanks in advance,
>>> > > >
>>> > > > Valentin Rouet
>>>
>>>
>>
>
[Attachment #7 (text/html)]
<div dir="ltr">Here is a patch that fixes the bug and adds some comments to the
calls of the libGit2 functions. It should still work for people who
didn't have the problem.<br>Can someone try it out and review the code to include \
it in the repo?</div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, \
Oct 31, 2015 at 6:28 PM, Valentin Rouet <span dir="ltr"><<a \
href="mailto:v.rouet@gmail.com" target="_blank">v.rouet@gmail.com</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex"><span class="im HOEnZb"><div \
dir="ltr"><div><div><div>OK, I've been investigating the source code, and have \
narrowed down the place where it happens. It does seem to come from some change in
Qt.<br><br></div>The problem is in file addons/project/kateprojectworker.cpp at line \
358:<br><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex" class="gmail_quote">if (relpathUtf8.isEmpty() ) { \
// git_object_lookup_bypath is not able to resolv "." as \
path<br></blockquote>It seems this line relies on the fact that \
Qdir::relativeFilePath() would return an empty string when called with the current \
working dir as argument, but what it actually returns is the string \
"."<br><br></div>When replacing that test with the following, the content \
loads:<br><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex" class="gmail_quote">if (relpathUtf8.isEmpty() || \
relpathUtf8 == ".") {</blockquote><div>But it's not a full solution, \
because then what you get is this:<br><img src="cid:ii_igfc3wyk0_150bed7d2b8e127e" \
height="156" width="273"><br></div> As you can see, there is a "." \
folder, instead of the files at the project's root being displayed directly. \
This makes me think that this change of Qt functions from returning an empty string \
to returning "." is affecting other places of the code. I also wonder if \
it really is Qt-related or if it has to do with an underlying OS-dependent
mechanism... Anyways I haven't been able to find any reference to such
change in Qt's changelogs. <br></div></div></span><div class="HOEnZb"><div \
class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Oct 31, 2015 \
at 6:03 PM, Valentin Rouet <span dir="ltr"><<a href="mailto:v.rouet@gmail.com" \
target="_blank">v.rouet@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr"><div><div>OK, I've been investigating the \
source code, and have narrowed down the place where it happens. It does seem to come \
from some change in Qt.<br><br></div>The problem is in file \
addons/project/kateprojectworker.cpp at line 358:<br><blockquote style="margin:0px \
0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" \
class="gmail_quote">if (relpathUtf8.isEmpty() ) { // git_object_lookup_bypath is not \
able to resolv "." as path<br></blockquote>It seems this line relies on \
the fact that Qdir::relativeFilePath() would return an empty string when called with \
the current working dir as argument, but what it actually returns is the string \
"."<br><br></div>When replacing that test with the following, the content \
loads:<br><blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex" class="gmail_quote">if (relpathUtf8.isEmpty() || \
relpathUtf8 == ".") {</blockquote><div>But it's not a full solution, \
because then what you get is this:<br><img src="cid:ii_igfc3wyk0_150bed7d2b8e127e" \
height="156" width="273"><br></div><div>As you can see, there is a "." \
folder, instead of the files at the project's root being displayed directly. This \
makes me think that this change of Qt functions from returning an empty string to \
returning "." is affecting other places of the code. I also wonder if it \
really is Qt-related or if it has to do with an underlying OS-dependent mechanism... \
Anyways I haven't been able to find any reference to such change in Qt's \
changelogs. <br></div></div><div><div><div class="gmail_extra"><br><div \
class="gmail_quote">On Sat, Oct 31, 2015 at 3:13 PM, Michal Humpula <span \
dir="ltr"><<a href="mailto:michal.humpula@hudrydum.cz" \
target="_blank">michal.humpula@hudrydum.cz</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Are you capable of downgrading to Qt5.4, just for the fun of \
testing?:)<br> <div><div><br>
On Saturday 31 of October 2015 15:11:12 Valentin Rouet wrote:<br>
> Hi,<br>
><br>
> I'm not completely sure after which update this started to happen, but \
from<br> > what I see in my packages cache, I was on Qt 5.5.0 since august and it \
was<br> > working.<br>
><br>
> 2 updates I see are:<br>
> - Kate 15.08.1 to 15.08.2 on october 13th<br>
> - Qt 5.5.0 to 5.5.1 on october 16th<br>
><br>
><br>
> The problem must have been introduced by one of these 2 updates, but I<br>
> don't know which.<br>
><br>
> On Sat, Oct 31, 2015 at 3:00 PM, Michal Humpula <<a \
href="mailto:michal.humpula@hudrydum.cz" \
target="_blank">michal.humpula@hudrydum.cz</a>><br> > wrote:<br>
> > Hi Valentin,<br>
> ><br>
> > can you pinpoint the time when it started to happen? Could it be after<br>
> > upgrade<br>
> > to Qt5.5?:) I have a very similar issue to yours, which is reproducible \
on<br> > > Qt5.5 but not on Qt5.4:)<br>
> ><br>
> > Cheers<br>
> > Michal<br>
> ><br>
> > On Saturday 31 of October 2015 13:51:07 Valentin Rouet wrote:<br>
> > > Dear Kate developers,<br>
> > ><br>
> > > First of all, before talking about problems, let me thank you for \
the<br> > ><br>
> > great<br>
> ><br>
> > > pieces of software that Kate/Kwrite are. I've been using them for \
about<br> > ><br>
> > 10<br>
> ><br>
> > > years, and they've always been fulfilling my text-editing and<br>
> > > programming<br>
> > > needs.<br>
> > ><br>
> > > I have already reported the bug I'm having on the bugtracker (<br>
> > > <a href="https://bugs.kde.org/show_bug.cgi?id=354494" rel="noreferrer" \
target="_blank">https://bugs.kde.org/show_bug.cgi?id=354494</a>), but didn't get \
any<br> > ><br>
> > feedback<br>
> ><br>
> > > from any dev, so I'd just like to be sure you're aware of its \
existence,<br> > ><br>
> > as<br>
> ><br>
> > > it seems to me that one major feature is involved. Someone did \
confirm<br> > ><br>
> > thay<br>
> ><br>
> > > had the same problem though, on a different distro but with the \
same<br> > > > package version.<br>
> > ><br>
> > > The Git repo scanning feature is great and absolutely necessary. \
I've<br> > ><br>
> > been<br>
> ><br>
> > > working without it for a few days, using the files browser plugin<br>
> ><br>
> > instead,<br>
> ><br>
> > > but it's really not filling the void...<br>
> > ><br>
> > > If you need any additional information to track down the bug, please \
let<br> > ><br>
> > me<br>
> ><br>
> > > know.<br>
> > ><br>
> > > Thanks in advance,<br>
> > ><br>
> > > Valentin Rouet<br>
<br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
["kate1.png" (image/png)]
["kateproject_git.patch" (text/x-patch)]
diff --git a/addons/project/kateprojectworker.cpp \
b/addons/project/kateprojectworker.cpp index 4a16513..b053156 100644
--- a/addons/project/kateprojectworker.cpp
+++ b/addons/project/kateprojectworker.cpp
@@ -203,7 +203,15 @@ void KateProjectWorker::loadFilesEntry(QStandardItem *parent, \
const QVariantMap
*/
KateProjectItem *fileItem = new KateProjectItem(KateProjectItem::File, \
fileInfo.fileName()); fileItem->setData(filePath, Qt::ToolTipRole);
- item2ParentPath.append(QPair<QStandardItem *, QStandardItem *>(fileItem, \
directoryParent(dir2Item, dir.relativeFilePath(fileInfo.absolutePath())))); +
+ // get the directory's relative path to the base directory
+ QString dirRelPath = dir.relativeFilePath(fileInfo.absolutePath());
+ // if the relative path is ".", clean it up
+ if (dirRelPath == QStringLiteral(".")) {
+ dirRelPath = QString();
+ }
+
+ item2ParentPath.append(QPair<QStandardItem *, QStandardItem *>(fileItem, \
directoryParent(dir2Item, dirRelPath))); fileItem->setData(filePath, Qt::UserRole);
(*file2Item)[filePath] = fileItem;
}
@@ -324,12 +332,17 @@ QStringList KateProjectWorker::filesFromGit(const QDir &dir, \
bool recursive) git_repository *repo = nullptr;
git_object *root_tree = nullptr, *tree = nullptr;
+ // check if the repo can be opened.
+ // git_repository_open_ext() will return 0 if everything is OK;
+ // if not, return an empty files list
const QByteArray repoPathUtf8 = dir.path().toUtf8();
if (git_repository_open_ext(&repo, repoPathUtf8.constData(), 0, NULL)) {
git_libgit2_shutdown();
return files;
}
+ // get the working directory of the repo
+ // if none was found, return an empty files list
const char *working_dir = nullptr;
if ((working_dir = git_repository_workdir(repo)) == nullptr) {
git_repository_free(repo);
@@ -347,7 +360,7 @@ QStringList KateProjectWorker::filesFromGit(const QDir &dir, bool \
recursive) workdir.setPath(QString::fromUtf8(working_dir));
const QByteArray relpathUtf8 = workdir.relativeFilePath(dir.path()).toUtf8();
- if (relpathUtf8.isEmpty()) { // git_object_lookup_bypath is not able to resolv \
"." as path + if (relpathUtf8.isEmpty() || relpathUtf8 == ".") { // \
git_object_lookup_bypath is not able to resolv "." as path tree = root_tree;
} else {
if (git_object_lookup_bypath(&tree, root_tree, relpathUtf8.constData(), \
GIT_OBJ_TREE)) {
[Attachment #10 (text/plain)]
_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic