Page MenuHomeFreeBSD

Framework: Fix `make describe` for SUBPACKAGES case
Needs ReviewPublic

Authored by arrowd on Feb 8 2024, 12:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Feb 26, 4:30 PM
Unknown Object (File)
Tue, Feb 25, 5:20 PM
Unknown Object (File)
Jan 26 2025, 5:57 PM
Unknown Object (File)
Jan 24 2025, 5:27 PM
Unknown Object (File)
Jan 23 2025, 6:41 PM
Unknown Object (File)
Jan 22 2025, 8:31 PM
Unknown Object (File)
Jan 6 2025, 10:28 AM
Unknown Object (File)
Dec 4 2024, 9:49 AM

Details

Reviewers
pizzamig
Group Reviewers
portmgr
Summary

The main idea behind the change is to remove ${deptype}_DEPENDS_ALL variables. Instead, all deps up to BUILD ones are essentially shared, while LIB, RUN and TEST deps are directly taken from the corresponding .${sp} ones. It is up to port writer to add ${LIB_DEPENDS} to LIB_DEPENDS.${sp}.

Test Plan

poud testport on audio/alsa-plugins (a port with subpackages) and sysutils/44bsd-more (a port without subpackages). The make describe output for the latter did not change.

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 55863
Build 52752: arc lint + arc unit

Event Timeline

arrowd created this revision.
pizzamig requested changes to this revision.Feb 8 2024, 2:54 PM
pizzamig added a subscriber: pizzamig.

In general, it should works, but the *DEPENDS_ALL needs to stay (even if not used by the framework) as they are used externally (by poudriere).

Mk/bsd.port.mk
2675

the *_DEPENDS_ALL you are removing are the one used by poudriere to fetch dependency of the port before building it.
For this reason, they cannot be removed until poudriere is not patched to use something else

4254

I would add, as a comment, that _DEPENDS_ALL_SUBPACKAGES is only defined for RUN.
Maybe it makes sense to check for RUN_DEPENDS_ALL_SUBPACKAGES separately

4497

While equivalent, I would keep the for loop in the else branch of empty(FLAVORS)

This revision now requires changes to proceed.Feb 8 2024, 2:54 PM
Mk/bsd.port.mk
2675

The ALL variables were introduced just recently with your https://reviews.freebsd.org/D40549
You mean Poudriere picked these up already?

4254

_DEPENDS_ALL_SUBPACKAGES are defined for RUN, TEST and LIB, but only RUN is used here.

Maybe it makes sense to check for RUN_DEPENDS_ALL_SUBPACKAGES separately

That'd mean more lines of code. But I will do that if you insist.

Mk/bsd.port.mk
2675

The ALL variables are used by poudriere to support subpackages.

4254

No strong opinion, but it seems uneven to me.
In any case, I won't block the merge because of this, tho.

The follow-up commit for Poudriere, if I get it right: https://github.com/freebsd/poudriere/commit/ee34405ae8e7d1763f2cc70e1f179b8c643de1e9

Didn't test it runtime, though.

arrowd marked an inline comment as done.
  • Address some comments
Mk/bsd.port.mk
4254

Maybe it makes sense to check for RUN_DEPENDS_ALL_SUBPACKAGES separately

I took another look at that and don't quite understand what do you mean by checking separately? You mean plugging in another .if defined(${deptype}_DEPENDS_ALL_SUBPACKAGES) check?

arrowd marked 2 inline comments as done.
  • Rebase
  • Add compatibility shim for Poudriere

To test this one can run git revert 7dd904d9178aef2ac7369c77551e6c7774997564
to bring audio/alsa-plugins into the subpackagized state.

Running make -V {BUILD,LIB,RUN}_DEPENDS_ALL now produce the same list as before
modulo some extra duplicated entries, which were present in the previous version anyways.

arrowd retitled this revision from Framework: Fix `make describe for SUBPACKAGES case to Framework: Fix `make describe` for SUBPACKAGES case.Thu, Feb 27, 6:54 AM