Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minetest: new maintainer & new Portfile #17077

Merged
merged 1 commit into from
Dec 31, 2022
Merged

minetest: new maintainer & new Portfile #17077

merged 1 commit into from
Dec 31, 2022

Conversation

Zweihorn
Copy link
Contributor

@Zweihorn Zweihorn commented Dec 27, 2022

MT 5.6.1 minetest Revision "1" (i.e. 1️⃣ ) with improved Portfile and new maintainer ready for review.

  • Portfile derived from legacy Portfile, however heavily improved
  • the b.m. GMP bugfix
  • updates and improvements to 'depends_lib-append'
  • updates and improvements to 'configure.args-append'
  • new maintainer (Zweihorn & openmaintainer)
  • keep all "path" requirements
  • keep "-DLUA_" options for better specificity
  • keep all diffs in files untouched

Closes: https://trac.macports.org/ticket/66408 - (bugfix)

Ref https://trac.macports.org/ticket/66562
Ref #17080

Description

MT 5.6.1 minetest Revision "1" (i.e. 1️⃣ ) with new Portfile greatly improving the overall performance of the Minetest App and continuing in good compatibility to other ports.

The technical details and the description of #17080 apply as amended by this PR.

Please refer to the conversation resolved in that a.m. PR and the resolve in this PR below.
That a.m. PR became obsolete and there is no need for an extra minetest-devel port.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 11.7.2 20G1020 x86_64
Command Line Tools 13.2.0.0.1.1638488800

Verification

Ref #17080

For the binary check and library bindings see below at:

Examples of Minetest 5.6.1 GUI

Example 1 of Minetest 5.6.1 GUI world view with Ethereal NG mod and other MT mods added for a better MT Player user experience.

screenshot_20221220_100146

Example 2 of Minetest 5.6.1 GUI with MT main menu:

Bildschirmfoto 2022-12-29 MT

The example shows text in the German locale with "Hauptmenü" at the UI main menu. The text "MacPorts" and the MacPorts revision No. are concatenated to the MT version text at the top of the GUI window.

NOTE: Amended in the new Profile to show "MT 5.6.1-MacPorts-rev1" for optical reasons.

Fascicle

More information on @Zweihorn at:

MT rocks. Have fun.

🌻 🚀 📦

@Zweihorn
Copy link
Contributor Author

This is related to #17080

@Zweihorn Zweihorn changed the title minetest new maintainer by change-minetest-maintainer.diff minetest: new maintainer Dec 27, 2022
Copy link
Contributor

@reneeotten reneeotten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be a "diff" file.

@Zweihorn
Copy link
Contributor Author

this shouldn't be a "diff" file.

Please provide some clarification and further advice.

IMHO this was just following the explanations from the MacPorts Guide as were:

  1. Create a patch from the changes you made to the Portfile and possible related files. To do that, run

%% diff -ru $(port dir $portname) . > change-$portname-maintainer.diff

in the directory where you edited the Portfile. You can inspect the generated unified diff in change-$portname-maintainer.diff if you want.

  1. If you are only changing the maintainer, file a pull request on GitHub.

See: 7.3.3. Becoming a Port Maintainer

Sorry, I might look a little lost here. Please what should I do to improve?

Would just deleting the misplaced diff help now and every body happy? Or what to do?

@Zweihorn
Copy link
Contributor Author

However, thank you very much. I highly appreciate your commitment in guiding me through this.

@Zweihorn Zweihorn marked this pull request as draft December 28, 2022 22:11
@Zweihorn
Copy link
Contributor Author

Changed to DRAFT - Rationale:

There is no need for twoe ports in parallel. Thus moving the new Portfile from #17080 to this PR and would change name to "new maintainer & new Portfile" apparently in due time.

Stay tuned . . .

@Zweihorn Zweihorn changed the title minetest: new maintainer minetest: new maintainer & new Portfile Dec 28, 2022
@Zweihorn
Copy link
Contributor Author

