Re: First SVG graphic

On 07/02/2019 18:11, Jürgen Purtz wrote:
>>> @@ -152,15 +156,15 @@ postgres.txt: postgres.html
>>>  postgres.pdf:
>>>   $(error Invalid target;  use postgres-A4.pdf or postgres-US.pdf as targets)
>>>
>>> -%-A4.fo: stylesheet-fo.xsl %.sgml $(ALLSGML)
>>> +%-A4.fo: stylesheet-fo.xsl %.sgml
>>>   $(XMLLINT) $(XMLINCLUDE) --noout --valid $(word 2,$^)
>>>   $(XSLTPROC) $(XMLINCLUDE) $(XSLTPROCFLAGS) --stringparam paper.type A4 -o $@ $(wordlist 1,2,$^)
>>>
>>> -%-US.fo: stylesheet-fo.xsl %.sgml $(ALLSGML)
>>> +%-US.fo: stylesheet-fo.xsl %.sgml
>>>   $(XMLLINT) $(XMLINCLUDE) --noout --valid $(word 2,$^)
>>>   $(XSLTPROC) $(XMLINCLUDE) $(XSLTPROCFLAGS) --stringparam paper.type USletter -o $@ $(wordlist 1,2,$^)
>>>
>>> -%.pdf: %.fo
>>> +%.pdf: %.fo $(ALLSGML) $(SVGSRC)
>>>   $(FOP) -fo $< -pdf $@
>>>
>> This seems a bit wrong.  The .fo target does depend on ALLSGML.  The
>> .pdf target does not, but it presumably does depend on SVGSRC.
> It's a transitive dependency: the pdf target is triggered after changes
> in svg (or sgml), this triggers the fo targets. Therefore it's not
> necessary to have svg (or sgml) dependencies at the fo level.

If $(ALLSGML) changes, then the .fo file needs to be rebuilt.  If
$(SVGSRC) changes, then the .pdf file needs to be rebuilt.  So those two
groups of files need to be handled differently.  The way you have
written it, a change in $(ALLSGML) will just rebuild the .pdf file from
a stale .fo.

>> The variable name SVGSRC is a bit confusing.  What is it the source of?
> The variable SVGSRC points to all svg-files, similar to ALLSGML which
> points to the sgml files. Whenever any of them changes, certain targets
> will fire.

Right, so maybe name it ALLSVG?

>>> @@ -209,7 +213,7 @@ check: postgres.sgml $(ALLSGML) check-tabs
>>>  install: install-html install-man
>>>
>>>  installdirs:
>>> - $(MKDIR_P) '$(DESTDIR)$(htmldir)'/html $(addprefix '$(DESTDIR)$(mandir)'/man, 1 3 $(sqlmansectnum))
>>> + $(MKDIR_P) '$(DESTDIR)$(htmldir)'/html/svg html/svg $(addprefix '$(DESTDIR)$(mandir)'/man, 1 3 $(sqlmansectnum))
>>>
>>>  # If the install used a man directory shared with other applications, this will remove all files.
>>>  uninstall:
>> html/svg is not an installation directory.  You need to create it
>> somewhere else.
> Please help. I haven't understood the distinction between installation
> directory and DESTDIR. On the other hand, in the Makefile there is a -
> redundant - command within the html-stamp target: $(MKDIR_P) html/svg.
> But this will run frequently, which is not necessary.

Right, so you probably don't need this change in the installdirs target
at all.

> There is a simple mechanism to create those list of figures: change line
> 55 of stylesheet-html-common.xsl to "book      toc,title,*figure*". But
> the result is ugly - see attached screenshot. The list is out-of-line.
> Additionally, in the future we will have many figures (and examples and
> tables). This will lead to similar problems we actually faced with the
> release notes.

Maybe we don't need a list of figures at all?  Let's just turn it off if
we don't like it.

> What is the problem here? Actually I don't have enough time to evaluate
> it in deep. If it is an urgent problem (I have seen that the
> commitfest-entry is tagged as "release 12") we shall shift the pg_dump
> figure to a later release.

The problem is that images don't show up in man pages, so we can't put
any images in there, or it will look silly.

> I think that the Backup chapter isn't a good place for this because it
> does not explain the complete interaction between pg_dump, psql and restore.

Then maybe it should explain it.  The pg_dump man page should be
strictly about pg_dump.  The Backup chapter should explain how it fits
together with the other tools.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services