D17289: KDevelop/Shell: set dedicated TMPDIR

[Milian Wolff]

mwolff added a comment.


  In D17289#429526 <https://phabricator.kde.org/D17289#429526>, @rjvbb wrote:
  
  > > And there are probably places where the simpler QProcess API was used
  > >  instead.
  >
  > But the KDevelop env. profiles are already based on or compatible with QProcess::setProcessEnvironment(), no?
  
  
  yes, but you actually have to use those profiles. And there are places where we don't do that, that's my point.
  
  >>   > Would it be feasible to reduce the footprint of this overhaul by using a wrapper around writableLocation(QSP::TempLocation) instead?
  >>   
  >>   I don't see how that would change anything, considering we need to change TMPDIR for libclang, which isn't using Qt internally?
  > 
  > I think that what I had in mind was that replacing one function call with another could be easy and unique enough to do it with a global search/replace. I didn't write down the exact thought I had when I asked that question but it must have been for a wrapper with an appropriate default for a flag that indicates whether or not the original TMPDIR had to be used/replaced.
  
  It's not one function call, it's different ones. There are the static `[KQ]Process` functions, then the non-static versions too.
  
  >>   You cannot, since env vars are process-wide and thus changing them from a background thread would lead to race conditions.
  > 
  > And that's going to remain a problem if there are places in KDevelop where you can only (re)set TMPDIR before spawning the child as opposed to changing the value in the child process (= between the fork() and the exec()).
  
  No, when you use QProcessEnvironment, then it won't mess with TMPDIR on the parent process.
  
  > In short, we have only 2 alternative (to this here) approaches that don't require a huge overhaul:
  > 
  > - request some sort of libclang::setTmpDir() function (and hope it becomes available this lifetime)
  
  I doubt they'll ever add that. We have much more useful features waiting in review limbo for a long time.
  
  > - add an environment profile to be applied to KDevelop itself at startup, so that users can decide for themselves if they want to change TMPDIR to something specific that will then be applied to all processes spawned by KDevelop too. I have a local patch (rejected long ago) that applies the selected default profile and it's been working just fine for me.
  
  Do it right, or don't do it.
  
  > Related question: shouldn't the environment profiles have one or all of these?
  > 
  > - an option to clear the existing environment and apply only the variables set in the profile, possibly with a whitelist of variables to preserve
  
  yes
  
  > - a list of variables to unset (if that's not already possible by leaving their value entry empty)
  
  setting to empty should do it
  
  > - a list of variables to cache at startup along with syntax to use these cached values (probably relevant only if profile is applied at startup)
  
  no, use a batch/bash script instead

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, kfunk, mwolff
Cc: aaronpuchert, mwolff, pino, kfunk, kdevelop-devel, gennad, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190312/0fe175f9/attachment.html>