Define KDE_DIST instead of MASTER_SITES and DIST_SUBDIR in every KDE port

[tcberner]

Summary

Add KDE_DIST=foo similar to the Qt ports QT_DIST.

At the moment this supports:

• kde4
• kactivities
• applications:legacy
• calligra, calligra:l10n

In the future, with KDE Framworks, Plasma and (up to date) Applications ports
this will be extended to include

• frameworks, framworks:portingaid
• plasma
• applications

I chose to add it directly below the USE_KDE=foo entry of the ports.

I also opted to move the distfiles of the kde4 stuff into

• ${DISTDIR}/KDE/kde4/<version>

from ${DISTDIR}/KDE/<version>, as I intend to do

• ${DISTDIR}/KDE/frameworks/<version>
• ${DISTIDR}/KDE/plasma/<version>
• ${DISTDIR}/KDE/applications/<version>

for the future options -- so this change is mainly done for my OCD.

[mat]

New variables should be used at a minimum, for example, if:

USES=kde:4
KDE_DIST=kde4

could be folded into:

USES=kde:4,dist

it would be great.

[mat]

Also, all the moves in DISTFILES don't server real purpose, it will invalidate all the distfiles downloaded by people, and, most importantly, the FreeBSD distcache.

[tcberner]

How about using USES=kde:4,dist=foo and USES=kde:5,dist=bar this way, it would also be easier to add further arguments to kde.mk should the need ever arise -- instead of USES=kde:4,foo and USES=kde:5,bar?

[mat]

In D7645#163550, @tcberner wrote:

How about using USES=kde:4,dist=foo and USES=kde:5,dist=bar this way, it would also be easier to add further arguments to kde.mk should the need ever arise -- instead of USES=kde:4,foo and USES=kde:5,bar?

Good idea :-)

[tcberner]

• Instead of defining KDE_DIST=foo, pass foo to kde.mk via kde:4,dist=foo
• Do not move distfiles, even if it breaks your heart :P

[mat]

Thinking back about it, I wonder if using an = sign is a good idea, it may confuse things. (not saying it is bad, just thinking out loud)

Also, I think using a : in the args is a bad idea, the : should only be used once, separating the uses from its args. You could easily replace it with an underscore as you are not trying to write sentences but simple words.

Also wondering, if dist=application:legacy is the only valid version of dist=application:*, why not drop the legacy part, or merge lines 90/91 into ​. if ${_KDE_DIST:Mapplications\:legacy} so that using application:foo is caught as being bad. (See, using a colon is bad, you have to escape it when doing :M)

[tcberner]

Yes, it looks a bit kludgey...
As mentioned in the beginning, once we have KF5 imported there will be dist=applications for the up-to-date version of KDE applications, the :legacy versions can then hopefully be dropped after some time...

Instead of using applications:legacy how about the good old applications.legacy?

Maybe we could also do the magic without using dist or KDE_DIST.
kde.mk could maybe do it itself, by comparing ${PORTVERSION} with the ones it knows about in case MASTER_SITES and DIST_SUBDIR is not set in the ports Makefile?
That would certainly make the Makefiles cleaner again to read :)

[mat]

In D7645#163595, @tcberner wrote:

Yes, it looks a bit kludgey...
As mentioned in the beginning, once we have KF5 imported there will be dist=applications for the up-to-date version of KDE applications, the :legacy versions can then hopefully be dropped after some time...

Instead of using applications:legacy how about the good old applications.legacy?

Sure, that works too, and avoids the : :-)

Maybe we could also do the magic without using dist or KDE_DIST.
kde.mk could maybe do it itself, by comparing ${PORTVERSION} with the ones it knows about in case MASTER_SITES and DIST_SUBDIR is not set in the ports Makefile?
That would certainly make the Makefiles cleaner again to read :)

Ah, well, if you can make things automatic, it is even better :-)

[tcberner]

Trying it out, I don't think the magic will work very nicely :/ -- just having the version to decide what the port is works fine, unless there are exceptions...

