Page MenuHomeFreeBSD

editors/texworks: Update to 0.6.7
AcceptedPublic

Authored by kfv_kfv.io on Oct 1 2021, 7:04 PM.

Details

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

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 45104
Build 41992: arc lint + arc unit

Event Timeline

koobs edited the test plan for this revision. (Show Details)

Acceptance pending QA confirmation (portlint, poudriere)

koobs requested changes to this revision.Nov 18 2021, 1:29 AM
koobs edited the summary of this revision. (Show Details)
This revision now requires changes to proceed.Nov 18 2021, 1:29 AM

Acceptance pending QA confirmation (portlint, poudriere)

Both portlint and poudriere testport successfully passed.

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 marked an inline comment as done.
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.

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.

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 requested changes to this revision.Apr 8 2022, 2:10 AM

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.

This revision now requires changes to proceed.Apr 8 2022, 2:10 AM

Loop in folks involved in the recent text update

  • editors/texworks: Update to 0.6.7
kfv_kfv.io retitled this revision from editors/texworks: Update to 0.6.6 to editors/texworks: Update to 0.6.7.Apr 8 2022, 11:21 PM
kfv_kfv.io edited the summary of this revision. (Show Details)
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 edited the summary of this revision. (Show Details)

Add changelog link to commit log message.

Otherwise LGTM πŸ‘

This revision is now accepted and ready to land.Apr 8 2022, 11:40 PM
This comment was removed by bofh.
bofh requested changes to this revision.Apr 9 2022, 10:43 PM

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

This revision now requires changes to proceed.Apr 9 2022, 10:43 PM

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

bofh requested changes to this revision.Apr 10 2022, 12:09 PM

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.

This revision now requires changes to proceed.Apr 10 2022, 12:09 PM
  • editors/texworks: Move PLIST_FILES to pkg-plist

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.

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

Macro stampofapproval:

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

This revision is now accepted and ready to land.Apr 20 2022, 8:36 PM

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.