editors/texworks: Update to 0.6.7

[kfv_kfv.io]

Summary

editors/texworks: Update to 0.6.7

Reviewed_by: koobs, ??
Approved by:
Differential_Revision: D32265
MFH: 2022Q2 (bug fixes and compatibility improvements)

Blocks: D32264

Test Plan

• portlint: passed
• testport: passed
• runtime: I don't know texworks, but I built and tried it successfully on FreeBSD 13.0-RELEASE

[koobs]

Acceptance pending QA confirmation (portlint, poudriere)

[kfv_kfv.io]

Acceptance pending QA confirmation (portlint, poudriere)

Both portlint and poudriere testport successfully passed.

[koobs]

editors/texworks/Makefile

35–36

Explain all changes in commit log (formatted) message in review summary. Something like:

- Include additional icons for desktop users

Or similar, which describes value/rationale to readers

44

Should we pass explicit PYTHON_VERSION here ?

What is cmake's behaviour when its not passed and multiple version of python exist on the system?

If the default version (or version it finds), is different than the Python version for this port build, it will be built incorrectly.

[kfv_kfv.io]

editors/texworks/Makefile

44

Does PYTHON_USES= python:3.4+ in line 51 not precede PYTHON_CMAKE_BOOL. If it does, we'll be safe. Otherwise, yes, it'll either build incorrectly or fail altogether.

[koobs]

editors/texworks/Makefile

44

USES=python only imports Mk/Uses/python.mk and sets variables (like PYTHON_VER). It doesn't (can't) know how to pass the environment to any particular build system.

I believe CMakeLists files usually have a variable for explicitly setting the Python version one wants to use/build with.

[kfv_kfv.io]

editors/texworks/Makefile

44

Oh OK, I see, you're right. Do you think using something like PYTHON_CMAKE_ON could do the job? I haven't ever used CMake professionally but just checked the CMakeLists.txt and there I found this:

if (WITH_PYTHON)
  CONFIG_VERSION("Python" "${PYTHON_VERSION_STRING}")
endif()

So, I think we need something like the following:

PYTHON_CMAKE_ON=   -DPYTHONVERSION:STRING="${PYTHON_VER}"

Right?

[koobs]

Pending QA confirm on Python.

editors/texworks/Makefile

44

Seems reasonable, but test it (in both OFF and ON cases) to confirm it does what you want, and has the value, and uses the Python you want.

I cant say anything more definitive than that, because not all softwares that use CMake use the standard Cmake mechanics.

Bottom line is, ports need to 'explicitly' and 'deterministically' specify the python version to use, otherwise different and incorrect versions may be detected if the system the port being build on has multiple.

[koobs]

Loop in folks involved in the recent text update

[kfv_kfv.io]

• editors/texworks: Update to 0.6.7

[kfv_kfv.io]

editors/texworks/Makefile

44

I checked ports using CMake to see how other maintainers handle such dependencies, and it seems like this port's using the same convention, and we don't need to change it.

According to the handbook, OPT_CMAKE_BOOL is a shorter helper for OPT_CMAKE_ON with -D settings to enable/disable and set/change defaults. Moreover, as far as I could check (and understand), OPT_USES already specifies the required version, and as long as CMake prioritise the system version and doesn't mandate a specific version, there will be no conflicts.

I recommend we keep the style as-is and only update the version. But I might be completely wrong, and we might need to improve the version handling - in which case, I would appreciate it if anyone educates me.

[koobs]

Add changelog link to commit log message.

Otherwise LGTM 👍

[bofh]

This comment was removed by bofh.

[bofh]

Sorry for the noise. Not sure why commenting on another review asked me to login and redirect to this page.

[kfv_kfv.io]

@bofh: It happens ;-D But one question, is there anything I should really change in this revision or the request is unintentionally opened?

[bofh]

Hi,
So far I can see two things:

1. The PLIST_FILES have grown big enough that it should be moved to a separate pkg-plist file now
2. Can you check the file Mk/bsd.options.mk rather than the Handbook

And as there is now TeX 2021 you should do a build check again after the changes; although I checked this port builds properly with TeX 2021. And actually @hrs can say more whether if there is any specifics about this port.

[kfv_kfv.io]

• editors/texworks: Move PLIST_FILES to pkg-plist

[kfv_kfv.io]

The PLIST_FILES have grown big enough that it should be moved to a separate pkg-plist file now

Done.

Can you check the file Mk/bsd.options.mk rather than the Handbook

Yes, I checked, and to my understanding, that's the reason why everyone follows this convention of OPT_USES= <lang>[:ver] + OPT_CMAKE_BOOL= WITH_<lang> for such a case without passing the version to CMAKE explicitly, as it picks the system version - but correct me if I'm wrong, please.

And as there is now TeX 2021 you should do a build check again after the changes; although I checked this port builds properly with TeX 2021. And actually @hrs can say more whether if there is any specifics about this port.

Yes, I ran poudriere-testport(8) on a fresh box with the new TeX 2021 port.

[kfv_kfv.io]

@bofh: could you take another look, please? Let me know if there's anything I can/should improve.

[bofh]

Normally we prefer that the submitter submits at least links to poudriere logs in all supported TIER-1 Versions. 😎

[kfv_kfv.io]

Thank you so much, sir. I did not know that. I make sure to keep it in mind for future contributions. I didn't have a dedicated server for my tests; I usually used to fire up an instance and remove it right after my tests. But I will take care of it shortly, roger that.

[jrm]

I'm going to guess this review has been overcome by events since 0.6.7 is in the tree and it seems the comments are resolved. @kfv_kfv.io, thanks for submitting. If I'm wrong here, please let me know.