Page MenuHomeFreeBSD

x11-toolkits/qwt6: Add qt flavors and update all dependent ports
ClosedPublic

Authored by lbartoletti on Feb 21 2018, 8:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 24, 12:42 PM
Unknown Object (File)
Thu, Dec 19, 7:09 AM
Unknown Object (File)
Mon, Dec 16, 12:42 PM
Unknown Object (File)
Wed, Dec 4, 4:10 AM
Unknown Object (File)
Sun, Dec 1, 2:41 AM
Unknown Object (File)
Sun, Dec 1, 2:06 AM
Unknown Object (File)
Fri, Nov 29, 9:17 AM
Unknown Object (File)
Fri, Nov 29, 9:17 AM

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Hi there

This looks like a good start!

Some things seem to be missing:

  • files have only size 0 but are not deleted
  • empty directories remained
  • PORTREVISIONS of the dependencies should be bumped
  • x11-toolkits/Makefile needs to be updated
  • MOVED entries for the removed -designerplugin port
finance/qhacc/Makefile
31 ↗(On Diff #39583)

${QT_INCDIR} and ${QT_LIBDIR}

x11-toolkits/qwt6-designerplugin/Makefile
1 ↗(On Diff #39583)

Analgously for the other deleted ports the directory has not been removed in the diff.

Further you need to

  • update x11-toolkits/Makefile and remove the deleted directories from it.
  • add a new entry to MOVED
x11-toolkits/qwt6/Makefile
20 ↗(On Diff #39583)

This looks dangerous, as it assumes the path of the Qt installation to be something specific.

You should try to use the variables exported by bsd.qt.mk for the paths.

x11-toolkits/qwt6/files/patch-qwtconfig.pri
39 ↗(On Diff #39583)

Here also you should use QT_MKSPECDIR from bsd.qt.mk.

Could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

mat requested changes to this revision.Feb 22 2018, 11:14 PM

Also, please, try to use flavor helpers whenever possible.

x11-toolkits/qwt6/Makefile
23–24 ↗(On Diff #39583)

NO

The default flavor is the first one.

This revision now requires changes to proceed.Feb 22 2018, 11:14 PM

Requested changes

  • Add QT_INCDIR, QT_LIBDIR and QT_MKSPECDIR whenever possible
  • Add more FLAVOR whenever possible
  • Add MOVED entry for x11-toolkits/qwt6-designerplugin
  • Add a new patch for FindQWT.cmake in x11/leechcraft
  • Change FLAVORS order for x11-toolkits/qwt6 (qt4 is still the default flavor

)

  • Remove pkg-plist.qt4 and pkg-plist.qt5 in favor of an unique pkg-plist for

x11-toolkits/qwt6

@mat Sorry for this first review, I installed arcanist but I have this error running arc diff

Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.

Not yet well configured... so I ran svn diff --ignore-properties | arc diff --raw and for the changes: svn diff --ignore-properties -x -U9999 | arc diff --raw --update D14462 --message 'Requested changes'

x11-toolkits/qwt6/Makefile
21–22 ↗(On Diff #39693)

This should happen earlier, see Chapter 15. Order of Variables in Port Makefiles.

25–30 ↗(On Diff #39693)

Put PKGNAMESUFFIX back where it was before, both definitions are the same, so no need to repeat it, or to have it in here.

36–39 ↗(On Diff #39693)

We use %%FOO%% as placeholders everywhere, it would be preferable to using % here too and not _.

x11-toolkits/qwt6/files/patch-qwt.prf
1–2 ↗(On Diff #39693)

Not sure how you regenerated your patches, but this change is not needed and would not have been generated by make makepatch.

x11-toolkits/qwt6/pkg-plist
112–116 ↗(On Diff #39693)

You already have PORTDOCS defined in the Makefile, you must not list those files here too.

x11/leechcraft/files/patch-plugins_azoth_plugins_otroid_otrhandler.cpp
1–2 ↗(On Diff #39693)

This change is ignored by make makepatch, it should be ommitted. (same for all patches where only this kind of changes happen.

x11-toolkits/qwt6/Makefile
25 ↗(On Diff #39693)

^ you could possibly get away with setting it as
PKGNAMESUFFIX=6-${_QT_RELNAME}
just once -- but I'm not sure if that is a good idea

x11-toolkits/qwt6/Makefile
25 ↗(On Diff #39693)

.. strike that, just write the current one once ^^

x11-toolkits/qwt6/Makefile
15 ↗(On Diff #39693)

This could be removed and changed to USES=uniquefiles:dirs

lbartoletti marked 10 inline comments as done.

In fact, I taken my patch on bugs.freebsd and apply it in a new poudriere ports tree. I think it's not a good practice.
I have recreate a new poudriere ports tree and made change directly, so it's more clear.

I have also add QwtDesigner which was forgotten.

Thanks

MOVED
10160 ↗(On Diff #40043)

That line is bogus.

x11-toolkits/qwt6/Makefile
37–39 ↗(On Diff #40043)

formatting.

x11-toolkits/qwt6/files/patch-qwt.prf
1–2 ↗(On Diff #40043)

Remove this change.

x11-toolkits/qwt6/files/patch-textengines_textengines.pri
1–3 ↗(On Diff #40043)

Remove this change.

x11-toolkits/qwt6/pkg-descr
1 ↗(On Diff #40043)

. should be followed by two spaces.

lbartoletti marked 6 inline comments as done.

Remove unmodified patches (textengines.pri and qwt.prf in qwt6.
Fix MOVED.
Formatting qwt6/Makefile (use only tab)

x11-toolkits/qwt6/pkg-plist
1 ↗(On Diff #40066)

^ I'm not sure if the ports should use QT_INCDIR as an installation directory. I assume you chose it to have the two flavours not conflict?
Maybe something like ${PREFIX}/include/qwt6-${FLAVOR} should be preferred -- but if it is needed to have it work at all, then no objection :)

x11/leechcraft/files/patch-cmake_FindQwt.cmake
7 ↗(On Diff #40066)

^ this should probably be than with substituting values form bsd.qt.mk (if you still chose to install to ${QT_INCDIR}) during build, instead of "hard"-coding the path n the file.

15 ↗(On Diff #40066)

^same.

lbartoletti marked 2 inline comments as done.

QT_INCDIR and QT_LIBDIR for x11/leechcraft

x11-toolkits/qwt6/pkg-plist
1 ↗(On Diff #40066)

Yes. I don't want conflicts :)
I don't know the "best" solution. I proposed that since qwt is very linked - like a sub module - with qt5.

The flavorization looks ok to me.

This revision is now accepted and ready to land.Apr 11 2018, 8:58 AM
In D14462#316678, @mat wrote:

The flavorization looks ok to me.

@mat, this will add a new package for the Qt5 flavor -- is this still blocked (as there currently only is the qt4 package).

In D14462#316678, @mat wrote:

The flavorization looks ok to me.

@mat, this will add a new package for the Qt5 flavor -- is this still blocked (as there currently only is the qt4 package).

@tcberner @mat FYI, qwt6@qt5 will be required by graphics/qgis version 3.0

This revision was automatically updated to reflect the committed changes.