Opened 13 years ago

Closed 12 years ago

#6605 closed defect (fixed)

[with patch, positive review] sage -docbuild DOC FORMAT should do better error checking on DOC

Reported by: jhpalmieri Owned by: jhpalmieri
Priority: major Milestone: sage-4.2
Component: documentation Keywords:
Cc: Merged in: sage-4.2.alpha0
Authors: John Palmieri Reviewers: Mitesh Patel
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

If I run sage -docbuild hello html, then a directory "hello" is created in SAGE_ROOT/devel/sage/doc/en. Then if I run sage -docbuild -help, "hello" is listed as one of the options.

Error-checking should be done on the "document" argument of sage -docbuild to make sure this doesn't happen.

Attachments (1)

trac_6605-docbuild-check.patch (1.5 KB) - added by jhpalmieri 13 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 13 years ago by jhpalmieri

  • Authors set to John Palmieri
  • Owner changed from tba to jhpalmieri
  • Status changed from new to assigned
  • Summary changed from sage -docbuild DOC FORMAT should do better error checking on DOC to [with patch, needs review] sage -docbuild DOC FORMAT should do better error checking on DOC

As written, this depends on #6187 (although it would be easy enough to modify it to avoid this). The new OMIT variable is for situations such as the one introduced by #5653, where there is a subdirectory "introspect" of SAGE_ROOT/doc/en/, which is not a document to be processed by "sage -docbuild", and so should be omitted from the list of all such documents. (This patch doesn't depend on #5653, but it will be compatible with it, should #5653 be merged.)

comment:2 Changed 13 years ago by mpatel

The patch works for me. I think a non-zero exit value is the UNIX convention for an error. I can make an updated patch. Or I can wait for more feedback on #6187.

Notes for other potential tickets:

  • We could also print a similar message in the less severe case of an invalid format.
  • Instead of using sys.exit(), we could raise (and catch) some exception, so that the builder can loop over documents, formats, and commands, printing warnings as necessary. At the moment, though, this feature is not implemented.

Changed 13 years ago by jhpalmieri

comment:3 Changed 13 years ago by jhpalmieri

This patch is identical, except it uses sys.exit(1) instead of sys.exit(0).

comment:4 Changed 13 years ago by mpatel

  • Reviewers set to Mitesh Patel
  • Summary changed from [with patch, needs review] sage -docbuild DOC FORMAT should do better error checking on DOC to [with patch, positive review] sage -docbuild DOC FORMAT should do better error checking on DOC

comment:5 Changed 13 years ago by mvngu

  • Merged in set to Sage 4.1.1.alpha1
  • Resolution set to fixed
  • Status changed from assigned to closed

comment:6 Changed 13 years ago by mvngu

The patch applies but with fuzz:

[mvngu@sage sage-main]$ hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6605/trac_6605-docbuild-check.patch && hg qpush
adding trac_6605-docbuild-check.patch to series file
applying trac_6605-docbuild-check.patch
patching file doc/common/builder.py
Hunk #3 succeeded at 614 with fuzz 1 (offset -44 lines).
Now at: trac_6605-docbuild-check.patch

comment:7 Changed 13 years ago by mvngu

  • Merged in Sage 4.1.1.alpha1 deleted
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Summary changed from [with patch, positive review] sage -docbuild DOC FORMAT should do better error checking on DOC to [with patch, needs work] sage -docbuild DOC FORMAT should do better error checking on DOC

The patch trac_6605-docbuild-check.patch is buggy when it comes to (re)building the documentation of a new release. Assume you have applied this patch to Sage 4.1.1.alpha0, then build a source release with

sage -sdist <version>

or a binary release with

sage -bdist <version>

After that, upgrade a previous release to this new release and rebuild the documentation. You then get this error message:

[mvngu@sage sage-4.1.1.alpha1-test-x86_64-Linux]$ ./sage -docbuild tutorial htmlTraceback (most recent call last):
  File "/scratch/mvngu/sage-4.1.1.alpha1-test-x86_64-Linux/devel/sage/doc/common/builder.py", line 673, in <module>
    getattr(get_builder(name), type)()
  File "/scratch/mvngu/sage-4.1.1.alpha1-test-x86_64-Linux/devel/sage/doc/common/builder.py", line 616, in get_builder
    elif name in get_documents() or name in AllBuilder().get_all_documents():
NameError: global name 'get_documents' is not defined

The same error is raised if you rebuild the documentation of the binary version of the new release. I'm reopening this ticket and marking it as needs work.

comment:8 Changed 13 years ago by jhpalmieri

  • Summary changed from [with patch, needs work] sage -docbuild DOC FORMAT should do better error checking on DOC to [with patch, positive review] sage -docbuild DOC FORMAT should do better error checking on DOC

It doesn't need work. As I mentioned in the first comment, it depends on #6187. That's where "get_documents" is defined.

Please wait for #6187 to merge.

comment:9 Changed 12 years ago by mhansen

  • Merged in set to sage-4.2.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.