Page MenuHomeFreeBSD

lang/python-doc-html: generate distfiles for all supported Python versions vs. all doc formats
ClosedPublic

Authored by leres on Feb 2 2019, 6:29 PM.

Details

Summary

Note: I will wait to commit until approved by koobs and at least
one of my mentors.

Compared to the patch in the PR, I added an "all" target to the
makefile the script outsputs so I could remove makesum from the
"${MAKE} -f $${t1}" command.

Proposed commit message:

Override the makesum target to run a script to update distfiles with
checksums for all supported Python versions vs. all doc formats.

While we're here update distfiles to match the order the script
outputs.

PR:		235169
Submitted by:	
Reported by:	
Reviewed by:	koobs (python, ports), ? (mentor)
Approved by:	koobs (python, ports), ? (mentor)
Differential Revision:	D19064

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

leres created this revision.Feb 2 2019, 6:29 PM
tobik added a subscriber: tobik.Feb 2 2019, 7:12 PM
tobik added inline comments.
lang/python-doc-html/Makefile
37–38 ↗(On Diff #53541)

make(1) supports reading the makefile from stdin, so I think this could be:

${FILESDIR}/gen-makesum.sh | ${MAKE} -f -
leres updated this revision to Diff 53549.Feb 2 2019, 8:43 PM

Integrate suggestion by tobik@ and have make read the temporary
makefile from stdin (thanks!) Also use ${FILESDIR}.

matthew added inline comments.Feb 3 2019, 7:30 AM
lang/python-doc-html/Makefile
35 ↗(On Diff #53549)

Hmmm.... the way I've seen this done before is to use something like:

.if make(makesum) || defined(FETCH_ALL)
   ...
.endif

to override variable settings when rebuilding distinfo. You should support FETCH_ALL as well here given the set of distfiles you'ld download is dependent on the selected options. This should allow you to put all the code into the main Makefile.

If you grep for make(makesum) you'll find plenty of examples.

leres updated this revision to Diff 53734.Feb 10 2019, 3:03 AM

I'd like all of the docformats to be in a variable and it seems like this could be used to simplify the DOCFORMAT/IGNORE block:

DOCFORMATS=   html pdf-a4 pdf-letter text

The problem is with formatting for the error message. I want commas between the items in DOCFORMATS. It's easy to add a comma after every format string but not every one except the last. And I couldn't figure out a "make" way to make a version so I punted and used a shell command:

PRETTY!=      ${ECHO} ${DOCFORMATS} | sed -e 's/ /, /g'

Is there a better way? It's wrapped with:

.if empty(DOCFORMATS:M${DOCFORMAT})
.endif

which only occurs if DOCFORMAT is set to something unreasonable so I think it's ok as per:

https://lists.freebsd.org/pipermail/freebsd-ports/2008-July/049777.html

Next I ran into the same problem that aimed me in the original direction of an external script; it's not very easy to loop over the versions in _PYTHON_VERSIONS. Maybe the problem is _PYTHON_VERSIONS isn't defined at the time my make(makesum) is running? This is a workaround:

_VERS!=               ${MAKE} -V _PYTHON_VERSIONS

but it seems like a hack. Again it's wrapped in a conditional so it only happens when when we're doing makesum (or fetch?)

I got stumped for a bit because MASTER_SITES changes depending on PYTHON_PORTVERSION but evetually figured out I could generate MASTER_SITES the same way I generate DISTFILES:

.if make(makesum)
DISTFILES!=     ${MAKE} all_distfiles
MASTER_SITES!=  ${MAKE} all_master_sites
.endif

More use of VAR!= but again I think this is ok.

Finally I don't understand FETCH_ALL. I see the defined(FETCH_ALL) reference you make. But I can't find anything that *sets* FETCH_ALL. How FETCH_ALL used? I don't get it.

leres marked 2 inline comments as done.Feb 10 2019, 3:06 AM
leres added inline comments.
lang/python-doc-html/Makefile
37–38 ↗(On Diff #53541)

Overcome by events

35 ↗(On Diff #53549)

See new version

matthew added a comment.EditedFeb 10 2019, 8:10 AM

Finally I don't understand FETCH_ALL. I see the defined(FETCH_ALL) reference you make. But I can't find anything that *sets* FETCH_ALL. How FETCH_ALL used? I don't get it.

Hmmm... you're right.

FETCH_ALL is a variable you can set from the command line if you need to fetch all of the distfiles for a port:

% make fetch FETCH_ALL=yes

There are only half-a-dozen ports that use it, so I guess it's probably old hat nowadays.

lucid-nonsense:/usr/ports:% grep -RE '\<FETCH_ALL\>' . | grep -v './.svn'
./mail/exim/Makefile:.if make(makesum) && !defined(FETCH_ALL)
./mail/exim/Makefile:.error "You forgot to define FETCH_ALL to create the sane distinfo"
./mail/exim/Makefile:.if ${PORT_OPTIONS:MSA_EXIM} || defined(FETCH_ALL)
./mail/exim-doc-html/Makefile:.if make(makesum) && !defined(FETCH_ALL)
./mail/exim-doc-html/Makefile:.error "You forgot to define FETCH_ALL to create the sane distinfo"
./mail/exim-doc-html/Makefile:.if defined(FETCH_ALL)
./textproc/sphinxsearch/Makefile:.if make(makesum) || defined(FETCH_ALL)
./textproc/sphinxsearch-devel/Makefile:.if make(makesum) || defined(FETCH_ALL)
./www/webalizer/Makefile:.if ${PORT_OPTIONS:MGEODB} || ${PORT_OPTIONS:MGEOIP} || make(makesum) || defined(FETCH_ALL)
./www/webalizer/Makefile:.if ${PORT_OPTIONS:MGEODB} || make(makesum) || defined(FETCH_ALL)
./www/opera/Makefile:.if defined(FETCH_ALL)
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MATOOLS}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MCOM}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MDLLC}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MDST}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MDATABASE} || ${PORT_OPTIONS:MDOTNET}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MCONTRIBUTED}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MARAGON}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MCOAST}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MDOME}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MJUN}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MSEASIDE}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MSILVERMARK}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MGEMSTONE}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MI18N}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MOBSOLETE}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MOPENTALK}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MPLUGIN}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MPREVIEW}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MSTORE}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MVMAIX}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MVMHPUX}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MVMMAC}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MVMSGI}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MVMSOLARIS}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MVMWINDOWS}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MVMLINUX}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MWEBAPP}
./lang/visualworks/Makefile:.if make(makesum) || defined(FETCH_ALL) || ${PORT_OPTIONS:MWEBSERVICES}

