GitLab

I have discovered this bug thanks to the following GCC warning:
warning: loop variable ‘filter’ of type ‘const KDevelop::Filter&’ binds to a temporary constructed from type ‘const KDevelop::SerializedFilter’ [-Wrange-loop-construct]
  162 |     for (const Filter& filter : filters) {
      |                        ^~~~~~
note: use non-reference type ‘const KDevelop::Filter’ to make the copy explicit or ‘const KDevelop::SerializedFilter&’ to prevent copying

Filter::Filter(const SerializedFilter& filter) prepends "*/" to an empty
pattern. So, aside from the inefficiency of constructing a temporary
Filter object, the bug prevented displaying the empty-pattern error.

The project filter validation was implemented in
59ed3308. The very next commit on the
next day - a15b683d - changed the type
of FilterModel::filters() from Filters to SerializedFilters and
introduced the bug. So the empty-pattern error must have been seen only
by its implementer during development testing.

Simply changing the type of the loop variable from Filter& to auto&
shows the empty-pattern error as soon as the user adds a new filter.
Prevent this annoyance by not rechecking filters when a row is inserted
(and when a row is moved, for good measure, to skip useless work).

In case of conflict in i18n, keep the version of the branch "ours"
To resolve a particular conflict, "git checkout --ours path/to/file.desktop"

Remove the unused indexedLocal container, we don't need it
and never really used it. I wrote this code with that bug 7 years
ago, funny how noone ever noticed!

0c3d13a8 removed this declaration
because in Qt 5 it is useful only for types stored in QVector,
QVarLengthArray and QList (and not QHash where FileModificationCache is
a mapped_type). However, QHash has been reimplemented in Qt 6 with some
optimizations enabled when both the key and the value type are
relocatable. Declaring FileModificationCache as Q_MOVABLE_TYPE makes no
difference now, but will benefit performance once KDevelop is ported to
Qt 6.

text/x-opencl-src was added to shared-mime-info 4 years ago in
https://gitlab.freedesktop.org/xdg/shared-mime-info/-/commit/4ab969c398671c519a10a601ad3eeb46f04361de

This MIME type is present in shared-mime-info version >= 1.9. Even
oldstable Debian "buster" has version 1.10. KDevelop's dependency
requirements don't allow building it on old distribution versions. So
there is no reason to keep the workaround.

This reverts commit 056932ba.

The test fails on my system and on KDevelop CI like this:
QDEBUG : TestFiles::testFiles(basicdeclsandcontexts.cpp) "identifier" FAILED: Declaration's identifier ("(unnamed struct at /home/jenkins/workspace/KDevelop/kdevelop/kf5-qt5 SUSEQt5.15/plugins/clang/tests/files/basicdeclsandcontexts.cpp:97:1)") doesn't match test data (""). (Declaration on line 97 in /home/jenkins/workspace/KDevelop/kdevelop/kf5-qt5 SUSEQt5.15/plugins/clang/tests/files/basicdeclsandcontexts.cpp)
QDEBUG : TestFiles::testFiles(basicdeclsandcontexts.cpp) "identifier" FAILED: Declaration's identifier ("(unnamed union at /home/jenkins/workspace/KDevelop/kdevelop/kf5-qt5 SUSEQt5.15/plugins/clang/tests/files/basicdeclsandcontexts.cpp:100:1)") doesn't match test data (""). (Declaration on line 100 in /home/jenkins/workspace/KDevelop/kdevelop/kf5-qt5 SUSEQt5.15/plugins/clang/tests/files/basicdeclsandcontexts.cpp)
FAIL!  : TestFiles::testFiles(basicdeclsandcontexts.cpp) 'validator.testsPassed()' returned FALSE. ()
   Loc: [/home/jenkins/workspace/KDevelop/kdevelop/kf5-qt5 SUSEQt5.15/plugins/clang/tests/test_files.cpp(116)]

The test fix is based on adding testDataAdjuster callback to
DeclarationValidator. It should be possible to reuse this new interface
to solve future test_files* failures.

I haven't verified that the test passes with older Clang versions. But
made sure that the test data is adjusted to the values from the parent
commit when Clang version is older than 13. So it should pass.

The absolute path is not trivially testable as the reverted commit's
message states. But the testing has been implemented in another branch
on top of an older commit. So the commit that simply expects failures in
newer clang versions has to be reverted to cleanly apply the proper fix.

This reverts commit c3d9ca6b.

The compiler warning reads:
warning: ‘void KDevelop::swap(KDevelop::IndexedString&, KDevelop::IndexedString&)’ has not been declared within ‘KDevelop’
  306 | void KDevelop::swap(IndexedString& a, IndexedString& b) noexcept
note: only here as a ‘friend’
  170 |     friend KDEVPLATFORMSERIALIZATION_EXPORT void swap(IndexedString&, IndexedString&) noexcept;

A relevant Stack Overflow question:
https://stackoverflow.com/questions/44749878/warning-defining-friend-operator-declared-inside-a-namespace

Unnamed structs/unions now get an identifier in the form

    (unnamed struct at /path/to/file:line:cursor)

The absolute path makes it impossible for us to easily test this,
so let's just expect failure for these in newer clang versions.

The .cpp file is `using namespace KDevelop` afer all.

Split the cache into one static with a mutex and the separate type
that one can only access while the mutex is held.

Instead, hoist the one function that requires it out into a helper
that operates on a locked mutex state and verify that is the case.

While at it, also remove the global static for the mutex and instead
use a function-local static that gets initialized on first use.

In case of conflict in i18n, keep the version of the branch "ours"
To resolve a particular conflict, "git checkout --ours path/to/file.desktop"

Several commits after the last release 21.12 changed KDevPlatform API
and ABI. For example, 32c8bbeb.
Therefore, KDEVELOP_SOVERSION and KDEV_PLUGIN_VERSION have to change.
Incrementing KDevelop minor version automatically increments these two
variables.

Default NumberOfJobs in makejob.cpp (1) and makebuilderconfig.kcfg (2)
is different. Kdevelop already tries to find the optimal number of threads
if NumberOfJobs field is not overridden. The current default makes
it impossible to override a number of jobs to 2 as NumberOfJobs will
switch to 1 due to makejob.cpp.

BUG: 448251
FIXED-IN: 5.8.220400

(cherry picked from commit 6bdb7840)

Look the SDK up first in the user installation then the system's.

Modern flatpak doesn't show ref unless asked for details. This is due
to changes in how the cli output has been reworked to be simpler. The
details flag is there on legacy and modern flatpak and is able to allow
reading for refs on both. 

Jonathan L. Verner authored Dec 03, 2021 and Igor Kushnir committed Dec 14, 2021

When renaming a variable, the function `BasicRefactoring::executeRenameAction`
calls the `isValid` method of `IndexedDeclaration` which is documented to
require the DUChain to be read locked. However the function does no such
locking. This commit introduces a lock around the two calls.

BUG: 444567
FIXED-IN: 5.8.220400

(cherry picked from commit 271731d1)

- on my system meson is a link to python2-exec
  $ ls -ld $(which meson)
  /usr/bin/meson -> ../lib/python-exec/python-exec2
  The python2-exec must not be executed directly.
- Currently the mesonExecutable is canonicalized, which also resolves symlinks
  This causes the meson plugin to execute stuff like:
  $ python2-exec --reconfigure
  instead of
  $ meson --reconfigure
  Fix this by not canonicalizing the mesonExecutable, validity is already checked earlier.

BUG: 412477
FIXED-IN: 5.8.220400

https://ericniebler.com/2013/08/07/universal-references-and-the-copy-constructo
describes the issue solved by this commit. The blog post recommends
constraining the forwarding-reference constructor. But that solution
would require much more code than added in this commit. Especially
because the constraint would have to be expanded to support template
parameter pack.

The deleted constructor overloads prevent surprising and undesirable
compilation of the following two code snippets:
    ScopedDialog<QDialog> s;
    ScopedDialog<QDialog> s2(s);
and
    const ScopedDialog<QDialog> s;
    ScopedDialog<QDialog> s2(std::move(s));
They were compiled successfully because the forwarding-reference
constructor was a better match than the deleted copy and move
constructor respectively (due to const-qualifier differences). In the
forwarding-reference constructor invoked in these snippets, a reference
to ScopedDialog `s` was implicitly converted to QDialog* and passed as a
parent...

Move constructor and move assignment operator have to be defaulted or
deleted to follow the rule of five. ScopedDialog is supposed to live
inside a specific scope, so moving it doesn't make sense. A similar
class template QScopedPointer is also not movable. So let us define the
move operations as deleted via Q_DISABLE_COPY_MOVE.

Unlike Q_DISABLE_COPY, Q_DISABLE_COPY_MOVE prevents the forwarding
constructor from acting as a move constructor. So the following code
    ScopedDialog<QDialog> s;
    ScopedDialog<QDialog> s2(std::move(s));
compiled before, but does not compile at this commit.

Move this declaration closer to other special member functions.

This change improves performance by doing less copying during
construction. In addition the forwarding allows to pass non-copyable
objects by reference.

GIT_SILENT

Pedro Olsen Ferreira authored Nov 18, 2021 and Igor Kushnir committed Nov 26, 2021

The declaration of QMutex(Recursive) is deprecated.
This fixes several deprecation warnings.
Many declarations had to be patched in order to return the correct
constructs, because QRecursiveMutex is not convertible to QMutex.

Igor Kushnir authored Nov 20, 2021 and Milian Wolff committed Nov 26, 2021

The loop in DocumentParsePlan::removeTargetsForListener() caches
m_targets.cend(), which can become invalidated when a target is erased
inside this loop. Comparing to the invalidated end iterator leads to
undefined behavior.

This undefined behavior makes QmlJS's test_files crash every other time,
both on my system and on the CI. The alternating crashes of this test
can be seen by clicking Next Build repeatedly starting from the first
build since the bug was introduced:
https://build.kde.org/job/KDevelop/job/kdevelop/job/kf5-qt5%20SUSEQt5.15/165/testReport/junit/projectroot.plugins.qmljs/tests/test_files/

KDevelop often crashes because of this undefined behavior too (see the
bug report referenced below).

m_targets is a QSet, which is implemented in terms of QHash. QHash's end
iterator `e` equals the d-pointer `d` via anonymous union. So the only
way cend() can be invalidated is if QSet::erase() detaches. That's
possible, because an entire DocumentParsePlan is copied in
BackgroundParserPrivate::parseDocumentsInternal():
    const DocumentParsePlan parsePlan = *parsePlanConstIt;
The involvement of multithreading explains why test_files crashes every
other time rather than each time. Examining call stacks of both
test_files and kdevelop crashes confirms this hypothesis: a background
thread crashes in DocumentParsePlan::removeTargetsForListener() while
the main thread waits on the `m_mutex.lock()` line in
parseDocumentsInternal().

This bug was introduced in 5ee9b9fe.

BUG: 445699

In case of conflict in i18n, keep the version of the branch "ours"
To resolve a particular conflict, "git checkout --ours path/to/file.desktop"

Do not repeat shell test in the kdevplatform_shell_environment.sh file,
but instead detect the shell which runs the script via ps and $$. The
testing and shell selection is done by the app/kdevelop! script.

Fix tests for executables: which is a builtin in zsh and output
redirestion does not work with it. Replace such tests with command -v,
for which POSIX defines behaviour and return values.

BUG: 442481
FIXED-IN: 5.7.0

Jonathan L. Verner authored Nov 15, 2021 and Igor Kushnir committed Nov 15, 2021

Previously, the goto source shortcut for summary diffs (e.g. diffs showing
all staged changes) would not work. The reason was that
the view data structure member url contained only the path to the project
root. This commit changes

  1. modifies the vcsDiff api diffLineToSource/Target
     to return relative source paths together with source lines
  2. uses the result together with the project root to compute
     the url to the path in the shortcut handler.

In case of conflict in i18n, keep the version of the branch "ours"
To resolve a particular conflict, "git checkout --ours path/to/file.desktop"

In case of conflict in i18n, keep the version of the branch "ours"
To resolve a particular conflict, "git checkout --ours path/to/file.desktop"

Initialize m_plugin with nullptr.

Simplify the expression assigned to perforceTestBaseDir.

The temporary directories are created before and removed after each test
function. This makes switching to QTemporaryDir more difficult and less
useful => retain QDir::tempPath() for the time being.

These directories were created and removed but never used.

PerforcePluginTest::removeTempDirsIfAny() attempted to remove
perforceTestBaseDir twice because of what was almost certainly a typo.
perforceTestBaseDir2 was supposed to be removed there, but it is better
to never create and never remove this unused directory.

ItemRepositoryRegistry::deleteRepositoryFromDisk() only marks the
repository for removal. Call ItemRepositoryRegistry::shutdown() to
actually remove the files when done testing. Even though QTemporaryDir
already removes the directory with all repository files in it, let us
shut down the repositories properly to test their cleanup too.

The temporary directory and files are now removed at test/bench exit.