Page MenuHomeFreeBSD

devel/geany and plugins: add flavors for gtk2 and gtk3
ClosedPublic

Authored by madpilot on Nov 17 2018, 4:36 PM.

Details

Summary

In bug report 226523 Greg V. proposed switching geany to gtk3 using an option. This was no possible due to not being able to force gtk3 in geany from the plugins.

I have built this patch set to leverage flavors, so that correct dependency chains can be enforced by pkg.

The devel/geany-plugins-l10n could actually be unflavored, but I am unable to find a way to make it unflavored and not break or pollute the dependency chain.

Test Plan

Geany and all plugins build fine in poudriere for both flavors on all supported versions.

They both run correctly on head.

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

madpilot created this revision.Nov 17 2018, 4:36 PM

Gtk 2 is legacy, Gtk 3 should be used always.

madpilot added a comment.EditedNov 17 2018, 5:23 PM

Gtk 2 is legacy, Gtk 3 should be used always.

I agree on that, but this project is still in the middle of migrating.

Some plugins work only when using gtk2. Jumping unconditionally to gtk3 would force me to delete those. The plugins not working with gtk3 are listed in the UPDATING entry in the patch.

I was planning to move to flavors for now, switch to gtk3 as default flavor at a later time, hoping in the while all plugin have been migrated.

Otherwise I could make gtk3 the default flavor now and plan removing the gtk2 one soon if no user complains about the missing plugins.

madpilot updated this revision to Diff 50593.Nov 19 2018, 9:11 AM
madpilot edited the summary of this revision. (Show Details)
madpilot edited the test plan for this revision. (Show Details)

Made the default flavor the gtk3 one.

Also fixed plugins not working with gtk3 to not have a gtk3 flavor (which would fail)

mat requested changes to this revision.Nov 23 2018, 11:00 AM
mat added inline comments.
devel/geany-plugin-debugger/Makefile
10 ↗(On Diff #50593)

Having only one flavor makes no sense. Simply set FLAVORS= so that it stays empty afterwards.

devel/geany-plugin-markdown/Makefile
16–20 ↗(On Diff #50593)

Please use flavors helpers.

devel/geany-plugins/files/Makefile.common
23–24 ↗(On Diff #50593)

FLAVOR only make sense to the current port, it is not transitive, it CANNOT be used in a _DEPENDS line. This is why framework generated flavors have their own flavors to add dependencies.

Please use static flavors in the depends names.

33 ↗(On Diff #50593)

That is awful.

This revision now requires changes to proceed.Nov 23 2018, 11:00 AM
madpilot planned changes to this revision.Nov 23 2018, 1:56 PM
madpilot marked 7 inline comments as done.

I marked the accepted comments as done. I'll followup with an updated diff including such changes after testing.

The while bunch of geany plugins Makefiles is a mess. If and when we get subpackages the whole bunch can be converted to one port with subpackages, since all plugins can be built together and then file picked for each generating separate packages.

I don't see an immediate way to overhaul them which would simplify things without bringing a lot of duplication.

UPDATING
30 ↗(On Diff #50593)

Also going to sort these.

devel/geany-plugins/files/Makefile.common
33 ↗(On Diff #50593)

It actually is. I don't even know what possessed me when I wrote this.

58 ↗(On Diff #50593)

This one also needs to be made a constant, then.

madpilot marked 2 inline comments as done.Nov 23 2018, 3:33 PM

I'm also seeing a problem with the options in the geany-plugins port. I'm planning to use groups to distinguish the ones actually available only with gtk2 and gtk3, leaving them all enabled by default, but making the gtk2 ones ineffective when the gtk3 ports are compiled.

I could also make the appear and disappear depending on the flavor, but I don't like that idea.

devel/geany-plugin-debugger/Makefile
10 ↗(On Diff #50593)

Testing shows this creates problems later in Makefile.common, where a flavor is expected to be defined.

Since this happpens only for gtk2 flavor, I can fix it by changine the

.if ${FLAVOR} == gtk2
.elif ${FLAVOR} == gtk3
.endif

to

.if ${FLAVOR} == gtk3
.else
.endif

since I don't plan adding any more flavors to these ports.

madpilot updated this revision to Diff 51041.Nov 24 2018, 10:08 AM

Solved the reported issues.

I also changed some RUN_DEPENDS+BUILD_DEPENDS to LIB_DEPENDS, which is what they actually are.

The OPTIONS in the devel/geany-plugins meta port are also now working correctly and subdivided using GROUPs.

greg_unrelenting.technology added inline comments.
UPDATING
17 ↗(On Diff #51041)

"disinstall" doesn't sound like a word :) "deinstall" or "uninstall"

20 ↗(On Diff #51041)

same here

madpilot planned changes to this revision.Nov 25 2018, 4:35 PM
madpilot marked 2 inline comments as done.
madpilot updated this revision to Diff 51089.Nov 25 2018, 4:37 PM

Change "disinstall" to "deinstall".

madpilot marked an inline comment as done.Nov 25 2018, 4:39 PM
madpilot added inline comments.
UPDATING
17 ↗(On Diff #51041)

Thanks for pointing this out!

Not being a native English speaker, thia was an "italianism" from my part.

"disinstallare" is the Italian word that fooled me.

mat added inline comments.Dec 6 2018, 3:31 PM
devel/geany-plugin-markdown/Makefile
16–18 ↗(On Diff #51089)

Wrong place in the Makefile. See Chapter 15. Order of Variables in Port Makefiles.

16–18 ↗(On Diff #51041)

Wrong place in the Makefile. See Chapter 15. Order of Variables in Port Makefiles.

devel/geany-plugin-vc/Makefile
18–20 ↗(On Diff #51089)

same.

madpilot planned changes to this revision.Dec 6 2018, 5:13 PM
madpilot marked 2 inline comments as done.
madpilot updated this revision to Diff 51666.Dec 6 2018, 5:24 PM

Fix variable order.

mat added inline comments.Dec 8 2018, 2:28 PM
devel/geany-plugin-markdown/Makefile
11–12 ↗(On Diff #51666)

Why +=? (Also, there should probably be a blank lines between the depends section and the flavors section.)

devel/geany-plugin-vc/Makefile
10–11 ↗(On Diff #51666)

Why +=?

devel/geany-plugin-webhelper/Makefile
14–16 ↗(On Diff #51666)

wrong place.

madpilot planned changes to this revision.Dec 8 2018, 4:49 PM
madpilot marked 4 inline comments as done.
madpilot added inline comments.
devel/geany-plugin-markdown/Makefile
11–12 ↗(On Diff #51666)

Was += because gtk[23]_LIB_DEPENDS is already populated in Makefile.common included below. Now that it's sorted back the += needs to be moved there really.

Thanks for noticing!

madpilot updated this revision to Diff 51757.Dec 8 2018, 6:05 PM
madpilot marked an inline comment as done.

Move += where appropriate, and add empty lines.

mat accepted this revision.Dec 9 2018, 10:28 AM

Looks ok, make sure it all builds, and does not break INDEX before committing ;-)

This revision is now accepted and ready to land.Dec 9 2018, 10:28 AM
This revision was automatically updated to reflect the committed changes.