New Profile Revision 1 with commit 2111856 makes the prior commits obsolete. C'est la vie.

@Zweihorn
Copy link
Contributor Author

MT 5.6.1 minetest Revision 1 with improved Portfile and new maintainer ready for review.

This PR makes #17080 obsolete. This was a good learning experience as new maintainer, I presume.

@Zweihorn Zweihorn marked this pull request as ready for review December 28, 2022 23:54
@Zweihorn
Copy link
Contributor Author

@reneeotten - Regarding maintainer {macports.org:Zweihorn @Zweihorn} in the Portfile:

I applied with subject "Commit access: Zweihorn" to macports-mgr at lists.macports.org today.
Date: Thu, 29 Dec 2022 02:04:19 +0100

Hope this helps.
🌻

games/minetest/Portfile Outdated Show resolved Hide resolved
games/minetest/Portfile Outdated Show resolved Hide resolved
games/minetest/Portfile Show resolved Hide resolved
games/minetest/Portfile Outdated Show resolved Hide resolved
games/minetest/Portfile Outdated Show resolved Hide resolved
games/minetest/Portfile Outdated Show resolved Hide resolved
games/minetest/Portfile Show resolved Hide resolved
games/minetest/Portfile Show resolved Hide resolved
games/minetest/Portfile Show resolved Hide resolved
games/minetest/Portfile Show resolved Hide resolved
@reneeotten
Copy link
Contributor

Also, you'll have to drop all this "personal license and EUPL-1.2" stuff.

If you want to contribute you will do so as anyone else under the MacPorts license - you do not have any personal rights to your contributions.

@Zweihorn
Copy link
Contributor Author

Also, you'll have to drop all this "personal license and EUPL-1.2" stuff.

If you want to contribute you will do so as anyone else under the MacPorts license - you do not have any personal rights to your contributions.

I am neutral on this. AFAIK there is no "MacPorts" licence but "MIT" or "BSD" or "LGPL" and "EUPL", I presume.
However, you already pointed me at BSD for MacPorts and LGPL-1.2+ for Minetst quite correctly. This is resolved.

My home is my castle. However, I will happily consider not polluting my next PRs and agree thse first PRs were a little tsunami of text and thoughts. Unfortunately, I am an extrovert and do my thinking while typing. Introverts differ and take the burden. However, I promise to improve and this is already a good learning experience to me and I started shortening some texts somehow already. Miles to go but a nice start for me as a maintainer in the making.

Thank you for your kind advice.

@Zweihorn
Copy link
Contributor Author

I would hope to have resolved the above now. The minor deviations should be agreeable to you, hopefully.

You obviously know were to find me if not.

port lint --nitpick                                      
--->  Verifying Portfile for minetest
Warning: Line 14 seems to hardcode the version number, consider using ${version} instead
--->  0 errors and 1 warnings found.

Next review please.
🌞

games/minetest/Portfile Outdated Show resolved Hide resolved

depends_lib-append port:irrlichtmt \
path:include/turbojpeg.h:libjpeg-turbo \
port:libjpeg-turbo \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I said before, please keep all the "path-style" dependencies - you can look them up in the MacPorts guide.

In short, for some ports there is a "-devel" subport or some other drop-in replacement which can fulfill the dependency requirement as well. To allow that one should use the path-style dependency instead of adding the explicit port dependency.

games/minetest/Portfile Show resolved Hide resolved
@Zweihorn
Copy link
Contributor Author

New idea and thus addendum 2 to 2nd review:

  • amend the rev# to "-DVERSION_EXTRA" for consistency and clarity

stay tuned

@Zweihorn
Copy link
Contributor Author

Zweihorn commented Dec 29, 2022

Ready for next review. (My poor joke: No natural logarithm effects hopefully.)

  • new maintainer (Zweihorn & open maintainer)
  • keep all "path" requirements for good reasons
  • keep "-DLUA_" options for better specificity
  • revision 2 intentionally by maintainer (there should be enough numbers to do fixes and variants later anyway)

Test & Verification

  • Deinstalled the "mesa" and the LuaJIT port for cleaner test and install of new Portfile, port install worked like a charm.
  • Both "mesa" and "luajit" ports were installed as dependencies.
  • The PNG of the MT main menu is an example from one of several port install runs and proof of verification.

Amended in the new Profile to show "MT 5.6.1-MacPorts-rev2" with two hyphens around the "MacPorts" text solely for optical reasons.

NOTE 1: The text "MIT 5.6.1-" with the first hyphen stems from the upstream code and cannot be easily changed.

NOTE 2: The commit's funny name "@Zweihorresolve 2nd round from reneeotten (add 2) was a mishap from unwanted cut & paste apparently. Should have been "resolve 2nd round from reneeotten (add 2)" instead. Sorry.

My next PR will get commits with shorter names anyway. Learning by doing. Always happy to learn and improve.

Hope this helps.

🌤️

@Zweihorn
Copy link
Contributor Author

@reneeotten - There was the request to check the library bindings of the binaries:

% pwd
/Applications/MacPorts/minetest.app/Contents/MacOS

% otool -L minetest
minetest:
	/opt/local/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.13)
	/opt/local/lib/libIrrlichtMt.1.9.0.8.dylib (compatibility version 0.0.0, current version 1.9.0)
	/opt/local/lib/libzstd.1.dylib (compatibility version 1.0.0, current version 1.5.2)
	/System/Library/Frameworks/OpenAL.framework/Versions/A/OpenAL (compatibility version 1.0.0, current version 1.0.0)
	/opt/local/lib/libvorbisfile.3.dylib (compatibility version 7.0.0, current version 7.8.0)
	/opt/local/lib/libvorbis.0.dylib (compatibility version 5.0.0, current version 5.9.0)
	/opt/local/lib/libogg.0.dylib (compatibility version 9.0.0, current version 9.5.0)
	/opt/local/lib/libsqlite3.0.dylib (compatibility version 9.0.0, current version 9.6.0)
	/opt/local/lib/libluajit-5.1.2.dylib (compatibility version 2.1.0, current version 2.1.0)
	/opt/local/lib/libgmp.10.dylib (compatibility version 15.0.0, current version 15.1.0)
	/opt/local/lib/libjsoncpp.25.dylib (compatibility version 25.0.0, current version 1.9.5)
	/opt/local/lib/libfreetype.6.dylib (compatibility version 25.0.0, current version 25.3.0)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1775.118.101)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)
	/opt/local/lib/libiconv.2.dylib (compatibility version 9.0.0, current version 9.1.0)
	/opt/local/lib/libcurl.4.dylib (compatibility version 13.0.0, current version 13.0.0)
	/opt/local/lib/libncurses.6.dylib (compatibility version 6.0.0, current version 6.0.0)
	/opt/local/lib/libform.6.dylib (compatibility version 6.0.0, current version 6.0.0)
	/opt/local/lib/libleveldb.1.dylib (compatibility version 0.0.0, current version 0.0.0)
	/opt/local/lib/libspatialindex.6.dylib (compatibility version 6.0.0, current version 6.1.1)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 905.6.0)

