Bug 253356 - devel/qt5-core: include/qt5/QtCore/qconfig-modules.h contains duplicated includes
Summary: devel/qt5-core: include/qt5/QtCore/qconfig-modules.h contains duplicated incl...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-kde (Team)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-08 20:24 UTC by nimaje+fbz
Modified: 2021-02-23 23:45 UTC (History)
2 users (show)

See Also:
tcberner: maintainer-feedback+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description nimaje+fbz 2021-02-08 20:24:03 UTC
All includes in include/qt5/QtCore/qconfig-modules.h are duplicated multiple times.

This probably has to do with pkg installation and upgrading as the number of duplicates varies (with over 20 for QtCore/modules/qconfig-core.h)
Comment 1 Tobias C. Berner freebsd_committer 2021-02-08 20:25:30 UTC
Moin moin 

I can confirm that the file also looks a bit shady on my system.  
We'll need to investigate why the cleanup during deinstallation seems to fail.


mfg Toibas
Comment 2 Adriaan de Groot freebsd_committer 2021-02-08 21:55:40 UTC
Let's take a look at what is in the package itself:

```
sqlite> select * from packages where origin = 'print/qt5-printsupport';
2048|print/qt5-printsupport|qt5-printsupport|5.15.2_1|Qt print support
sqlite> select * from pkg_script where package_id=2048;
2048|1|327
2048|7|317
sqlite> select * from script where script_id=327;
327|echo '#include <QtCore/modules/qconfig-printsupport.h>' >> /usr/local/include/qt5/QtCore/qconfig-modules.h
```

There we go, that's the install script. The type 7 script is deinstall, which removes the qconfig-printsupport.h line from qconfig-modules.


So if we install, deinstall the package, then the include line is removed.

Does pkg upgrade run deinstall / reinstall? No it does not. Doing an upgrade, or a reinstall, or an `install -f`, all run the **install** script again, but not the deinstall script (the right order would be deinstall, install, but even that is flaky). That is where the duplications come from.

If the install script was smarter -- e.g. doing `sort -u` or something on the `qconfig-modules.h` file -- then it would not produce duplicates. The install script is generated in `Mk/Uses/qt-dist.mk` as part of the *qt-post-install* target. We could add an extra line in there to do the `sort -u` in-place.
Comment 3 commit-hook freebsd_committer 2021-02-23 20:12:35 UTC
A commit references this bug:

Author: adridg
Date: Tue Feb 23 20:11:57 UTC 2021
New revision: 566427
URL: https://svnweb.freebsd.org/changeset/ports/566427

Log:
  Re-work Qt5 install- and deinstall scripts.

  Qt5 packages *together* manage a header file qconfig-modules.h which
  `#includes` other headers. A given Qt5 package may get a line in there,
  or it might not: that depends on package settings.

  On installing a package, add the line (if needed), on deinstall, remove
  the line (if it was there). There's a tricky case, too: upgrading
  a package that *had* a line, but no longer has one. That behaves like
  deinstall. Previously we used @postexec and @postunexec lines, but
  these were fragile in their own special way. See the linked PRs for
  examples -- or tcberner's comment that his qconfig-modules.h file
  *was* a screenful, and is now much shorter (and alphabetical, too!)

  An additional complication comes from needing to update qtchooser,
  *if* it is installed and *if* the package needs it. There was another
  set of @postexec lines for that.

  Migrate all the update logic to a shell script (there already was one
  for deinstall, now make it do triple-duty). From the template file,
  `pkg-change.in`, we generate a pkg-install and a pkg-deinstall
  script that does the right thing.

  PR:		253356 253360
  Reported by:	Nimaje on IRC
  Reviewed by:	tcberner

Changes:
  head/Mk/Uses/qt-dist.mk
  head/devel/qt5/files/pkg-change.in
  head/devel/qt5/files/pkg-deinstall.in