Rethink %SUBDIR% for Xfce's MASTER_SITES
AcceptedPublic

Authored by olivierd on Nov 1 2016, 7:13 PM.

Details

Reviewers
ehaupt
mat
Summary

This patch simplify MASTER_SITES in each Xfce ports. Users can only override the default component (xfce), possible values are:

  • apps
  • art
  • bindings
  • thunar-plugins
  • panel-plugins
  • archive

'xfce' contains core components. Test is needed, for ports where ${PORTNAME} differ from official name.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
olivierd retitled this revision from to Rethink %SUBDIR% for Xfce's MASTER_SITES.Nov 1 2016, 7:13 PM
olivierd updated this object.
olivierd edited the test plan for this revision. (Show Details)
olivierd added a reviewer: mat.
olivierd added subscribers: xfce, ehaupt.
ehaupt accepted this revision.Nov 2 2016, 8:36 AM
ehaupt added a reviewer: ehaupt.

Looks good to me.

This revision is now accepted and ready to land.Nov 2 2016, 8:36 AM
mat added inline comments.Nov 2 2016, 11:58 AM
Mk/bsd.sites.mk
1240–1245

I don't understand the difference in the if and else here.

if DISTNAME without DISTVERSION is PORTNAME, then use a lowercased version of PORTNAME, otherwise, use DISTNAME without DISTVERSION.
I had a quick check, I could not find any port in this diff where both are not the same thing.

Lowercase is for x11-fm/thunar, package name (and tarball) is spelled Thunar, but it is located in src/xfce/thunar/ directory.

Check is required, because I noticed, when ${DISTNAME} is defined in Makefile, path is well formed, we can fetch tarball.

For example x11/libexo

make fetch
===>  License GPLv2 LGPL21 accepted by the user
===>  Found saved configuration for libexo-0.11.2
===>   libexo-0.11.2 depends on file: /usr/local/sbin/pkg - found
=> exo-0.11.2.tar.bz2 doesn't seem to exist in /usr/ports/distfiles/xfce4.
=> Attempting to fetch http://archive.xfce.org/src/xfce/exo/0.11/exo-0.11.2.tar.bz2
exo-0.11.2.tar.bz2                            100% of 1282 kB 1591 kBps 00m01s
===> Fetching all distfiles required by libexo-0.11.2 for building

But, when ${DISTNAME} is not defined in Makefile, it does not work (e.g. x11/xfce4-terminal):

make fetch
===>  License GPLv2 accepted by the user
===>   xfce4-terminal-0.8.1 depends on file: /usr/local/sbin/pkg - found
=> xfce4-terminal-0.8.1.tar.bz2 doesn't seem to exist in /usr/ports/distfiles/xfce4.
=> Attempting to fetch http://archive.xfce.org/src/apps/xfce4-terminal-0.8.1/0.8/xfce4-terminal-0.8.1.tar.bz2
fetch: http://archive.xfce.org/src/apps/xfce4-terminal-0.8.1/0.8/xfce4-terminal-0.8.1.tar.bz2: Not Found

Path should be src/apps/xfce4-terminal/0.8/ not src/apps/xfce4-terminal-0.8.1/0.8/

mat added a comment.Nov 3 2016, 3:32 PM

Lowercase is for x11-fm/thunar, package name (and tarball) is spelled Thunar, but it is located in src/xfce/thunar/ directory.

If there are no cases of the directory being uppercased, then it could always be based on DISTNAME with :tl, so it is a non issue.

Check is required, because I noticed, when ${DISTNAME} is defined in Makefile, path is well formed, we can fetch tarball.

For example x11/libexo

make fetch
===>  License GPLv2 LGPL21 accepted by the user
===>  Found saved configuration for libexo-0.11.2
===>   libexo-0.11.2 depends on file: /usr/local/sbin/pkg - found
=> exo-0.11.2.tar.bz2 doesn't seem to exist in /usr/ports/distfiles/xfce4.
=> Attempting to fetch http://archive.xfce.org/src/xfce/exo/0.11/exo-0.11.2.tar.bz2
exo-0.11.2.tar.bz2                            100% of 1282 kB 1591 kBps 00m01s
===> Fetching all distfiles required by libexo-0.11.2 for building

But, when ${DISTNAME} is not defined in Makefile, it does not work (e.g. x11/xfce4-terminal):

make fetch
===>  License GPLv2 accepted by the user
===>   xfce4-terminal-0.8.1 depends on file: /usr/local/sbin/pkg - found
=> xfce4-terminal-0.8.1.tar.bz2 doesn't seem to exist in /usr/ports/distfiles/xfce4.
=> Attempting to fetch http://archive.xfce.org/src/apps/xfce4-terminal-0.8.1/0.8/xfce4-terminal-0.8.1.tar.bz2
fetch: http://archive.xfce.org/src/apps/xfce4-terminal-0.8.1/0.8/xfce4-terminal-0.8.1.tar.bz2: Not Found

Path should be src/apps/xfce4-terminal/0.8/ not src/apps/xfce4-terminal-0.8.1/0.8/

Right. There is some lazy evaluation problem. This seems to work for me:

_PATH=        ${DISTNAME:S/-${DISTVERSIONFULL}//:tl}/${DISTVERSION:C/^([0-9]+\.[0-9]+).*/\1/}

(Also, can _PATH be renamed to, say, _XFCE_PATH or something, to make it less generic.)

In D8416#175494, @mat wrote:

Lowercase is for x11-fm/thunar, package name (and tarball) is spelled Thunar, but it is located in src/xfce/thunar/ directory.

If there are no cases of the directory being uppercased, then it could always be based on DISTNAME with :tl, so it is a non issue.

Check is required, because I noticed, when ${DISTNAME} is defined in Makefile, path is well formed, we can fetch tarball.

For example x11/libexo

make fetch
===>  License GPLv2 LGPL21 accepted by the user
===>  Found saved configuration for libexo-0.11.2
===>   libexo-0.11.2 depends on file: /usr/local/sbin/pkg - found
=> exo-0.11.2.tar.bz2 doesn't seem to exist in /usr/ports/distfiles/xfce4.
=> Attempting to fetch http://archive.xfce.org/src/xfce/exo/0.11/exo-0.11.2.tar.bz2
exo-0.11.2.tar.bz2                            100% of 1282 kB 1591 kBps 00m01s
===> Fetching all distfiles required by libexo-0.11.2 for building

But, when ${DISTNAME} is not defined in Makefile, it does not work (e.g. x11/xfce4-terminal):

make fetch
===>  License GPLv2 accepted by the user
===>   xfce4-terminal-0.8.1 depends on file: /usr/local/sbin/pkg - found
=> xfce4-terminal-0.8.1.tar.bz2 doesn't seem to exist in /usr/ports/distfiles/xfce4.
=> Attempting to fetch http://archive.xfce.org/src/apps/xfce4-terminal-0.8.1/0.8/xfce4-terminal-0.8.1.tar.bz2
fetch: http://archive.xfce.org/src/apps/xfce4-terminal-0.8.1/0.8/xfce4-terminal-0.8.1.tar.bz2: Not Found

Path should be src/apps/xfce4-terminal/0.8/ not src/apps/xfce4-terminal-0.8.1/0.8/

Right. There is some lazy evaluation problem. This seems to work for me:

_PATH=        ${DISTNAME:S/-${DISTVERSIONFULL}//:tl}/${DISTVERSION:C/^([0-9]+\.[0-9]+).*/\1/}

(Also, can _PATH be renamed to, say, _XFCE_PATH or something, to make it less generic.)

I tried your suggestion, but it fails with some ports.
For failing ports, if I replace ${DISTVERSIONFULL} by ${DISTVERSION} everything is fine, except for ports which were functional at first.

  • x11/xfce4-terminal (not working with ${DISTVERSION}, success with ${DISTVERSIONFULL})
  • x11/libexo (not working with ${DISTVERSIONFULL}, success with ${DISTVERSION})
  • misc/xfce4-wm-themes (not working with ${DISTVERSIONFULL}, success with ${DISTVERSION})
  • x11-fm/thunar (not working with ${DISTVERSION}, success with ${DISTVERSIONFULL})
mat added a comment.Nov 4 2016, 9:51 AM
  • x11/xfce4-terminal (not working with ${DISTVERSION}, success with ${DISTVERSIONFULL})
  • x11/libexo (not working with ${DISTVERSIONFULL}, success with ${DISTVERSION})
  • misc/xfce4-wm-themes (not working with ${DISTVERSIONFULL}, success with ${DISTVERSION})
  • x11-fm/thunar (not working with ${DISTVERSION}, success with ${DISTVERSIONFULL})

Ok, those can also be fixed.

The problem is the way DISTNAME is created.

By default, it is ${PORTNAME}-${DISTVERSIONFULL}. If you ask to match -${DISTVERSIONFULL} to do something, it does not have to be expanded, it can simply use the -${DISTVERSIONFULL} and be done with it.

If you set DISTNAME to foo-${PORTVERSION} and ask to match -${DISTVERSIONFULL} it needs to expand both sides. At that point, I figure DISTVERSIONFULL is not yet usable, and it so, it does not do what you want. On the other hand, if you change to foo-$(DISTVERSIONFULL} (which is the same, from your point of view) it does not need to expand it to match -${DISTVERSIONFULL} and it works.

So, for those two the following patch solves the problem:

--- misc/xfce4-wm-themes/Makefile
+++ misc/xfce4-wm-themes/Makefile
@@ -9 +9 @@ MASTER_SITES=       XFCE/art
-DISTNAME=      xfwm4-themes-${PORTVERSION}
+DISTNAME=      xfwm4-themes-${DISTVERSIONFULL}
--- x11/libexo/Makefile
+++ x11/libexo/Makefile
@@ -8 +8 @@ MASTER_SITES=       XFCE
-DISTNAME=      exo-${PORTVERSION}
+DISTNAME=      exo-${DISTVERSIONFULL}

Adding a comment in bsd.sites.mk in the XFCE block saying that DISTNAME must use DISTVERSIONFULL would be enough to steer people in the right way.

(Everything you definitively did not want to know about how make(1) variables work under the hood)