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

test: fix line endings in tests on Windows #68

Merged
merged 15 commits into from
Sep 12, 2023
Merged

Conversation

mortenpi
Copy link
Member

I think the issue with the Windows CI is that the git checkout checks the reference files out with \r\n line endings. But we can just normalize the line endings to \n before we string-compare the generated output and the reference output.

@mortenpi mortenpi marked this pull request as draft September 11, 2023 07:13
@mortenpi
Copy link
Member Author

Okay, the line endings issue is just the tip of the iceberg here. The bigger problem is that Windows doesn't really properly support symlinks...

To make Git actually create symlinks on Windows, you can set

git config --global core.symlinks true

Otherwise it seems to just create a text file with the destination directory. But even then it doesn't look like you can really read them, even with admin permissions..

@@ -266,7 +286,8 @@ function maybe_clone(docs::Vector{MultiDocRef})
catch e
# We're only interested in catching `git` errors here
isa(e, ProcessFailedException) || rethrow()
@error "Unable to update existing clone at $(doc.upstream)" exception = (e, catch_backtrace())
@error "Unable to update existing clone at $(doc.upstream)" exception =
(e, catch_backtrace())
Copy link
Member Author

@mortenpi mortenpi Sep 12, 2023

Choose a reason for hiding this comment

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

Unrelated formatting fixes from #61.

Comment on lines +143 to +162
# This keyword is for internal test use only:
_override_windows_isinteractive_check::Bool = false,
)
if Sys.iswindows() && !isinteractive()
if _override_windows_isinteractive_check || isinteractive()
@warn """
Running a MultiDocumenter build interactively in Windows.
This should only be used for development and testing, as it will lead to partial
and broken builds. See https://github.com/JuliaComputing/MultiDocumenter.jl/issues/70
"""
else
msg = """
MultiDocumenter deployments are disabled on Windows due to difficulties
with handling symlinks in documentation sources.
You _can_ test this build locally by running it interactively (i.e. in the REPL).
See also: https://github.com/JuliaComputing/MultiDocumenter.jl/issues/70
"""
error(msg)
end
end
Copy link
Member Author

Choose a reason for hiding this comment

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

@pfitzseb So initially I thought we could check if we run into problematic symlinks, and only then error. But I don't think that works, because by default Git on Windows will not create symlinks as far as I can tell. So the "symlinks" are just normal files, and so we can't really distinguish them easily. So instead I added the isinteractive() checks here at the very top of make.jl.

One consequence here is that we fully "break" running MultiDocumenter on Windows. So we may want to bump this to the next 0.x minor?

@mortenpi mortenpi marked this pull request as ready for review September 12, 2023 02:00
@pfitzseb pfitzseb merged commit 7f60411 into main Sep 12, 2023
10 checks passed
@delete-merged-branch delete-merged-branch bot deleted the mp/fix-windows-ci branch September 12, 2023 11:10
@mortenpi mortenpi mentioned this pull request Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants