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 25933
Build 24495: 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
11679

^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
11679

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
11679

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
29

NLS

30

This line is not necessary.

devel/git-cola/pkg-plist
203–218

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
29

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
203–218

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.Jun 27 2019, 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.Jun 28 2019, 10:24 AM

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

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

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

dch accepted this revision.Jul 27 2019, 9:08 AM
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
jrm added a comment.Jul 29 2019, 1:52 AM

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

jrm updated this revision to Diff 60900.Fri, Aug 16, 6:15 PM

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.Fri, Aug 16, 6:15 PM
jrm updated this revision to Diff 60932.Sat, Aug 17, 1:08 PM

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

jrm added a comment.Sat, Aug 17, 1:14 PM

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.