This whole topic doesn't appear to be mentioned in the PH. Looks like a small projectette to improve the ports documentation...

This comment was removed by matthew.
matthew added inline comments.Feb 10 2019, 8:52 AM
lang/python-doc-html/Makefile
29 ↗(On Diff #53734)

See paste(1) for a shellish way of turning a sequence of lines into a comma separated list:

$ paste -s -d , - <<EOF
> foo
> bar
> baz
> blurfl
> EOF
foo,bar,baz,blurfl

The Makefile-ish way of doing this is to use the :ts modifier:

lucid-nonsense:/tmp:% cat Makefile

STUFF= foo bar baz blurfl

all:
	@echo ${STUFF:ts,}
lucid-nonsense:/tmp:% make
foo,bar,baz,blurfl
matthew added inline comments.Feb 10 2019, 9:18 AM
lang/python-doc-html/Makefile
50 ↗(On Diff #53734)

I don't think you need the loop over ${DOCFORMATS} when generating all_master_sites -- the results are very repetetive:

lucid-nonsense:~...ports/lang/python-doc-html:% make makesum -V MASTER_SITES
https://www.python.org/ftp/python/doc/2.7.15/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/2.7.15/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/2.7.15/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/2.7.15/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.6.8/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.6.8/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.6.8/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.6.8/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.7.2/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.7.2/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.7.2/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.7.2/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.5.6/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.5.6/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.5.6/ https://www.python.org/ftp/python/doc/current/ https://www.python.org/ftp/python/doc/3.5.6/ https://www.python.org/ftp/python/doc/current/

Just some thoughts, which I'm not insisting you implement (but you can if you want):

a) This port is the master port for the four python-doc ports:

lang/python-doc-html
lang/python-doc-pdf-a4
lang/python-doc-pdf-letter
lang/python-doc-text

where you're going to a lot of trouble to create a distinfo file in lang/python-doc-html with the data for all the distfiles for the other three document format variants as well.
However, you could just add a distinfo file to each of the slave ports with the distinfo for just that docformat which would should simplify the logic a bit. (compare to what the databases/postgresql??-{client,server} ports do)

That does mean running 'make makesum' four times rather than just once if any of the upstream python versions change.

b) Ideally instead of all the master/slave business this would be a single highly flavoured 'lang/python-doc' port. It seems to be the textbook example of where we'd like to have a port with double flavouring - both python version and document format. I tried to make double flavouring work for django+python but I never could make it independent of the order in which the USES were processed. However given that the document format part is entirely local to this port, it should be possible here.

