Stop polluting filesystem (replay)

[strk]

This is a re-proposition of PR GH-57606 after all commits in that PR were reverted

This PR addresses the problem reported by @nyalldawson in #57606 (review) (reportedly causing a segfault on his system)

[nyalldawson]

This still has issues @strk. You don't check to see if that folder exists anywhere, neither is it ever cleaned up. You're trading proper operating system temporary file handling for storing everything in some custom location which is NEVER cleaned up.

[github-actions]

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit da12030)

[strk]

This still has issues @strk. You don't check to see if that folder exists anywhere, neither is it ever cleaned up. You're trading proper operating system temporary file handling for storing everything in some custom location which is NEVER cleaned up.

Given CI is green, may I ask you to give more details about what fails ? Note I filed at least 14 tickets about non-working testsuite when run locally but most of the times those issues are disregarded because "CI works!". I'm happy to improve offline testsuite experience but I need more details about what is not working for you.

So: how exactly those missing checks break the run for you ? Could you see a way to encode those situations in the CI so I can get a red here instead of a green ?

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[benoitdm-oslandia]

Hi, as we are building qgis on a mutliuser basis on the same big-cpu-ram server, we encounter issues with the mixed up of temp files in the /tmp. As our different users build different branches, we cannot use the same user for everyone. And tests fail due to conflicts on the files generated by tests owned by different users.

Do you have some updates about the last remaining issues? Do some tests are still failing?

Regards

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[github-actions]

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.

[strk]

It's a pity to close this PR only because neither @nyalldawson nor me found the time yet to provide a test showing the problem reported by Nyall. The CI is green while we want it to be RED to show the problem. Wasn't there a way to ask stalebot to not get in the way ?

[strk]

You don't check to see if that folder exists anywhere,

I believe the functions used to create temporary directory create the target if non-existing, wouldn't CI be red if this was not the case ?

neither is it ever cleaned up.

This is already the case before this PR, the only difference is that all that mess is left in the system shared /tmp directory rather than in the build tree. I agree it is a problem but is a problem that needs to be addressed by each test leaving temp files behind (not all tests do).

You're trading proper operating system temporary file handling for storing everything in some custom location

It's the build directory, it is already used to store build artifacts. These temporary files are the same. I agree things could be further improved and it would be easy to wipe out the whole temporary directory at the end of the ctest run but as we're not talking about a problem introduced by this PR I'd rather merge the improvement and further improve later instead of letting this opposed by stalebot and letting it fade out.

[nyalldawson]

@strk you didn't actually fix any of the issues I've explicitly outlined in #57882 .👎 I'm out of patience and time for dealing with this, and am muting this issue now. I'll leave to others to carefully review and confirm that my issues have been resolved before they take the response to merge.

[benoitdm-oslandia]

Hi @strk, to go forward on this issue I created a PR (but I forgot to mention this PR in mine). I fixed some of the hardcoded '/tmp/' but I did not changed the CMake build.

Now, it is possible to sandbox the test runs in a specific directory by modifying the TMPDIR env var.

Regards.

[benoitdm-oslandia]

linked to qgis/QGIS-Documentation#9208

[strk]

Thank you @benoitdm-oslandia

[strk]

So this PR changes are solely to change the default temporary directory from the system temporary directory to a 'tmp' directory under the build tree. We can see CI is still green so the target directory does not need seem to exist or the tests are broken (false successes?). It would help if @nyalldawson gave us a recipe to reproduce the problem he's having with these changes.

I've observed locally that the system /tmp dir is still polluted with files like /tmp/QGIS-PythonTestConfigPath-xxxxx directories, and I found still some hard-coded /tmp directories that could be further turned into using a tempdir function (ie: src/core/maprenderer/qgsmaprendererjob.cpp)

I could work on those hard-coded tmpdir in a separate PR if it's easier to accept

[benoitdm-oslandia]

sorry @strk as said above the hard-coded tmpdir were already fixed here.

[strk]

sorry @strk as said above the hard-coded tmpdir were already fixed here.

Yeah, on a closer look I found that the left-over hard-coded /tmp/ are protected by debug macros, probably not worth fixing them at the moment.

What do you think about using the build dir to store the temporary files ? If we do so we may wipe out the directory at test start (as a fixture). I don't see two concurrent builds using the same build dir, which should also resolve your multi-user issue

[benoitdm-oslandia]

It seems it is better not to bother with the cmakelist ... so I started to prefix my command lines with TMPDIR=/tmp/my_test_dir to isolate my test runs. It is not as inconfortable as it looks and it does the job.

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[github-actions]

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.