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

Building documentation from the tarball fails #2187

Merged
merged 2 commits into from Jul 19, 2013

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Jul 2, 2013

As reported by Sandro Tosi on the mailing list in the thread: "Error building documentation for 1.3.0rc4".

@WeatherGod
Copy link
Member

Just to double-check, was the install done first from the source that you
are building the docs from?

On Mon, Jul 1, 2013 at 9:57 AM, Michael Droettboom <notifications@github.com

wrote:

As reported by Sandro Tosi on the mailing list in the thread: "Error
building documentation for 1.3.0rc4".


Reply to this email directly or view it on GitHubhttps://github.com//issues/2187
.

@mdboom
Copy link
Member Author

mdboom commented Jul 2, 2013

I'm able to reproduce. Looking into it now.

@ghost ghost assigned mdboom Jul 2, 2013
@mdboom
Copy link
Member Author

mdboom commented Jul 2, 2013

doc/mpl_examples is a symlink to examples (to get around the fact that Sphinx won't let the doc build pull from files outside of the doc tree). distutils includes the symlink in the tarball, but setuptools (presumably because it tries to produce cross-platform packages) does not. The solution I came up with is to just create the symlink within the doc build script if it doesn't exist.

@sandrotosi: Could you confirm that this resolves your issue?

if hasattr(os, 'symlink'):
os.symlink('../examples', 'mpl_examples')
else:
shutil.copytree('../examples', 'mpl_examples')
Copy link
Member

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.

Copy link
Member Author

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.

@NelleV
Copy link
Member

NelleV commented Jul 4, 2013

Apart from @dmcdougall 's comment, 👍

@GBillotey
Copy link
Contributor

As a windows user, to build the doc from mpl master I manually create the following 2 symbolic links:

cd doc
set link_path="mpl_examples"
set target_path="..\examples"
MKLINK /D %link_path% %target_path% 

and:

cd mpl_toolkits\axes_grid
set link_path="examples"
set target_path="..\..\..\examples\axes_grid"
MKLINK /D %link_path% %target_path% 

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).

@dmcdougall
Copy link
Member

Right, we need to support the other toolkits, too. Good point.

@sandrotosi
Copy link
Contributor

@mdboom I can confirm that the patch allows the doc to start be built, but it uncovered #2206

@mdboom
Copy link
Member Author

mdboom commented Jul 17, 2013

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.

@NelleV
Copy link
Member

NelleV commented Jul 18, 2013

Code wise, this looks good. I'm 👍 for merging once @sandrotosi has confirmed the build works !

@sandrotosi
Copy link
Contributor

@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 :) ).

mdboom added a commit that referenced this pull request Jul 19, 2013
Building documentation from the tarball fails
@mdboom mdboom merged commit bda9f25 into matplotlib:v1.3.x Jul 19, 2013
@mdboom mdboom deleted the build-docs-from-tarball branch August 7, 2014 13:54
@jbmohler
Copy link
Contributor

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):

index ceaea5f..c81f26a 100755
--- a/doc/make.py
+++ b/doc/make.py
@@ -250,6 +250,16 @@ required_symlinks = [
     ]

 for link, target in required_symlinks:
+    if os.path.isfile(link):
+        # This is special processing that applies on platforms that don't deal
+        # with git symlinks -- probably only MS windows.
+        delete = False
+        with open(link, 'r') as content:
+            delete = target == content.read()
+        if delete:
+            os.unlink(link)
+        else:
+            raise RuntimeError("doc/{} should be a directory or symlink -- it isn't")
     if not os.path.exists(link):
         if hasattr(os, 'symlink'):
             os.symlink(target, link)

@tacaswell
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants