Bug 242905 - misc/qtchooser symlinks break users tools and qt5 deps
Summary: misc/qtchooser symlinks break users tools and qt5 deps
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-kde (Team)
URL: https://reviews.freebsd.org/D22991
Keywords:
Depends on: qtchooser-binaries
Blocks:
  Show dependency treegraph
 
Reported: 2019-12-27 11:10 UTC by grarpamp
Modified: 2020-07-19 21:45 UTC (History)
2 users (show)

See Also:
tcberner: maintainer-feedback+


Attachments
v1 (from the qt-5.14 branch of the kde@ repo) (18.00 KB, patch)
2020-01-01 17:11 UTC, Tobias C. Berner
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description grarpamp 2019-12-27 11:10:21 UTC
qtchooser seems to be less than ideal model...

It installs a bunch of symlinks that confuse
users other apps/tools into thinking that the
particular symlinked binary/module exists or
may do something useful, when in fact it does not.
That causes variety of problems with those apps...
commonly configure builders detecting optional
things, etc.

Repeat:

Install qtchooser...
ls /usr/local/lib/qt?/bin
Diff that list to all the qtchooser symlinks in...
ls -l /usr/local/bin | grep qtchooser
Pick and exec any unneeded or not present app from there and see the confusion ensue...
qmake: could not exec '/usr/local/lib/qt5/bin/qmake': No such file or directory
moc: could not exec '/usr/local/lib/qt5/bin/moc': No such file or directory
etc...


Fix options:

1) qtchooser should not install any symlinks at all.
Instead, whatever ports that want to use qtchooser should
install themselves into their respective /usr/local/lib/qt hiers,
then install their own respective symlinks to qtchooser in /usr/local/bin,
and then depend on qtchooser (now just offering its bin and config).

2) qtchooser could be deprecated from the dependency list
of all current ports since qt4 is gone... only qt5 exists.

3) Force users to manually install and maintain a bunch
of unneeded ports, dependencies, qt modules... just to
satisfy qtchooser's symlinks. This can cause more problems
when those ports get out of date, conflict, etc. This
option seems not an elegant minimal prospect.

4) The qt5 meta port should probably be made available as
a package anyway. This might make pulling in (3) above easier.

?) ?
Comment 1 Tobias C. Berner freebsd_committer 2019-12-27 11:32:59 UTC
Moin moin 

1) qtchooser should not install any symlinks at all.
It does not. it installs wrapper binaries that can switch to multiple different ones the file system. 
However, only creating the wrapper-bin when it is needed, might be possible, however, it already gets messy when looking forward to having again multiple Qt versions in the tree. The cleanest version would be to have an additional port per binary which installs the wrapper... but that would be >30 additional ports. Which does not seem to compare well to the benefit we would gain from it.

2) qtchooser could be deprecated from the dependency list
Qt6 is on the horizon; as long as it is not clear whether Qt5->Qt6 will be equally as incompatible as Qt4->Qt5, I would like to keep the current framework.

3) Force users to manually install and maintain a bunch of unneeded ports, dependencies, qt modules
Why would you need to satisfy them? If you need them, well, you obviously also need the package that includes them. Granted, it's un-nice to have wrapper binaries in path which miss the thing being wrapped. If you don't need them, then why install them?

4) The qt5 meta port should probably be made available as a package anyway. This might make pulling in (3) above easier.
I would love to only provide one Qt5 package instead of splitting it in multiples.. however, I would assume many people would not like that :)





mfg Tobias
Comment 2 grarpamp 2019-12-28 04:46:11 UTC
> (1) It does not ... install symlinks

The qtchooser *package* tarball as created and shipped
from the *port* does lay down symlinks, they are not
made from the pkg script, they come from the hier...
tar -tvf qtchooser-66.txz | grep '^l' | wc -l
      47

If upstream qtchooser is creating them,
the port could comment the symlink part out.

> only creating the wrapper-bin when it is needed

Maybe meaning... not creating the wrapper-bin aka qtchooser,
but the symlink.

> messy when looking forward to having again multiple Qt versions

The symlinks target and source are not versioned,
and they just point to qtchooser. So on install
different vers can test or overwrite.

On remove, the pkg would have to check if other
versions are still needing the link. I think the
pkg database can test if a file is managed.

If not that's out, if so it's a hack.

> additional port per binary which installs the wrapper
> that would be >30 additional ports.

Messy, true.


> (2) Qt6 is on the horizon

True.


> (3) Why would you need to satisfy them?

Users other tools/apps may test for their existance (some
filename on disk), or for execution, instead of for full function.
Further, the exit qtchooser throws for a broken symlink
is the same as a bare exec of some of the apps it links to.

$ not_on_disk 2>/dev/null ; echo $?
127
$ [ -e not_on_disk ] ; echo $?
1
$ qgltf ; echo $?
qgltf: could not exec '/usr/local/lib/qt5/bin/qgltf': No such file or directory
1
$ qmake > /dev/null ; echo $?
1
$ qmake --version > /dev/null ; echo $?
0

