Bug 249537 - Mk/bsd.port.mk: unbreak makesum for lang/python-doc-html
Summary: Mk/bsd.port.mk: unbreak makesum for lang/python-doc-html
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Tobias Kortkamp
URL: https://reviews.freebsd.org/D27226
Keywords: patch
Depends on:
Blocks: 249063
  Show dependency treegraph
 
Reported: 2020-09-22 21:56 UTC by Craig Leres
Modified: 2021-11-02 11:58 UTC (History)
4 users (show)

See Also:
tobik: maintainer-feedback-


Attachments
patch (525 bytes, patch)
2020-09-22 21:56 UTC, Craig Leres
leres: maintainer-approval?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Craig Leres freebsd_committer 2020-09-22 21:56:23 UTC
@dbaio reported that "make makesum" was not working for lang/python-doc-html. This was something I had fixed last year via r492965.

r513191 does some cleanup of bsd.prog.mk:

    Reduce code duplication by calling fetch target
 
    when converting the do-fetch target to proper scripting we lost
    the ability to overwrite do-fetch when running make makesum.
    as reported here: 
    https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=215530
    
    Let's call again do-fetch directly instead of duplicating its
    content

This was a nice cleanup but broke makesum lang/python-doc-html and also some linux ports resulting in r514097:

    Let "make makesum" pass DISTFILES to "make fetch".  For Linux 
    ports "make makesum" downloads distfiles for all supported
    architectures while "make fetch" only downloads files for the
    build architecture.

This partially fixed lang/python-doc-html but MASTER_SITES also needs to be passed down to the sub-make; here's a patch to do that.
Comment 1 Craig Leres freebsd_committer 2020-09-22 21:56:50 UTC
Created attachment 218195 [details]
patch
Comment 2 Mathieu Arnold freebsd_committer 2020-11-16 10:06:46 UTC
It seems to me that the framework does not really need fixing, the port should be fixed instead.
Nothing prevents the port from having the correct MASTER_SITES value outside of make(makesum).
Comment 3 Danilo G. Baio freebsd_committer 2021-05-12 01:11:10 UTC
ports 5693283b37e182d63579863491412355997533f9
Comment 4 Mathieu Arnold freebsd_committer 2021-05-17 08:03:02 UTC
I don't think the fix is correct at all, you should absolutely never need to use .export.
Comment 5 Tobias Kortkamp freebsd_committer 2021-05-17 17:05:00 UTC
I also needed to manually export MASTER_SITES for makesum.  I don't
understand why the framework does not export MASTER_SITES to the
submake on its own.  It exports DISTFILES for some reason since
ports 50d2c82e016fd176868cdc6e4befa606fa61c50e but not MASTER_SITES?
This looks like an oversight to me.

I know we could generate or set MASTER_SITES statically instead of
conditionally but that isn't exactly free.  It would be pretty
awful to do this in lang/rust-bootstrap which currently has nested
conditional variables for each FLAVOR.  It is already complicated
enough.

Is there any technical reason to not just commit this one line
patch?  If not please approve it.
Comment 6 Craig Leres freebsd_committer 2021-05-17 17:15:06 UTC
(In reply to Tobias Kortkamp from comment #5)
>  It exports DISTFILES for some reason since ports 50d2c82e016fd176868cdc6e4befa606fa61c50e but not MASTER_SITES?

I believe it's the case that DISTFILES and MASTER_SITES are co-dependent and should receive the same treatment.
Comment 7 Danilo G. Baio freebsd_committer 2021-05-17 22:26:57 UTC
here is all context: ports r513191 and ports 50d2c82e016fd176868cdc6e4befa606fa61c50e

I can revert the export, but we will probably need to do that locally as well. Any other idea?

Regards.
Comment 8 Tobias Kortkamp freebsd_committer 2021-10-06 13:48:19 UTC
Ping about that approval request from comment #5.
Comment 9 Tobias Kortkamp freebsd_committer 2021-10-25 09:08:04 UTC
Ok, this is one last ping as a courtesy.

But enough is enough.  It is my intention to commit Craig's patch on 2021-11-01
if I do not hear back from anyone and I will view that as implicit approval.

And again: Is there any downside to explicitly passing MASTER_SITES to the
submake as Craig suggested? Just wondering if I am missing something.
Comment 10 Craig Leres freebsd_committer 2021-10-25 16:49:48 UTC
I maintain that DISTFILES and MASTER_SITES should share the same fate; any port with "Multiple Distribution or Patches Files from Multiple Locations" (see section 5.4.9 of the porters handbook) will always want DISTFILES and MASTER_SITES to be treated the same.

I would argue that either both variables or neither should be passed to "make fetch".

From the porters handbook:
> Example 22. Simplified Use of MASTER_SITES:n with One File Per Site
> 
> MASTER_SITES=	ftp://ftp1.example.com/:source1 \
>		http://www.example.com/:source2
> DISTFILES=	source1.tar.gz:source1 \
>		source2.tar.gz:source2
Comment 11 commit-hook freebsd_committer 2021-11-01 19:10:52 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=46d53485d0bab3225dab27bac82ec6a382478626

commit 46d53485d0bab3225dab27bac82ec6a382478626
Author:     Tobias Kortkamp <tobik@FreeBSD.org>
AuthorDate: 2021-10-13 11:59:48 +0000
Commit:     Tobias Kortkamp <tobik@FreeBSD.org>
CommitDate: 2021-11-01 19:01:09 +0000

    devel/freebsd-sysroot: Drop MASTER_SITES workaround

    The bug that made that necessary has been squashed.

    PR:             249537

 devel/freebsd-sysroot/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 12 commit-hook freebsd_committer 2021-11-01 19:10:53 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=8d07ec826a7af0500b5982aed7327fa3f7efeca8

commit 8d07ec826a7af0500b5982aed7327fa3f7efeca8
Author:     Craig Leres <leres@FreeBSD.org>
AuthorDate: 2021-10-13 08:24:09 +0000
Commit:     Tobias Kortkamp <tobik@FreeBSD.org>
CommitDate: 2021-11-01 19:01:08 +0000

    Mk/bsd.port.mk: Fix makesum for ports that override MASTER_SITES in make(makesum)

    fba040e62bff04323a29d5ad2e21f516b18e9bb4 broke makesum for ports
    that conditionally override MASTER_SITES in a make(makesum)
    block to simplify maintenance like lang/python-doc-html or
    devel/freebsd-sysroot where it is impractical to just statically
    list all possible MASTER_SITES. It also broke USES=linux ports
    that do the same for DISTFILES.

    The breakage with DISTFILES was fixed in
    50d2c82e016fd176868cdc6e4befa606fa61c50e by explicitly passing
    it to the sub-make fetch. Do the same with MASTER_SITES and
    PATCH_SITES so we do not have to workaround this with .MAKEFLAGS
    or .export.

    PR:             249537
    Approved by:    portmgr (implicit)

 Mk/bsd.port.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)