public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
To: Markus Heiser <markus.heiser@darmarit.de>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Jani Nikula <jani.nikula@intel.com>,
	Jonathan Corbet <corbet@lwn.net>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	linux-doc@vger.kernel.org,
	Daniel Vetter <daniel.vetter@intel.com>
Subject: Re: [PATCH 1/2] doc/sphinx: Enable keep_warnings
Date: Wed, 20 Jul 2016 09:49:45 -0300	[thread overview]
Message-ID: <20160720094945.2fd01fe9@recife.lan> (raw)
In-Reply-To: <5A592AAE-F6FB-4B0F-83EF-7A3752CDDE05@darmarit.de>

Em Wed, 20 Jul 2016 14:29:01 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> Am 20.07.2016 um 13:27 schrieb Daniel Vetter <daniel.vetter@ffwll.ch>:
> 
> > On Wed, Jul 20, 2016 at 12:55 PM, Markus Heiser
> > <markus.heiser@darmarit.de> wrote:  
> >> Hi Daniel, hi Mauro,
> >> 
> >> Am 19.07.2016 um 17:32 schrieb Daniel Vetter <daniel.vetter@ffwll.ch>:
> >>   
> >>> On Tue, Jul 19, 2016 at 5:25 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:  
> >>>> On Tue, Jul 19, 2016 at 4:59 PM, Markus Heiser
> >>>> <markus.heiser@darmarit.de> wrote:  
> >>>>> 
> >>>>> Am 19.07.2016 um 13:42 schrieb Daniel Vetter <daniel.vetter@ffwll.ch>:
> >>>>>   
> >>>>>> Unfortunately warnings generated after parsing in sphinx can end up
> >>>>>> with entirely bogus files and line numbers as sources. Strangely for
> >>>>>> outright errors this is not a problem. Trying to convert warnings to
> >>>>>> errors also doesn't fix it.
> >>>>>> 
> >>>>>> The only way to get useful output out of sphinx to be able to root
> >>>>>> cause the error seems to be enabling keep_warnings, which inserts
> >>>>>> a System Message into the actual output. Not pretty at all, but I
> >>>>>> don't really want to fix up core rst/sphinx code, and this gets the job
> >>>>>> done meanwhile.  
> >>>>> 
> >>>>> Hi Daniel,
> >>>>> 
> >>>>> may I misunderstood you. Did you really get more or different warnings
> >>>>> if you include them into the output with "keep_warnings"?
> >>>>> 
> >>>>> The documentation says:
> >>>>> 
> >>>>> "Regardless of this setting, warnings are always written
> >>>>>  to the standard error stream when sphinx-build is run."
> >>>>> 
> >>>>> see http://www.sphinx-doc.org/en/stable/config.html#confval-keep_warnings
> >>>>> 
> >>>>> Or did you not run "make cleandoc" first? Sphinx caches the doctrees
> >>>>> and reports markup errors only when you rebuild the cache.
> >>>>> The cache is also rebuild if you touch one of the source, e.g.
> >>>>> the drivers/gpu/drm/drm_crtc.c or the rst-file where the drm_crtc.c
> >>>>> is referred by a kernel-doc directive .. these dependence sometimes
> >>>>> confuse me .. when I missed log messages, I clean the cache e.g. by
> >>>>> target cleandocs.  
> >>>> 
> >>>> Yes I'm aware that sphinx it's WARNINGs when doing a partially
> >>>> rebuild, this is something entirely different. I didn't get more or
> >>>> less warnings this way, but keep_warning = True seems to be the only
> >>>> way to get reasonable information about them. Without that I get
> >>>> warnings (for included kernel-doc) where the source file is the .rst
> >>>> file that pulls in the kernel doc, and the line number is entirely
> >>>> bogus (often past the end of the containing .rst).
> >>>> 
> >>>> With this I can at least then open the generated .html file, search
> >>>> for the System Message and figure out (by looking at the surrounding
> >>>> context) where the error really is from.
> >>>> 
> >>>> Strangely this only happens for WARNING. If I manged the kerneldoc
> >>>> enough to upset sphinx into generating an ERROR, the line numbers and
> >>>> source files are correct.
> >>>> 
> >>>> See patch 2/2 in this series for examples of such WARNINGs: Mostly
> >>>> it's unbalanced _ * or `` annotations that confuse sphinx/rst a bit.
> >>>> If you want to play around with the gpu sphinx conversion to reproduce
> >>>> these locall you can grab the drm-intel-nightly branch from
> >>>> 
> >>>> https://cgit.freedesktop.org/drm-intel
> >>>> 
> >>>> It already includes Jon's latest docs-next branch.  
> >>> 
> >>> btw, I couldn't check this since I didn't figure out how to intercept
> >>> the parsed rst tree and view it, but I think what's going on is:
> >>> - The source file for these warnings is .rst file containing the
> >>> kernel-doc directive. This seems to be a bug in sphinx/docutils since
> >>> we never use that file name when appending files at all.
> >>> - The line number looks like it's just counting the inserted
> >>> kernel-doc lines as part of the containing .rst file. At least
> >>> changing the content_offset in nested_parse seems to suggest that this
> >>> is the start line (e.g. adding 10k there results in all bogus WARNING
> >>> line numbers being increased by 10k). And adding more blank lines at
> >>> the beginning of the inserted kernel-doc rst also increases the
> >>> reported lines. But not when inserting blank lines at the end (i.e. it
> >>> seems like it's being reset after each directive again).  
> >> 
> >> Thanks for the explanation.
> >>   
> >>> All that suggest to me this is a sphinx-internal issue, and google
> >>> sugggests there's lots of errata around line reporting. Hence why I
> >>> went with this. But of course a proper fix would be awesome! Just a
> >>> bit outside of what I think I can pull off ...  
> >> 
> >> It is not really a sphinx-internal issue (rather a drawback of the design).
> >> The state machine needs a system reporter that takes the origin file
> >> and it's line numbers as context.
> >> 
> >> I send a fix to Jon:
> >> 
> >> http://mid.gmane.org/1469011138-12448-1-git-send-email-markus.heiser@darmarit.de
> >> 
> >> could you test this patch and send us some feedback / thanks.  
> > 
> > Yup, seems to work nicely. Thanks a lot for fixing this. Jon, pls
> > drop/revert my hack and take Markus' proper fix instead.
> >   
> >> One remark: The line numbers are not "perfect". This is due to the fact,
> >> that the kernel-doc parser could not generate "perfect" line numbers
> >> or all extracted doc-items .. daniel knows this ;)
> >> 
> >> If you did not find the cause of a warning in the line number given
> >> by the warning, take a look one line or one block above and/or below,
> >> mostly you will see the cause.  
> > 
> > Hm, I think I still have a few off-by-one in the kernel-doc line
> > numbers. But tbh with all the intermediate layers I wasn't sure which
> > one is wrong and where it would need to be fixed up. But it seems like
> > for a bunch of cases kernel-doc reports 1 line too much.
> > 
> > If someone with more insight into all this would try to improve this,
> > I think it'd be awesome ;-)  
> 
> It will never be "perfect" ... as far as I know, Sphinx (docutils) will
> always report on the block level, not on line level of the rst-origin.

