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

Clean up leftover files before serialization doctests #4573

Conversation

lgoettgens
Copy link
Member

Resolves #4572.

@lgoettgens lgoettgens added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Feb 10, 2025
@lgoettgens lgoettgens requested review from aaruni96 and removed request for fingolfin February 10, 2025 16:42
@aaruni96
Copy link
Member

I think this may ultimately not be needed. If you write to a file, and it already exists, you just overwrite it. And if you can't overwrite it, chances are that you can't remove it either (as was the case which motivated this : the file was owned by someone else and the runner process couldn't write to it).

My current concern is what happens when two different tests running on two different runners try to do this to the same file at the same time. Either we make the runners present a different tmp to each run of the test, or, in the test, we use mktemp and write files there (and cleanup after the run).

Thoughts @lgoettgens @fingolfin @benlorenz ?

@lgoettgens
Copy link
Member Author

I think that our CI jobs shouldbe allowed to expect not to share any memory with other concurrently running jobs. In that regard, I think the cleanest solution would be that each runner has its own /tmp directory, as it is the case for the github-hosted runners.

@benlorenz
Copy link
Member

benlorenz commented Feb 11, 2025

I would like to do it with a temporary directory in the doctest, but it should not be user-visible. And so far Documenter doesn't have a cleanup for a doctestsetup (to get back after switching the directory in a setup block).

In this case, I don't think this can trigger a race-condition as long as all runners have the same uid: The files are always identical and are written atomically, i.e. any job can rewrite them and there should always be a file to read from.
(In this case the atomic rewrite failed because of the permissions and then julia used cp && rm as fallback, which also failed)

@fingolfin
Copy link
Member

So having JuliaDocs/Documenter.jl#2577 in Documenter would be useful.

@lgoettgens lgoettgens closed this Feb 12, 2025
@lgoettgens lgoettgens deleted the lg/serialization-doctest-cleanup branch February 12, 2025 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doctest failure on macos (serialization related)
4 participants