Bug 276792 - devel/cmake*: Make port Makefiles more consistent
Summary: devel/cmake*: Make port Makefiles more consistent
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-kde (group)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-03 07:02 UTC by Daniel Engberg
Modified: 2024-02-03 23:33 UTC (History)
3 users (show)

See Also:
jhale: maintainer-feedback+


Attachments
Patch for CMake* (4.58 KB, patch)
2024-02-03 07:02 UTC, Daniel Engberg
no flags Details | Diff
Patch for CMake* v2 (437 bytes, patch)
2024-02-03 21:36 UTC, Daniel Engberg
no flags Details | Diff
Patch for CMake* v3 (4.50 KB, patch)
2024-02-03 21:50 UTC, Daniel Engberg
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Engberg freebsd_committer freebsd_triage 2024-02-03 07:02:51 UTC
Created attachment 248149 [details]
Patch for CMake*

No functional change, just adjust Makefiles to use the same syntax
There's also a duplicated line in devel/cmake-man

Poudriere checks out fine on 13.2 and 14.0
Comment 1 Jason E. Hale freebsd_committer freebsd_triage 2024-02-03 20:22:21 UTC
What tools did you check with when making this patch? Prior to, portclippy(1) had 2 suggestions for cmake-core alone, now it has 4. Sorry to be so blunt, but this seems like a lot of unnecessary churn and putting CXXFLAGS/LDFLAGS above CONFIGURE_ARGS/CMAKE_ARGS is just gross. The configure stage comes first, after all.

Good catch with the duplicate DISTINFO_FILE in cmake-man, but it definitely doesn't belong between LICENSE and BUILD_DEPENDS. We can surely make some improvements in the cmake ports, at least to be consistent, but let's at least try to make portlint(1) and portclippy(1) happy.
Comment 2 Daniel Engberg freebsd_committer freebsd_triage 2024-02-03 21:36:03 UTC
Created attachment 248164 [details]
Patch for CMake* v2
Comment 3 Jason E. Hale freebsd_committer freebsd_triage 2024-02-03 21:45:50 UTC
(In reply to Daniel Engberg from comment #2)
Hmm...I'm not sure that's what you wanted attach. I already said in bug #276782 that CONFIGURE_ARGS work better for increasing verbosity and I have changes I'm testing in poudriere at this very moment with other improvements such as supporting ccache in the bootstrap phase and switching to Qt6. I've also sorted the Makefiles to satisfy portlint(1) and portclippy(1).
Comment 4 Daniel Engberg freebsd_committer freebsd_triage 2024-02-03 21:49:13 UTC
Portlint is happy, portclippy isn't but then again it does get some things wrong and makes Makefiles a lot harder to follow if you were to follow it completely.

I guess we're going to disagree with this but most if not all ports tries to follow the "manual flow" which portclippy gets wrong.

Placing CXXFLAGS before configure makes more logical sense as they need to be defined before getting parsed by whatever build framework or tool the project (port) is using.

This also goes for build framework related variables,
toggles like CONFIGURE/CMAKE/MESON_ON, _OFF, _ARGS, _ENV. These are preferably placed at the end right before you define options because that makes more sense as you likely will use build framework options and increases readbility. It also makes the Makefile a lot easier to follow and is what we do in the vast majority of ports.

Anyhow, I've moved it further down now.

Best regards,
Daniel
Comment 5 Daniel Engberg freebsd_committer freebsd_triage 2024-02-03 21:50:30 UTC
Created attachment 248165 [details]
Patch for CMake* v3

Attach correct patch
Comment 6 Jason E. Hale freebsd_committer freebsd_triage 2024-02-03 22:08:42 UTC
(In reply to Daniel Engberg from comment #4)
Well, agree to disagree. PHB 15.9 says that the order of standard bsd.port.mk variables is not important, but even before portclippy came along it always made more sense to me to put configure-type arguments (CONFIGURE_ARGS/CMAKE_ARGS/MAKE_ENV) higher than XFLAGS and in a separate block. FWIW, I don't think portclippy is always right either. For example, I think LIB_DEPENDS should be sorted by origin rather than library name, but since this is a team port, I think it is best to follow the advice of the tools we have so we are all on the same page and not constantly making unnecessary personal style changes.
Comment 7 Daniel Engberg freebsd_committer freebsd_triage 2024-02-03 22:13:16 UTC
Either way, this at least makes all files consistent which they weren't before. If you want to do more adjustments go ahead.
Comment 8 commit-hook freebsd_committer freebsd_triage 2024-02-03 23:26:06 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=97c31b93991c7a4bef0b2e8a5f8ec6c42e3e7d86

commit 97c31b93991c7a4bef0b2e8a5f8ec6c42e3e7d86
Author:     Jason E. Hale <jhale@FreeBSD.org>
AuthorDate: 2024-02-03 21:34:32 +0000
Commit:     Jason E. Hale <jhale@FreeBSD.org>
CommitDate: 2024-02-03 23:19:43 +0000

    devel/cmake*: Pet portclippy(1) and migrate to Qt6

    Migrate towards Qt6:
    - Switch the default flavor of devel/cmake-gui to qt6.
    - Use qt6-tools instead of qt5-help in devel/cmake-doc to generate the Qt
      help file.

    Depend on the flavorized version of textproc/py-sphinx.

    Pet portclippy(1) and portlint(1) and make Makefile order more consistent
    within the devel/cmake* ports. Remove duplicate DISTINFO_FILE line from
    devel/cmake-man. [1]

    Make build of devel/cmake-core verbose [2] and respect WITH_CCACHE_BUILD
    in bootstrapping phase.

    Remove the "experimental" annotation from the CPACK option in
    devel/cmake-core to avoid end-user confusion. [3] The FreeBSD CPack
    generator has been marked experimental since version 3.11.4, but is
    supported upstream and shouldn't be considered experimental even if
    we've had to make some local changes to it. If it isn't working
    correctly, please file a bug report.

    PR:             276792 [1], 276782[2], 274640 [3]
    Reported by:    diizzy [1], Anton Saietskii <vsasjason@gmail.com> [2],
                    Martin Waschbüsch <martin@waschbuesch.de> [3]

 devel/cmake-core/Makefile                             | 17 +++++++++++------
 .../patch-Source_CPack_cmCPackFreeBSDGenerator.cxx    |  6 ++----
 devel/cmake-doc/Makefile                              | 19 ++++++++++---------
 devel/cmake-gui/Makefile                              |  7 ++++---
 devel/cmake-man/Makefile                              | 17 ++++++++---------
 5 files changed, 35 insertions(+), 31 deletions(-)
Comment 9 Jason E. Hale freebsd_committer freebsd_triage 2024-02-03 23:33:12 UTC
Makefiles are more consistent now along with some other fixups.