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
Win fixes #3409
Win fixes #3409
Conversation
👎 on messing with the symlinks unless it is wrapped in win-specific cut-outs. |
I added the conditionals (squashing the commits) and a warning since changing git-controlled files is just too wrong to pass silently. At least now the warning might aid understanding and will give more documentation in make.py of the ugly. |
shutil.copytree(os.path.join(link, '..', target), link) | ||
|
||
if sys.platform == 'win32' and len(symlink_warnings) > 0: | ||
print('The following items related to symlinks will show up '+ \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a preference for ()
line continuation over \
git on windows really says 'unsupported symlink foo removed' ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git on windows checks on the symlink as a one line text file with the target as the content of said file (why git doesn't properly use the machinery behind mklink is a whole other issue -- NTFS does support symlinks although no one uses them). The shutil.copytree & requisite prior delete will cause spurious changes in git status. The actual string "'unsupported symlink {} removed'" is in the code I just wrote.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I tend to be overly terse (particularly before I have had coffee).
I think it would be best if the error message matched (as closely as possible) what git status
would print out if run in the top level directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, that's a good idea; done
Thanks this looks better to me. |
@Tillsten as another know windows developer can you take a look at this? |
This may be out-of-scope for this PR, but would it be possible to replace the symlink files after building the docs is done? |
I considered replacing them but it still imposes risk of confusion if the doc-making process dies. It's just plum ugly and I'd rather not write more code on ugly. |
Fair enough. |
lgtm to me @mdboom can you take a look at this as well? |
for link, target in required_symlinks: | ||
if sys.platform == 'win32' and os.path.isfile(link): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stupid question, does 64bit windows id as win32?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the check is consistent with many other checks in MPL setup.py related code & verified on python3.4 64 bit myself
I never was able to compile mpl on windows, so i am not sure i am the right person to ask. Maybe i'll give it another try tomorrow to be able to give some feedback. |
@Tillsten This is just about building the docs (which will use what ever version of mpl it can import). |
BLD/DOC : patch over git + symlink issues on windows
@jbmohler Thanks for this, sorry it languished for so long. |
two windows related tweaks
The doc build issue makes clutters 'git status' output after building docs because windows git doesn't handle the sym links correctly (win 7; git 1.9.4). I think its a decent solution in a bad situation.