[FFmpeg-devel] [PATCH] doc/Doxfyile: Refactor for maintainability

[Andrew Sayers]

On Tue, Apr 30, 2024 at 07:26:39PM +0200, Michael Niedermayer wrote:
> On Mon, Apr 29, 2024 at 09:53:13AM +0100, Andrew Sayers wrote:
> > On Sun, Apr 28, 2024 at 11:46:18PM +0200, Michael Niedermayer wrote:
> > > Hi
> > > 
> > > On Sun, Apr 28, 2024 at 11:10:22AM +0100, Andrew Sayers wrote:
> > > [...]
> > > > Ping ?
> > > > 
> > > > Michael, this is in response to your request[0].  I've queued up some more
> > > > patches that depend on this and would help attract and retain developers (e.g.
> > > > warning when you're looking at old documentation).  What would be involved in
> > > > getting this sort of thing on the website itself?
> > > 
> > > I dont have the time to review this
> > > 
> > > if you split the patch between updates to to comments and updates to teh actual
> > > configuration this may make it easier to review to whoever does review it
> > 
> > Thanks for letting me know.
> > 
> > For anyone who does pick this up - this patch does not contain any changes
> > to the configuration.  I've attached a default 1.8.2 configuration file in case
> > you want to use them for a review, but it's a real timesink so I'd recommend
> > the build-and-diff approach in the patch description.
> > 
> > My interest in this patch was about getting the website itself updated.
> > My understanding is that can't happen without Michael's involvement,
> > so I'm planning to focus my attention elsewhere.
> 
> I can update the file on the server but i cannot currently review a 200+k doxy
> configuration diff someone else needs to review whatever needs review for the doxy config
> 
> thx

Reading the patch again, I don't think the patch body is the problem.
The problem is that the message doesn't properly explain what it does.

The repo configuration (doc/Doxyfile) and website configuration[0] have spent
a decade drifting apart, and now behave in ways that are too different to knit
back together in a single patch.  So this patch avoids any attempt to analyse
the logic, and simply provides a collection of files that produce byte-for-byte
identical output while being easier to maintain in future.

doc/Doxyfile-1.8.17 is literally just the 1.8.17 output of `doxygen -g`.

doc/Doxyfile-website contains configuration that existed in the file you
provided, but not in the stock 1.8.17 configuration or doc/Doxyfile.

doc/Doxyfile contains configuration that existed in the old doc/Doxyfile,
but not in the stock 1.8.17 configuration or your file.

doc/Doxyfile-FFmpeg contains configuration that was in both your file and the
old doc/Doxyfile, but not the stock 1.8.17 configuration.

So each file represents at least a three-way diff, arguably four or five
if you include the older stock configuration files they were originally based
on.  Trying to present that as a series of two-way diffs will always be
confusing, so it's best not to read the diff itself, and instead concentrate
on the contents of the new files.

Reviewing doc/Doxyfile-1.8.17 just means doing `doxygen -g` with the version of
doxygen on the server, then diffing them to confirm they're the same.

doc/Doxyfile looks like a huge diff because 99% of the configuration is now in
doc/Doxyfile-1.8.17.  The diff is particularly un-human-readable for this file,
so I've attached a copy of the new file for reference.  Would it be better if
the attached file was called doc/Doxyfile-repo, and the old doc/Doxyfile
was changed to just "@INCLUDE = doc/Doxyfile-repo" and a recommendation to
update any scripts that pointed there?

doc/Doxyfile{,-FFmpeg,-website} are only about 200 lines between them.
Running doxygen with old and new configuration should confirm they don't
change behaviour, so reviewing them beyond that just means skimming through
to make sure I haven't snuck in an `rm -rf /` or something.  You can review
these files for logic if you like, and if I've done my job right you should 
feel a strong temptation to fix them.  But doing so is a problem for another
patch, so explicitly out-of-scope for this review.

[0] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/326010.html
-------------- next part --------------
# Local doxygen configuration
#
# This configuration is optimised to produce pleasant documentation,
# but its output may change at any time.
#
# Only put configuration here if it breaks compatibility with older versions.
# For example, if a new version of doxygen adds a new option,
# enable it here.  Put "safe" improvements in Doxyfile-FFmpeg,
# and compatibility changes in Doxyfile-website

@INCLUDE = doc/Doxyfile-1.8.17
@INCLUDE = doc/Doxyfile-FFmpeg

#---------------------------------------------------------------------------
# Project related configuration options
#---------------------------------------------------------------------------

OUTPUT_DIRECTORY       = doc/doxy
STRIP_FROM_INC_PATH    = .

#---------------------------------------------------------------------------
# Configuration options related to the preprocessor
#---------------------------------------------------------------------------

PREDEFINED             = __attribute__(x)= \
                         "DECLARE_ALIGNED(n,t,v)=t v" \
                         offsetof(x,y)=0x42 \
                         av_alloc_size(...)= \
                         AV_GCC_VERSION_AT_LEAST(x,y)=1 \
                         AV_GCC_VERSION_AT_MOST(x,y)=0 \
                         "FF_PAD_STRUCTURE(name,size,...)=typedef struct name { __VA_ARGS__ } name;" \
                         __GNUC__