Page MenuHomeFreeBSD

devel/git-cola: revive port and use qt5
ClosedPublic

Authored by dch on May 29 2019, 11:15 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jun 4, 9:39 AM
Unknown Object (File)
Mon, Jun 3, 6:05 AM
Unknown Object (File)
Mon, Jun 3, 5:29 AM
Unknown Object (File)
May 21 2024, 11:24 AM
Unknown Object (File)
May 19 2024, 3:56 PM
Unknown Object (File)
May 15 2024, 6:17 AM
Unknown Object (File)
May 4 2024, 12:06 PM
Unknown Object (File)
Apr 30 2024, 10:30 PM
Subscribers
None

Details

Summary

revive port after r495962 qt4 deprecation

  • Tobias this needs a Qt person reviewing I have no idea what I am doing
  • I assume Jonathan is ok with taking maintainership as port was deleted
  • still needs symlinks tidied up from stage-qa check
Test Plan

poudriere 13.0-CURRENT amd64 ok, would be great if others can

check this

Diff Detail

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

Event Timeline

MOVED
11685 ↗(On Diff #58036)

^it needs to be repo-copied from the removed one.

devel/git-cola/Makefile
30 ↗(On Diff #58036)

^ don't you need to pass some flag so that translations are not built in the presence of gettext (i.e. unclean environments).

  • remove NLS option as the port has no build/config toggle
  • clean up dependencies & appease portlint
  • poudriere validation is on its way
dch marked an inline comment as done.Jun 9 2019, 11:11 AM
dch added inline comments.
MOVED
11685 ↗(On Diff #58036)

thanks, I assume I can only do that at commit time, not during phab diff creation?

I will test 11/12 i386/amd64 as soon as our build box becomes available, which will hopefully be tomorrow morning (local time).

MOVED
11685 ↗(On Diff #58036)

Very happy to see this port getting updated for Qt5! And yes, entirely happy to relinquish maintainership... I only know enough about the ports tree to be dangerous. :)

When I build this port I see the following Q/A warning:

====> Running Q/A tests (stage-qa)
Warning: Bad symlink '/usr/local/share/icons/hicolor/scalable/apps/git-cola.svg' pointing to an absolute pathname '/usr/local/share/git-cola/icons/git-cola.svg'
devel/git-cola/Makefile
28 ↗(On Diff #58429)

NLS

29 ↗(On Diff #58429)

This line is not necessary.

devel/git-cola/pkg-plist
202–217 ↗(On Diff #58429)

Do these need to be protected with %%NLS%%?

dch marked 2 inline comments as done.Jun 27 2019, 8:53 AM

@tcberner @jrm comments on gettext dependency vs NLS please, I'm not clear
what the best cause of action is.

devel/git-cola/Makefile
28 ↗(On Diff #58429)

Coming back to NLS & the gettext dependency:

  1. the port has no compile-time flags for this. Do I simply leave gettext as a standard USES=gettext? If I do so, portlint rightly complains with "consider adding an NLS knob...".

There are a number of KDS ports that do this, so I assume its not
completely forbidden.

As the port itself *can't* be built without the NLS libraries, I
don't know what makes most sense - leave the NLS files in, no
toggle/options, and ignore portlint?

30 ↗(On Diff #58036)

can you clarify? The app doesn't provide any build or compile-tyime flags to disable translations, is there something else I can/should do here?

devel/git-cola/pkg-plist
202–217 ↗(On Diff #58429)

TBH I am not sure, I am not familiar with NLS. There is no compile-time switches to disable creating these, so they're always generated.

If I build in poudriere, I see pkg-plist errors related to the language files.

===> Checking for items in pkg-plist which are not in STAGEDIR
Error: Missing: share/locale/cs/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/de/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/es/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/fr/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/hu/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/id_ID/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/it/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/ja/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/pl/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/pt_BR/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/ru/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/sv/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/tr_TR/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/uk/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/zh_CN/LC_MESSAGES/git-cola.mo
Error: Missing: share/locale/zh_TW/LC_MESSAGES/git-cola.mo
===> Error: Plist issues found.

If I add an NLS knob, NLS_USES makes sense (OPT_USES implies OPT exists) and everything works for me if I guard those files in pkg-plist with %%NLS%%. What @tcberner says makes sense though, i.e., in a unclean environment with gettext already installed, I would expect those translation files to be unconditionally installed and a pkg-plist error would result. To confirm this I temporarily added USES+=gettext, so that gettext is always installed and turned the NLS knob off and everything installed fine. I don't have time at the moment to dig into why this is the case, but maybe you can confirm that it makes sense (and I am not doing something stupid) given the application does not have any build or compile-time flags to disable the translations.

I'm running poudriere testport for 11/12 i386/amd64 with these changes now.

The poudriere testport matrix with the changes I described above all look good.

Here are the changes I made against above. There is probably a better way to share this diff, but I'm short on time.

jrm@storage2 /usr/ports [arcpatch-D20460|✚2] % git diff
diff --git a/devel/git-cola/Makefile b/devel/git-cola/Makefile
index d6e43b5ce3e5..cf90b7737503 100644
--- a/devel/git-cola/Makefile
+++ b/devel/git-cola/Makefile
@@ -25,8 +25,7 @@ NO_ARCH=      yes
 # NO_VENDOR_LIBS ensures we use ports QtPy not embedded version
 MAKE_ARGS=     prefix=${PREFIX} PYTHON=${PYTHON_CMD} NO_VENDOR_LIBS=1

-OPTIONS_DEFINE=        DOCS
-OPTIONS_DEFAULT=   DOCS
+OPTIONS_DEFINE=        DOCS NLS
 OPTIONS_SUB=       yes

 NLS_USES=      gettext
diff --git a/devel/git-cola/pkg-plist b/devel/git-cola/pkg-plist
index e338f4a91e30..92051b9ce4c7 100644
--- a/devel/git-cola/pkg-plist
+++ b/devel/git-cola/pkg-plist
@@ -199,22 +199,22 @@ share/icons/hicolor/scalable/apps/git-cola.svg
 %%DATADIR%%/lib/cola/widgets/text.py
 %%DATADIR%%/lib/cola/widgets/toolbar.py
 %%DATADIR%%/lib/cola/widgets/toolbarcmds.py
-share/locale/cs/LC_MESSAGES/git-cola.mo
-share/locale/de/LC_MESSAGES/git-cola.mo
-share/locale/es/LC_MESSAGES/git-cola.mo
-share/locale/fr/LC_MESSAGES/git-cola.mo
-share/locale/hu/LC_MESSAGES/git-cola.mo
-share/locale/id_ID/LC_MESSAGES/git-cola.mo
-share/locale/it/LC_MESSAGES/git-cola.mo
-share/locale/ja/LC_MESSAGES/git-cola.mo
-share/locale/pl/LC_MESSAGES/git-cola.mo
-share/locale/pt_BR/LC_MESSAGES/git-cola.mo
-share/locale/ru/LC_MESSAGES/git-cola.mo
-share/locale/sv/LC_MESSAGES/git-cola.mo
-share/locale/tr_TR/LC_MESSAGES/git-cola.mo
-share/locale/uk/LC_MESSAGES/git-cola.mo
-share/locale/zh_CN/LC_MESSAGES/git-cola.mo
-share/locale/zh_TW/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/cs/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/de/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/es/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/fr/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/hu/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/id_ID/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/it/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/ja/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/pl/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/pt_BR/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/ru/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/sv/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/tr_TR/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/uk/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/zh_CN/LC_MESSAGES/git-cola.mo
+%%NLS%%share/locale/zh_TW/LC_MESSAGES/git-cola.mo
 %%PORTDOCS%%%%DOCSDIR%%/git-cola.rst
 %%PORTDOCS%%%%DOCSDIR%%/git-dag.rst
 %%PORTDOCS%%%%DOCSDIR%%/hotkeys.html

I should clarify. Everything looks except the message about the bad symlink that Jonathan already mentioned.

dch marked 3 inline comments as done.

@jrm I think this is ready to land now, I'm far enough afk not to be able to commit it, would you mind doing the honours? I can't do svn mv from where I am going.

This revision is now accepted and ready to land.Jul 27 2019, 9:08 AM

No problem. I'll commit it within a few days.

devel/git-cola: Update to v3.4; Add NLS knob; patch link creation

@dch: Are you OK with these proposed changes?

This revision now requires review to proceed.Aug 16 2019, 6:15 PM

Add pkg-message to let users know that git-cola wants devel/git to be built with the GUI option on

dch@, since you are away, I think I will pull the trigger on this. I'm fairly comfortable that the latest changes are improvements. Let's get this out to users and if you have changes you want to make, you can always make them when you return.

One thing that is not quite ideal for users is that git-cola makes use of gitk, which is only installed with the git package when it's built with the GUI option on which is not the default. For now, I have just added a pkg-message to make users aware. git-cola seems to work well without gitk, but there are a few features missing.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 21 2019, 1:13 PM
This revision was automatically updated to reflect the committed changes.