D17289: KDevelop/Shell: set dedicated TMPDIR

[René J.V. Bertin]

rjvbb created this revision.
rjvbb added a reviewer: KDevelop.
rjvbb added a project: KDevelop.
rjvbb requested review of this revision.

REVISION SUMMARY
This is a simple patch implementing a dedicated KDevelop temporary directory (on Unix). With this, every session has its own directory for temporary files under the default `QSP::TempLocation` directory, set via the `TMPDIR` env. variable. The clang parser's preamble* files go here, for instance, but also any files the session creates under `QSP::TempLocation` itself.

The entire directory is removed during a clean exit, and also at startup. This takes care of preventing the accumulation of (large) old temporary files I tend to get on Mac.

TEST PLAN
Tested on Mac and Linux. Processes launched by KDevelop will also see the new directory as their `TempLocation` but I haven't noticed any unwanted side-effects in my workflow. Processes spawned by the session and outliving it may run into issues but how common are those? (The only such processes I create are other KDevelop sessions which are evidently not concerned.)

Setting TEMPDIR (or TEMP) instead of TMPDIR may have the same on MS Windows(?)

REPOSITORY
R32 KDevelop

REVISION DETAIL
https://phabricator.kde.org/D17289

AFFECTED FILES
kdevplatform/shell/core.cpp
kdevplatform/shell/core_p.h

To: rjvbb, #kdevelop
Cc: kdevelop-devel, glebaccon, hase, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Kevin Funk]

kfunk requested changes to this revision.
kfunk added a comment.
This revision now requires changes to proceed.


Not usable as-is.

- You're changing behavior for all of KDevelop, while you only want to fix the Clang backend's behavior. This can lead to subtle side effects. I'm not sure we want this. Not sure how to implement this properly the way we use libclang right now either, without further thought.
- Why not using `QTemporaryDir` in the first place? Also see patch in QtCreator (https://codereview.qt-project.org/#/c/142566). (Yes, I know QtC usesd libclang out-of-process, we don't. Not relevant here. The interesting bits are the env vars and the `QTemporaryDir` usage.)

This needs more thought.

REPOSITORY
R32 KDevelop

REVISION DETAIL
https://phabricator.kde.org/D17289

To: rjvbb, #kdevelop, kfunk
Cc: kfunk, kdevelop-devel, glebaccon, hase, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Kevin Funk]

kfunk added a comment.


Forgot: Also needs documentation *in* source code why were are doing this. Noone can figure out that the temp dir is changed due to issues with the Clang backend here...

REPOSITORY
R32 KDevelop

REVISION DETAIL
https://phabricator.kde.org/D17289

To: rjvbb, #kdevelop, kfunk
Cc: kfunk, kdevelop-devel, glebaccon, hase, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[René J.V. Bertin]

rjvbb added a comment.


Actually, issues with clang temp files is why I started to think about this kind of change but ultimately I realised it didn't seem such a bad idea at all to put all KDevelop-related temporary files in a dedicated location. I often clean out a bunch of KDevelop's own temp files that were left behind, e.g. after a crash. I just didn't mention them because they're negligible in size (and I purge before their numbers really start to grow).

I'm not sure what QTemporaryDir would improve here; I use a deterministic name for a reason here. The relevant Qt Creator code I've seen also sets TMPDIR. Probably because it gets picked up by QSP, and most non-Qt applications also seem to recognise TMPDIR as the variable that defines the (user) location for storing temporary files.

I'm curious what kind of subtle side effects others here can think of. The only one I could think of is what could happen with processes that outlive their parent KDevelop process. For the rest, every application spawned by KDevelop should see the same TMPDIR location and I'm guessing (hoping) that none expect to exchange files with other processes under the assumption that everyone uses the same temp. directory.
Files in TMPDIR are usually private, or else their full path is handed to external apps that need to use them (like sockets), no?

Note that there is a design oversight in the initial version: if the QSP::TempLocation ends with `/kdevelop-temp-session-XXXX` that substring should be clipped to avoid recursion when launching another session through KDevelop.

REPOSITORY
R32 KDevelop

REVISION DETAIL
https://phabricator.kde.org/D17289

To: rjvbb, #kdevelop, kfunk
Cc: kfunk, kdevelop-devel, glebaccon, hase, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

[Kevin Funk]

This post might be inappropriate. Click to display it.

[Pino Toscano]

This post might be inappropriate. Click to display it.