> un-nice to have wrapper binaries in path which miss the thing being wrapped.

Generally yes, there shouldn't be broken links or
broken exec installed.
Sure the links are preparatory for other packages,
it just seems like the sub packages should be add/del
their own links themselves.

> If you don't need them, then why install them?

Because in this situation, for users tool/app to
act a bit cleaner/obvious, user either have to remove
the broken symlink[s], or install the sub pkg[s].


> (4) I would love to only provide one Qt5 package instead of splitting it in multiples

That seems a different topic.

Was referring specifically to making this meta port ...
https://svnweb.freebsd.org/ports/head/devel/qt5
... produce its own meta package whose sole purpose
upon pkg-install is for pkg to then go fetch and
install all its underlying dep requirement packages...
today's 'multiples'.
Pkg tarball would not physically contain the other pkgs
in it, only call them as deps. It would not effect the
existing splitting, no change, other than qt5 metaport
now also appears as a pkg. Xorg and lots of other
metaports run packages like this.

The links in qt5 would still be subject to the
pkg-remove problem vs qt<N> as above.



On discussion balance I don't think the issue is critical,
users will still figure it out.
Whatever works or close it, cheers :)
Comment 3 Tobias C. Berner freebsd_committer 2019-12-31 21:06:41 UTC
I'll try to improve the situation with the upcoming Qt 5.14 update... no promises on though :P
Comment 4 Tobias C. Berner freebsd_committer 2020-01-01 17:11:09 UTC
Created attachment 210369 [details]
v1 (from the qt-5.14 branch of the kde@ repo)

Moin moin

attached is a WIP version of what we have now.


mfg Tobias
Comment 5 grarpamp 2020-01-02 00:10:22 UTC
Pretty smooth approach, no additional ports,
and shouldn't need ongoing port maintenance.

D22991 summary has typo: "qtcreator" 

For a fully-self-doc file operation echo,
consider using "rm -v" and "ln -sv" instead,
or add the QTCHOOSER var to the target echo
of create function.


For pkg-install and pkg-remove of...

- qtchooser itself: Should trigger only one of the
wrapper create or remove links operations depending
on whether is pkg-install or pkg-remove mode.
This could be done by wrapping the function calls
at the bottom with a test for any pkg mode...
if pkg AND name qtchooser ; call mode ; else call both for binaries

- Any of qt<n>-* binaries: For minimal changes to disk,
the wrapper might wish to only create or remove
the links of the respective binary involved.
This could be done with a test for respective
"basename" in the logic of both functions.

For the two pkg situations above, maybe it would work
to pass the pkg mode, and or the respective name of the
qt binary "basename", into the wrapper as environment
variables or as commandline arguments or getopts.

For qtchooser:
The needed pkg vars are probably already in the
environment, see "env".

For qt<n>-*, maybe not as simple:
(A) The port/pkg "name" might not be same as the file "basename",
and the name might change, have -devel versions, etc.
(B) There might be multiple files involved in one port/pkg.
If A or B are complex or fragile or might require maintenance
of file lists... then the disk change is likely not worth bothering,
at least it is contained to scope of qt.
Comment 6 grarpamp 2020-01-02 01:31:36 UTC
Lines 10~13:
Redundant, bin/qtchooser presence of lines 15~18 implies this.

Line 15 and 30:
The admin etc may have disabled the perms, and
we are only managing the hier not exec them,
so change both the -x to -f.

