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

Comparison of target path and configuration directory on WIN32 #2

Open
kreuzberger opened this issue May 12, 2022 · 3 comments
Open

Comments

@kreuzberger
Copy link
Contributor

In file collections/collections.py the target directory and conf directory are compared to evaluate if the link is created in the right directory. This fails on windows due to comparing configured python paths with os.path.realpath ones.

Assume you must also compare with the os.path.realpath of the target in Line 94:

if not target.startswith(os.path.realpath(app.confdir)): raise CollectionsException( 'Target path is not part of documentation folder\n'

if not os.path.realpath(target).startswith(os.path.realpath(app.confdir)): raise CollectionsException( 'Target path is not part of documentation folder\n'

@danwos
Copy link
Member

danwos commented May 12, 2022

Thanks for the bug report and the detailed analysis 👍

A fix will need some time, as I'm currently quite busy.
If you like and as you already proposed a solution, you can create a PR and add yourself to the AUTHORS file (ohh this is not there yet, but we should create it :) )
And if there are any questions about this, I'm will be happy to answer them.

@kreuzberger
Copy link
Contributor Author

kreuzberger commented May 12, 2022

I will provide a fix cause the suggested solution has issues with clean/incremental build cause "realpath" of the target behaves different if symbolic link exists or not.
To provide a pull request: How do i run the tests in the package? I am not so familiar with python packages....

  • before i commit
  • do they "autorun" after commit?

@danwos
Copy link
Member

danwos commented May 12, 2022

Thanks for supporting here 👍

To be honest, this project does not have any tests or a CI configured.
So this is the reason, why the win-specific problem got not detected. I should really do this one day :(

So the PR can contain the bug fix only. No tests yet...

kreuzberger added a commit to procitec/sphinx-collections that referenced this issue May 13, 2022
kreuzberger added a commit to procitec/sphinx-collections that referenced this issue May 13, 2022
danwos added a commit that referenced this issue Sep 19, 2022
[#2] symlink driver handling
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

No branches or pull requests

2 participants