Well, with kernel-doc and DocBook, the error reports were always with
some offset, but I don't think that this is a big issue. I mean: if it
points to the right documentation markup block at the source file, it
is not hard to identify where the bugs are.

> 
> The off-by-on could be fixed, I plan to revise the kernel-doc perl script, when
> we know, what we need for man-pages [1], but I will wait for Jon's and Jani's
> thoughts about man pages first.  
> 
> [1] http://mid.gmane.org/2CE565E6-19D4-4835-9A32-2FCAE754B357@darmarit.de
> 
> -- Markus --
> 
> > 
> > Cheers, Daniel
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch  
> 



Thanks,
Mauro

  reply	other threads:[~2016-07-20 12:49 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-19 11:42 [PATCH 1/2] doc/sphinx: Enable keep_warnings Daniel Vetter
2016-07-19 11:42 ` [PATCH 2/2] drm/doc: Fix more kerneldoc/sphinx warnings Daniel Vetter
2016-07-19 12:36   ` Daniel Vetter
2016-07-20 12:20     ` Mauro Carvalho Chehab
2016-07-20 18:35       ` Markus Heiser
2016-07-20 20:48         ` Mauro Carvalho Chehab
2016-07-22 15:06         ` Mauro Carvalho Chehab
2016-07-19 11:49 ` ✗ Ro.CI.BAT: failure for series starting with [1/2] doc/sphinx: Enable keep_warnings Patchwork
2016-07-19 14:59 ` [PATCH 1/2] " Markus Heiser
2016-07-19 15:25   ` Daniel Vetter
2016-07-19 15:32     ` Daniel Vetter
2016-07-20 10:55       ` Markus Heiser
2016-07-20 11:27         ` Daniel Vetter
2016-07-20 12:29           ` Markus Heiser
2016-07-20 12:49             ` Mauro Carvalho Chehab [this message]
2016-07-19 22:23 ` Jonathan Corbet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160720094945.2fd01fe9@recife.lan \
    --to=mchehab@s-opensource.com \
    --cc=corbet@lwn.net \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=markus.heiser@darmarit.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox