Page MenuHomeFreeBSD

x11/libfm, x11-fm/pcmanfm: Update and FLAVORIZE both.
ClosedPublic

Authored by rigoletto on Jul 22 2018, 2:34 AM.

Details

Summary

PR: 229944

  • update x11/libfm to version 1.3.1
  • update x11-fm/pcmanfm to version 1.3.1
  • FLAVORIZE both.

Also touch the x11/libfm-extra pkg-plist file, that is a libfm SLAVEPORT.

Ps. since these are GTK related ports I took the liberty to add @kwm as reviewer.

Thanks!

Test Plan

Apparently nothing on x11/lxde-meta is broken because all ports in there that depends on libfm depends on the GTK2 version what is the default FLAVOR.

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

rigoletto created this revision.Jul 22 2018, 2:34 AM
rigoletto edited the summary of this revision. (Show Details)Jul 22 2018, 2:45 AM
rigoletto edited the test plan for this revision. (Show Details)
rigoletto edited the summary of this revision. (Show Details)Jul 22 2018, 2:49 AM
rigoletto edited the summary of this revision. (Show Details)Jul 22 2018, 2:55 AM

Why not make an extra flavor too?

mat added inline comments.Jul 22 2018, 9:17 AM
x11-fm/pcmanfm/Makefile
8 ↗(On Diff #45669)

Please use the <flavor>_PKGNAMESUFFIX helpers for that, do not reuse FLAVOR in it, and try not to change the default flavor's package name so that people running pkg upgrade will not wonder wtf happened again.

21–22 ↗(On Diff #45669)

FLAVOR is the flavor of the local port, it cannot be transitive.
Please use litteral @gtk2 and @gtk3.

36–38 ↗(On Diff #45669)

Chaining both .ifs with .else/.elif would make parsing faster.

x11/libfm/Makefile
8 ↗(On Diff #45669)

Do not change the default's flavor package name, so that running pkg upgrade works.

30 ↗(On Diff #45669)

You should probably use the current pkg-plist file for the gtk2 flavor, and remove this line.

rigoletto marked 5 inline comments as done.Jul 22 2018, 9:26 PM
rigoletto updated this revision to Diff 45702.

Done.

Converting libfm-extra to FLAVOR (libfm@extra) resulted on:

Warning: (x11/libfm@gtk3): Error: x11/libfm incorrectly depends on itself.

Thanks! ^^

mat added a subscriber: bdrewery.Jul 23 2018, 1:19 PM

It seems you forgot to add revert the pkg-plist deletion.

In D16387#348109, @lbdm_privacychain.ch wrote:

Done.
Converting libfm-extra to FLAVOR (libfm@extra) resulted on:
Warning: (x11/libfm@gtk3): Error: x11/libfm incorrectly depends on itself.
Thanks! ^^

This does not look like a ports framework message, but a poudriere one. Poke @bdrewery.

mat added a comment.Jul 23 2018, 6:43 PM

I was wondering about the two plists. the only difference is libfm-gtk3.so* and the libfm-gtk3.pc, it would probably be better to add this:

.if ${FLAVOR} = gtk2
PLIST_SUB= GTK3=
.else
PLIST_SUB= GTK3=3
.endif

and write libfm-gtk%%GTK3%%.* in the pkg-plist file, it would save on maintenance.

x11/libfm/Makefile
103 ↗(On Diff #45728)

Remove blank line.

rigoletto updated this revision to Diff 45741.EditedJul 23 2018, 7:53 PM

Strictly following the example would break the gtk2 version, and so I changed it a bit.

Thanks!

rigoletto marked an inline comment as done.Jul 23 2018, 7:57 PM
mat added a comment.Jul 24 2018, 7:33 AM
In D16387#348376, @lbdm_privacychain.ch wrote:

Strictly following the example would break the gtk2 version, and so I changed it a bit.

Not sure what it would break, but ok.

x11/libfm/Makefile
64–70 ↗(On Diff #45759)

There already is all the .if dance 10 lines above, you should put PLIST_SUB in there instead of adding all this blob.

rigoletto marked an inline comment as done.Jul 24 2018, 1:28 PM
mat accepted this revision as: portmgr.Jul 24 2018, 5:41 PM

Looks ok wrt flavors.

There are other ports depending on the now flavored x11/libfm -- maybe they should be flavored too? -- like x11/lxpanel.

Is the flavoring really needed?
The number of packages is exploding

IIRC LXDE can be completely FLAVORed however I guess just a handful of people should be using it on FreeBSD, and that is one of the reasons I didn't FLAVORed evertyhing since the beginning ( I think it does not worth the work ).

The second reason is pcmanfm/libfm are not part of LXDE development group anymore. They are now developed in separated since there are tons of people using them outside LXDE ( I guess there are more people using only pcmanfm/libfm in total than using LXDE in general, since pcmanfm is present in most of the WMs installations ).

Also, IIRC the next LXDE and pcmanfm/libfm version will be gtk3 only, I just don't have idea when they will come out.

Thanks!

rigoletto updated this revision to Diff 49197.Oct 15 2018, 11:14 PM

Make ports that depends on x11/libfm aware of the FLAVORing.

Also, pet portlint.

mat added a comment.Oct 16 2018, 8:55 AM

Could you remove all the whitespace changes? They makes it hard to see what you are actually changing.

Hi @mat,

Do you mean the tab space I've added in some ports? There were some without space after =.

I will revert them, the actual important change is the addition of @gtk2 on the libfm dependency:

libfm-gtk.so:x11/libfm@gtk2
rigoletto updated this revision to Diff 49236.Oct 16 2018, 5:18 PM

Reverting part of the changes in the ports that depends on libfm.

Thanks!

rigoletto updated this revision to Diff 52325.Dec 26 2018, 9:57 PM
  • updating x11/libfm and x11-fm/pcmanfm to version 1.3.1
  • also affect x11/libfm-extra
  • #Created by header removed of all affected ports
rigoletto edited the summary of this revision. (Show Details)Dec 26 2018, 10:05 PM
rigoletto edited the test plan for this revision. (Show Details)Dec 26 2018, 10:09 PM
mat added a comment.Dec 27 2018, 9:31 AM

Why do you remove all the Created by lines, were those removal requested?

In D16387#398067, @mat wrote:

Why do you remove all the Created by lines, were those removal requested?

Given the previous direction when reviewing x11/i3blocks, to remove that line without asking, I took that as a default behavior. ^^

adamw added a subscriber: adamw.Dec 27 2018, 3:33 PM

"Created by" lines should never be removed without the express consent of the person named in there. Please, put them all back.

bapt added a subscriber: bapt.Dec 27 2018, 3:39 PM
In D16387#398067, @mat wrote:

Why do you remove all the Created by lines, were those removal requested?

Given the previous direction when reviewing x11/i3blocks, to remove that line without asking, I took that as a default behavior. ^^

Just to explain the history here, there used to be a "# Created by line" long ago, it has been decided, that this wasn't necessary anymore, so we do recommend on new things not to add that line anymore, but it should be left on historical makefiles except if the initial creator accept removing it explicitly

rigoletto updated this revision to Diff 52341.Dec 27 2018, 5:01 PM

Reverting the Created by line.

@adamw @bapt

Thank you for the explanations.

tcberner accepted this revision.Jan 7 2019, 6:00 AM

Looks good to me. Do you have the maintainers approval?

This revision is now accepted and ready to land.Jan 7 2019, 6:00 AM

Looks good to me. Do you have the maintainers approval?

I think the @jsm reply on bugs was a yes. Same for Chris (@portmaster_BSDforge.com). ^^

This revision was automatically updated to reflect the committed changes.