Page MenuHomeFreeBSD

devel/git-cola: revive port and use qt5
Needs ReviewPublic

Authored by dch on May 29 2019, 11:15 AM.

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

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24779
Build 23536: arc lint + arc unit

Event Timeline

dch created this revision.May 29 2019, 11:15 AM
tcberner added inline comments.May 29 2019, 11:21 AM
MOVED
11685

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

devel/git-cola/Makefile
31

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

dch updated this revision to Diff 58429.Jun 9 2019, 10:56 AM
  • 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

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

jrm added a comment.Jun 10 2019, 1:11 AM

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

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'
jrm added inline comments.Jun 10 2019, 6:01 PM
devel/git-cola/Makefile
28

NLS

29

This line is not necessary.

devel/git-cola/pkg-plist
202–217

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

dch marked 2 inline comments as done.Thu, Jun 27, 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

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?

31

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

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.

jrm added a comment.Thu, Jun 27, 12:56 PM

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.

jrm added a comment.Fri, Jun 28, 10:24 AM

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

jrm added a comment.Fri, Jun 28, 10:27 AM

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
jrm added a comment.Fri, Jun 28, 10:31 AM

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