Line 50:
This appears to be anti clobber/problem test,
any type of inode may be there, so be wider...
a) change -f to -e
b) if test fails, emit the clue with else "ls -l ${target}"
Comment 7 Tobias C. Berner freebsd_committer 2020-01-02 07:26:05 UTC
(In reply to grarpamp from comment #5)
I intentionally kept it simple and decided to always look for all that are missing or too much. The diff should always just be the change of the "current" package -- so the churn should be the same. 
That way the shell script is easily readable, and no argument parsing is required.

mfg Tobias
Comment 8 Tobias C. Berner freebsd_committer 2020-01-02 07:28:16 UTC
(In reply to grarpamp from comment #6)

Could you do the review part in phabricator? :)

mfg tobias
Comment 9 commit-hook freebsd_committer 2020-01-25 20:20:03 UTC
A commit references this bug:

Author: tcberner
Date: Sat Jan 25 20:19:45 UTC 2020
New revision: 524129
URL: https://svnweb.freebsd.org/changeset/ports/524129

Log:
  misc/qtchooser: do not create unnecessary wrappers

  misc/qtchooser provides a wrapper binary bin/qtchooser, that when called with
  name foo, will lauch the corresponding binary from lib/qt${current_qt_version}/bin/foo.

  Previously qtchooser would install a list of 30-ish symlinks to itself automatically.
  Now we switch this around.

  qt-dist ports that define QT_BINARIES will now have a @postexec and @postunexec
  entry added to their plist to run the shell-script update-qtchooser-wrapper
  (installed by qtchooser).

  update-qtchooser-wrapper removes all symlinks to bin/qtcreator that have no
  corresponding binary in lib/qt*/bin, and readds links that are missing.

  Exp-run by:	antoine
  PR:		242905
  PR:		243443
  Reported by:	grarpamp@gmail.com
  Reviewed by:	adridg
  Differential Revision:	https://reviews.freebsd.org/D22991

Changes:
  head/Mk/Uses/qt-dist.mk
  head/comms/qt5-serialbus/Makefile
  head/devel/qt5-assistant/Makefile
  head/devel/qt5-buildtools/Makefile
  head/devel/qt5-core/Makefile
  head/devel/qt5-dbus/Makefile
  head/devel/qt5-designer/Makefile
  head/devel/qt5-help/Makefile
  head/devel/qt5-linguist/Makefile
  head/devel/qt5-linguisttools/Makefile
  head/devel/qt5-qdbus/Makefile
  head/devel/qt5-qdbusviewer/Makefile
  head/devel/qt5-qdoc/Makefile
  head/devel/qt5-qmake/Makefile
  head/devel/qt5-remoteobjects/Makefile
  head/devel/qt5-scxml/Makefile
  head/graphics/qt5-3d/Makefile
  head/graphics/qt5-pixeltool/Makefile
  head/graphics/qt5-wayland/Makefile
  head/misc/qtchooser/Makefile
  head/misc/qtchooser/files/patch-Makefile
  head/misc/qtchooser/files/update-qtchooser-wrapper.in
  head/misc/qtchooser/pkg-plist
  head/sysutils/qt5-qtdiag/Makefile
  head/sysutils/qt5-qtpaths/Makefile
  head/sysutils/qt5-qtplugininfo/Makefile
  head/textproc/qt5-xmlpatterns/Makefile
  head/www/qt5-webengine/Makefile
  head/x11/qt5-qev/Makefile
  head/x11-toolkits/qt5-declarative/Makefile
  head/x11-toolkits/qt5-gui/Makefile
  head/x11-toolkits/qt5-widgets/Makefile
Comment 10 grarpamp 2020-01-26 11:30:04 UTC
(Re comment #6)

These nits may have been missed
(while being too new to phab to use for this bug)...

Updated to the line numbers in...
https://svnweb.freebsd.org/ports/head/misc/qtchooser/files/update-qtchooser-wrapper.in?revision=524129&view=markup

Lines 17~20:
Redundant, path of QTCHOOSER presence test in
lines 22~25 will already prove this.

Line 22 and 39:
We are only physically managing the file hier, not
its perms or considering executing anything in it,
so change both of these -x to -f.

Line 59:
This appears to be an anti clobber/problem test,
wherein any type of inode may actually exist there,
so skip those too by changing -f to -e
and if test fails, emit a short debugging clue in some form...
else ; echo "$target exists..."  or  ls -ld ${target}

Line 41:
The 'break' saves up to a VERSION worth of stats,
but the line isn't necessary, and removing it may
offer more clarity that a full search was done.

Lines 30~32:
The intent of the -L filter upon the find output could
be made more clear by replacing it with adding the
equivalent argument to line 29...
find ... \! -regex "^${QTCHOOSER}$"



And more of a note...

Line 29 and 57:
-maxdepth currently filters nothing, but may serve as extra
depth synchronization sanity check in the script between
the /usr/local/bin and /usr/local/lib/qt5/bin hiers.
nb: qt* upstream might happen to do some things in future,
such as using a private BINDIR/subdir for some qt things,
or even putting under PREFIX yet outside of under BINDIR,
such as libexec, sbin, ... .
Comment 11 commit-hook freebsd_committer 2020-07-19 21:38:30 UTC
A commit references this bug:

Author: adridg
Date: Sun Jul 19 21:37:57 UTC 2020
New revision: 542599
URL: https://svnweb.freebsd.org/changeset/ports/542599

Log:
  Massage misc/qtchooser

  - expand documentation of the tool used to manage the links; while we
    have only Qt5 in the tree the whole thing is rather moot, it's good
    to tidy up before Qt6 shows up on the horizon.
  - update the documentation in the Makefile to reflect the current
    state of Uses/qt.mk

  I've bumped PORTREVISION because the package contents will change
  with the updated script, but there's no functional change at all.

  SVN hooks have forced me to remove svn:executable from the
  script file, but this doesn't change the resulting permissions
  on the installed executable and script.

  PR:		242905

Changes:
  head/misc/qtchooser/Makefile
  head/misc/qtchooser/files/update-qtchooser-wrapper.in
Comment 12 Adriaan de Groot freebsd_committer 2020-07-19 21:45:22 UTC
I've gone over the nits and have turned them into documentation in the script, but there's plenty of nits that I feel are opinion-based or purely stylistic in nature. For instance testing with -x means that the thing-being-linked-to **is** an executable, rather than just any file that lives in (versioned) bin/.

Closing this with "qtchooser seems to do well enough in the qt5-only time, and is now prepped for Qt6".