Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop polluting filesystem (replay) #57934

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

strk
Copy link
Contributor

@strk strk commented Jun 29, 2024

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)

@strk strk requested a review from nyalldawson June 29, 2024 04:52
@github-actions github-actions bot added this to the 3.40.0 milestone Jun 29, 2024
@strk strk requested a review from elpaso June 29, 2024 04:54
@strk strk added the testsuite Issue related to testsuite label Jun 29, 2024
Copy link
Collaborator

@nyalldawson nyalldawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

github-actions bot commented Jun 29, 2024

🪟 Windows builds ready!

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

(Built from commit e482323)

strk added 3 commits June 29, 2024 08:33
This is an amended cherry-pick of commit da11a8a
which was reverted in commit a0e6e16 by Nyall Dawson

The amendment is a fix in QString syntax as spotted by Nyall
in
qgis#57606 (review)
@strk
Copy link
Contributor Author

strk commented Jun 30, 2024

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 ?

Copy link

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 github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 15, 2024
@benoitdm-oslandia
Copy link
Contributor

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 github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 19, 2024
Copy link

github-actions bot commented Aug 3, 2024

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 github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Uh oh! Seems this work is abandoned, and the PR is about to close. testsuite Issue related to testsuite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants