-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Building documentation from the tarball fails #2187
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
Conversation
Just to double-check, was the install done first from the source that you On Mon, Jul 1, 2013 at 9:57 AM, Michael Droettboom <notifications@github.com
|
I'm able to reproduce. Looking into it now. |
@sandrotosi: Could you confirm that this resolves your issue? |
if hasattr(os, 'symlink'): | ||
os.symlink('../examples', 'mpl_examples') | ||
else: | ||
shutil.copytree('../examples', 'mpl_examples') |
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.
Is there an OS that doesn't support symlinks? If so, should we delete the folder if it was copied (not symlinked)? Of course, there's an argument to be made against a makefile's 'install' target to delete things.
Also, subsequent documentation builds will copy this regardless if it is out of date or not. I notice a copy_if_out_of_date
function on line 237. I think I'd like that to be used here.
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.
MS-Windows doesn't support symlinks, at least not on all file systems. Even Linux, when running on an external FAT32 USB drive, for example, may not support symlinks.
Not a bad idea to "copy_if_out_of_date" (recursively), of course.
Apart from @dmcdougall 's comment, 👍 |
As a windows user, to build the doc from mpl master I manually create the following 2 symbolic links:
and:
I understand this PR fixes the first one ; but I am not enough familiar with the code to know if the second should be handled similarly - and where. (If this is useful I can investigate, though). |
Right, we need to support the other toolkits, too. Good point. |
Thanks. @sandrotosi: I have added another commit to this PR that I believe addresses #2206. Can you test this when you have a chance. This is the last serious blocker for 1.3.0. |
Code wise, this looks good. I'm 👍 for merging once @sandrotosi has confirmed the build works ! |
@mdboom hi and thanks for the fix! I can confirm it works and the documentation is correctly built (I'll re-verify soon in a clean chroot, just to avoid some weird setup of my local machine, but it takes a bit of effort given the time and resources requirement of mpl to be built :) ). |
Building documentation from the tarball fails
This fix works for the tarball case because (evidently) the symlinks are just flat out not included in the tarball. However, "git clone" from a windows machine brings down the symlink as a file -- that is a very silly looking kludge in git from my perspective. Therefore the manual operation as described by @GBillotey is required. But building docs from windows should just work with-out manual intervention. I think that the symlink should simply be deleted from the git repository altogether and @mdboom fix will apply in a source clone as well. That would need to be coupled with some .gitignore things as well. I would welcome (and desire!) alternative suggestions. Another hack is (this is a hack because it modifies files under git control):
|
I am in favor of special casing the doc-build do deal with the brokenness of windows and against removing the symlinks. Copying the files over is a major hack and semi-magical. If you now want to work on those files you have to sort out which version to change and keep them in sync, where as with the symlinks there is only one copy and no confusion. Yes, windows devs would still have this problem, but they made their choices ;), there is no need to inflict this pain on the rest of the devs. Could you put together a PR with that patch? |
As reported by Sandro Tosi on the mailing list in the thread: "Error building documentation for 1.3.0rc4".