I think sticking with passing an argument to kde.mk is probably the cleanest way. I'll go and think a little about how to name it, so it's not as
kludgey -- maybe just passing it directly like you initially suggested is the best way after all :)

[tcberner]

And an other try :)

• Get rid again of the dist=, and simply pass the distname as argument.
• Do not overcomplicate it, and instead of splitting the distarg, just use it as is, i.e. no longer calligra and calligra:l10n but simply calligra_l10n for the latter.

[tcberner]

Correct the IGNORE message, when an invalid KDE_DIST is passed

[tcberner]

Other stupid idea: Add additional virtual CATEGORIES, to the existing kde: kde-frameworks, kde-plasma, kde-applications.
And only apply this to the new ports for applications-16.*, kf5-* and plasma5*-.

CATEGORIES= foo kde

becomse

CATEGORIES= foo kde-frameworks

[mat]

In D7645#164753, @tcberner wrote:

Other stupid idea: Add additional virtual CATEGORIES, to the existing kde: kde-frameworks, kde-plasma, kde-applications.
And only apply this to the new ports for applications-16.*, kf5-* and plasma5*-.

CATEGORIES= foo kde

becomse

CATEGORIES= foo kde-frameworks

That sounds a good idea, maybe foo kde kde-frameworks

[tcberner]

Cool :) I'll update the diff tomorrow to use this.

[tcberner]

Add new virtual category kde-kde4 .

If MASTER_SITES or DIST_SUBDIR is not set, kde.mk will fill now in the blanks
depending on the kde-foo CATEGORY present. At the moment this is only kde-kde4.

With the import of the new KDE ports the following additional categories would be added:

• kde-frameworks: ~ 70 ports
• kde-plasma: ~ 40 ports
• kde-applications: ~200 ports

[mat]

Mk/Uses/kde.mk

87

I don't think :O:u are needed here, the variable is literally defined on the previous line and can be kept clean. If it needs to be sorted, maybe just add a comment about it, as the unique thing, well, eyes should be enough to figure that out before committing a change 😄

Mk/bsd.port.mk

2442

I wonder if ${_KDE_CATEGORIES_SUPPORTED} could not be added here instead.

[tcberner]

I'll give the bsd.port.mk change a go :) -- but there seems to be an issue with metaports, as they now try to fetch a distfile. However, I cannot check for empty(DISTFILES) in kde.mk it seems.

[tcberner]

• Slightly simplify the code in kde.mk: the outer if to check for empty MASTER_SITES or DIST_SUBDIR is not really necessary, and we can just check for the category.
• Use ${_KDE_CATEGORIES_SUPPORTED} in bsd.port.mk

[tcberner]

mark stuff done

[tcberner]

Actually remove :O:u .from the for-loop :D -- does not need to be sorted at all.

[rakuco]

The CATEGORIES approach looks unusual, but seems to work well. If mat is OK with it, so am I. I guess there's no need to change any of the distinfo files, which would reduce the size of this diff quite a bit.

Mk/Uses/kde.mk

102

Is this comment still relevant? I don't see anything Calligra-specific below.

editors/calligra-l10n/files/bsd.l10n.mk

9

This looks like a leftover from a previous iteration of this patch.

[tcberner]

Adress issues mentioned by Raphael:

• Remove the distinfo changes
• Remove old entry in calligra's bsd.l10n.mk
• Remove calligra comment from kde.mk

[tcberner]

mark stuff done

[rakuco]

There are a few things you should do before landing:

• Update the description of this review request, as things have changed significantly compared to the original idea.
• Ask for an exp-run to see if nothing broke; if possible, the exp-run shouldn't use FreeBSD's distcache so we can verify no port was broken by this change.

[mat]

Looks in great shape, can't wait for the exp-run :-)

[tcberner]

Committed: https://svnweb.freebsd.org/changeset/ports/424182
[somehow it didn't close it automatically].

[mat]

The Differential Revision line was not the last line.