% otool -L minetestserver
minetestserver:
	/opt/local/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.13)
	/opt/local/lib/libzstd.1.dylib (compatibility version 1.0.0, current version 1.5.2)
	/opt/local/lib/libsqlite3.0.dylib (compatibility version 9.0.0, current version 9.6.0)
	/opt/local/lib/libjsoncpp.25.dylib (compatibility version 25.0.0, current version 1.9.5)
	/opt/local/lib/libluajit-5.1.2.dylib (compatibility version 2.1.0, current version 2.1.0)
	/opt/local/lib/libgmp.10.dylib (compatibility version 15.0.0, current version 15.1.0)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1775.118.101)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)
	/opt/local/lib/libiconv.2.dylib (compatibility version 9.0.0, current version 9.1.0)
	/opt/local/lib/libncurses.6.dylib (compatibility version 6.0.0, current version 6.0.0)
	/opt/local/lib/libform.6.dylib (compatibility version 6.0.0, current version 6.0.0)
	/opt/local/lib/libleveldb.1.dylib (compatibility version 0.0.0, current version 0.0.0)
	/opt/local/lib/libspatialindex.6.dylib (compatibility version 6.0.0, current version 6.1.1)
	/opt/local/lib/libcurl.4.dylib (compatibility version 13.0.0, current version 13.0.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 905.6.0

Happy to be of some service.

Hope this helps.

🌻

games/minetest/Portfile Outdated Show resolved Hide resolved
application as new maintainer for the minetest port as laid down by the MacPorts Guide section 7.3.3

Delete change-minetest-maintainer.diff

+ according to PR conversation

Update Portfile

* new maintainer

* prove the `license` by more appropriate OSI indicaor

Update Profile rev 1

MT 5.6.1 _minetest_ with **improved Portfile** and **new maintainer**

* Portfile derived from legacy Portfile, however heavily improved
* updates and improvements to 'depends_lib-append'
* updates and improvements to 'configure.args-append'
* no changes to diffs from `files`
* new maintainer (Zweihorn)
* license `LGPL` with more appropriate OSI indicator

Ref https://trac.macports.org/ticket/66562
Ref #17080 and resolve

Update Profile acc to resolve

* should resolve 1st round from @reneeotten

* Closes: https://trac.macports.org/ticket/66408

* should resolve 2nd round from @reneeotten

* keep all the "path-style" dependencies for good reasons
* reintroduce "_DLUA_" options for greater specificity and improved compatibility with ports
* correct maintainer field
* rev 2 intentionally

should resolve 2nd round from @reneeotten (add 1)

addendum #1 to 2nd round of review

* keep doxygen path
* keep "all paths" he said
* stay at rev 2
* however rounds nor adds are revs

 @Zweihorresolve 2nd round from @reneeotten (add 2)

* maintainer late addition
* shows port `${revision}` to MT GUI head

Update Portfile

 * back to rev 1 he said
* expect a squash
@Zweihorn
Copy link
Contributor Author

Zweihorn commented Dec 30, 2022

IMO the final PR and commit both can wait until next year and would accept last remarks until then, I presume.

NOTE 1 - My first squash ever went wrong apparently. The result is unexpected and my summary and new text to the preliminary final commit got lost while using the GitHub Desktop tool on macOS for reasons unkown. Me not happy.

NOTE 2 & NB - Fun fact. My first (and only and last) ever Squash play in real life went utterly wrong too. Took me six weeks in hospital and I had to postpone for one year my planned technical work placement during university almost thirty years ago. Sweet old memories.

NOTE 3 - Agreed in advance that this is a non-technical issue and not relevant to the core content of the PR.

Please accept in advance my sincere apologies for any inconvenience.

Stay tuned until mid of January 2023 (ETA) earliest.
👀

@reneeotten reneeotten merged commit 878d316 into macports:master Dec 31, 2022
@reneeotten
Copy link
Contributor

thanks for the update @Zweihorn ; in the future please keep things concise and to the point 😉

@Zweihorn
Copy link
Contributor Author

thanks for the update @Zweihorn ; in the future please keep things concise and to the point 😉

Alea iacta est

@Zweihorn Zweihorn deleted the minetest-newmaintainer branch December 31, 2022 07:45
@Zweihorn
Copy link
Contributor Author

@reneeotten -THX

This was a good learning experience. Little me became a little maintainer. Three cheers on minetest @5.6.1_1 (active)

You gave me a little "birthday present" to the end of the year. "My precious ..."

@Zweihorn
Copy link
Contributor Author

Have a look at:

Have fun.

🌻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants