D17289: KDevelop/Shell: set dedicated TMPDIR
René J.V. Bertin
noreply at phabricator.kde.org
Mon Jan 28 10:52:38 GMT 2019
rjvbb added a comment.
> we use CXTranslationUnit_CreatePreambleOnFirstParse to get code completion results fast. otherwise the first code completion request would create the preamble, which felt much worse
Shall we keep that discussion to D18551 <https://phabricator.kde.org/D18551>?
> that's ~50ish code locations... Quite a lot :-/
Erm, yes, so many that it begs the question if "this is worth it"... Is it?
I only looked at places where the existing env. profile feature is used. There were already enough of those that I didn't feel like changing them all potentially for nothing, but I don't think there were this many.
Why would we not limit the change to those locations, or in other words, why is the selected env. profile not used for every process KDevelop spawns?
Would it be feasible to reduce the footprint of this overhaul by using a wrapper around writableLocation(QSP::TempLocation) instead? I'd have to check in how many locations libclang reads TMPDIR and so if it's feasible to change that variable temporarily around the parser's entrypoints into libclang.
(replying by email)
>> +#if defined(Q_OS_UNIX)
>> + auto tmpLocation = QStandardPaths::writableLocation(QStandardPaths::TempLocation);
> imo this should also be handled on windows via `TMP` and `TEMP`, according to
Aren't those used in the MSWin version of `TempLocation`? I can try to implement this for MSWin too but only if someone promises to test it (I can't).
BTW, cf. https://codereview.qt-project.org/#/c/239952
> all of this code shouldn't be required - the system env used to launch subprocesses should not use the changed tmpdir after all, so it cannot be recursive
Evidently this can go once that restored system env. is used.
>> + m_tmpDir->mkpath(tmpLocation);
> it exists, you create the path in the line above
Except when mkpath failed because tmpLocation was messed up. KDevelop does NOT like that. That happened to me during development and I decided to leave the check in.
>> + void systemEnvironment(QProcessEnvironment& env) const;
> don't use out params, just return a QProcessEnvironment similar to how it's done in `QProcessEnvironment::systemEnvironment()`
This was out of a concern that certain spawned processes might be performance critical. I guess the overhead of the extra copy can be neglected then?
To: rjvbb, #kdevelop, kfunk, mwolff
Cc: aaronpuchert, mwolff, pino, kfunk, kdevelop-devel, glebaccon, hase, antismap, iodelay, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the KDevelop-devel