leres updated this revision to Diff 53753.Feb 11 2019, 12:32 AM
leres marked 2 inline comments as done.

The Makefile-ish way of doing this is to use the :ts modifier:

Hah! How did I miss that? Thanks, that works nicely.

Is there a way to get numeric value of a word in a variable? For example

ALL_PYTHON_VERSIONS=	2.7.15 3.6.8 3.7.2 3.5.6

if I .for loop over them is there something that expands to 1, 2, 3, or 4? (It would be handy for tagging the DISTFILES and MASTER_SITES.)

I don't think you need the loop over ${DOCFORMATS} when generating all_master_sites -- the results are very repetetive:

And not even correct since, for example, you're not going to find the 3.6.8 archives in the 2.7.15 directory. This new version ties each group of DISTFILES to the correct MASTER_SITES.

Just some thoughts, which I'm not insisting you implement (but you can if you want):

a) [...] you could just add a distinfo file to each of the slave ports with the distinfo for just that docformat

I don't like this. I think it's much nicer (and less work for the maintainer) for the master port to just do the right thing when you make makesum.

b) [...] double flavouring

Would it be ok continue down the current path and commit a working solution and then revisit double flavouring? I know I'll certainly learn more that way.

And I am interested in learning more about flavors. For example I opened PR 226618 last year because I'd like to see two flavors of subversion, regular and FreeBSD which add stuff to the commit template. In my environment I only want/need the FreeBSD template stuff on my main dev poudriere box. Elsewhere it just confuses people.

matthew added a comment.EditedFeb 11 2019, 6:45 AM

Is there a way to get numeric value of a word in a variable? For example

ALL_PYTHON_VERSIONS=	2.7.15 3.6.8 3.7.2 3.5.6

if I .for loop over them is there something that expands to 1, 2, 3, or 4? (It would be handy for tagging the DISTFILES and MASTER_SITES.)

You mean treating them as an array? Unfortunately I don't think so.

Just some thoughts, which I'm not insisting you implement (but you can if you want):
a) [...] you could just add a distinfo file to each of the slave ports with the distinfo for just that docformat

I don't like this. I think it's much nicer (and less work for the maintainer) for the master port to just do the right thing when you make makesum.

OK. I'm happy to abide by your judgement on the matter.

b) [...] double flavouring

Would it be ok continue down the current path and commit a working solution and then revisit double flavouring? I know I'll certainly learn more that way.
And I am interested in learning more about flavors. For example I opened PR 226618 last year because I'd like to see two flavors of subversion, regular and FreeBSD which add stuff to the commit template. In my environment I only want/need the FreeBSD template stuff on my main dev poudriere box. Elsewhere it just confuses people.

Absolutely. Go for it. Although making a doubly-flavoured port really is jumping in at the deep end...

matthew added inline comments.Feb 11 2019, 7:12 AM
lang/python-doc-html/Makefile
52 ↗(On Diff #53753)

Ref. your question about array indexing above -- you could just use the value of ${v} as the distfile group tag here.

leres updated this revision to Diff 53792.Feb 11 2019, 5:35 PM

Ref. your question about array indexing above -- you could just use the value of ${v} as the distfile group tag here.

That's pretty awesome. The group tag must be alpha-numeric but otherwise this works great and really cleans things up.

This revision is now accepted and ready to land.Feb 13 2019, 8:29 PM
koobs accepted this revision.Feb 15 2019, 1:03 AM
koobs edited the summary of this revision. (Show Details)

Looks good, happy for you to proceed if you're comfortable/confident with the changeset.

Thanks Craig!

This revision was automatically updated to reflect the committed changes.