Closed blu256 closed 3 years ago
blu256
commented
3 years ago 
Ray-V
commented
3 years ago SlackBuilds for Codeine, KlamAV, Komposé, KPlayer, twin-style-SUSE2
I assume these are apps you find useful, in which case I'm happy to add them, but I have no intention to add all packages just for the sake of 'completeness' - the script and maintenance would become unmanageable.
There are some issues to sort out.
I've made these changes locally to test them, so if you don't want to take this any further, I'll add them to your commits and add the builds for the 14.0.11 release.
[1]
Not in 14.0.10 release:
kplayer
twin-style-suse2
The emphasis for this repository is to build the current release, so won't be added before the 14.0.11 release.
[2]
For slack-desc, .po files are in translations/messages:
codeine
kplayer - see [6]
twin-style-suse2
[3]
Codeine has a man page, so add
-DMAN_INSTALL_DIR=$INSTALL_TDE/man
and
mangzip_fn
[4]
Cmake options:
I pull these out of the top level CMakeLists.txt so that they're more obvious and can be set to override the default options
klamav:
WITH_EMBEDDED_SQLITE
I would make that conditional on whether sqlite is installed
The cmake test for that is with pkg_search_module(), so I would do the same before cmake starts
[[ ! -s /usr/lib$LIBDIRSUFFIX/pkgconfig/sqlite3.pc ]] && EMB_SQL="ON"
and then add the cmake option
-DWITH_EMBEDDED_SQLITE=${EMB_SQL:-"OFF"}
and
-DBUILD_KLAMMAIL="ON"
kompose:
WITH_XCOMPOSITE
Set this on condition that xdamage and xcomposite are installed - unlikely not to be, but check anyway
[[ -s /usr/lib$LIBDIRSUFFIX/pkgconfig/xdamage.pc && -s /usr/lib$LIBDIRSUFFIX/pkgconfig/xcomposite.pc ]] && XCOMP="ON"
and
-DWITH_XCOMPOSITE=${XCOMP:-"OFF"}
[5]
Cmake '-DPLUGIN_INSTALL_DIR' option not required for:
klamav
kompose
[6]
Translations for kplayer:
LINGUAS hasn't been defined to enable selectively building translations, so let's add that via tde_conditional_add_project*() - pending upstream commit
.desktop files won't be localized as that requires a pre-build setup in the source.
Add after untar_fn in the SlackBuild
mkdir translations
mv po translations/messages
find doc -name CMakeLists.txt | xargs rm
find translations -name CMakeLists.txt | xargs rm
sed -i '/tde_conditional_add_subdirectory/d' CMakeLists.txt
sed -i 's|add_subdirectory( icons )|&\
\
\
##### other data\
\
tde_conditional_add_project_docs( BUILD_DOC )\
tde_conditional_add_project_translations( BUILD_TRANSLATIONS )|' CMakeLists.txt[7]
Localized documentation recorded for slack-desc
Add for codeine, kplayer, klamav, and kompose
ldocs="en";for Lang in $I18N;do [[ -d doc/$Lang ]] && ldocs="$ldocs $Lang";done
$PRGNAM: With help documentation for:
$PRGNAM: $ldocs
[8]
Non-Slackware packages
ClamAV is required for the klamav build. It's only needed for klamav so I would include it as part of the build, in a similar way to the required builds for inkscape.
Imlib2 is required for the kompose build and could be of more general use so add its own SlackBuild to the Misc list.
Plus notes that they are required dependencies.
blu256
commented
3 years ago Hello and thank you for reviewing my PR. I'm somewhat new to writing SlackBuilds so your notes will be very useful for me.
I've noticed that your review mostly covers the new SlackBuilds. But there are also these little fixes I included:
- Package tde-common-cmake instead of cloning common cmake repository (upstream change)
- Replace tqca-tls with tqca (upstream change)
- Enable SASL by default in tdebase (for SMTP in KMail)
Should I assume that they're good enough?
but I have no intention to add all packages just for the sake of 'completeness'
Can't say I totally agree with this point (somebody might actually need an application which nobody considered to be useful enough to package) but I can totally understand why you would want this. :)
I've made these changes locally to test them, so if you don't want to take this any further, I'll add them to your commits and add the builds for the 14.0.11 release.
I'm finished with this PR so if you have already made these changes locally you can push the appropriate commits.
Ray-V
commented
3 years ago I'm somewhat new to writing SlackBuilds so your notes will be very useful for me
Bear in mind that they're not standard SlackBuilds and need the supporting scripts to work.
somebody might actually need an application which nobody considered to be useful enough to package
Yes - and I'll add it if requested as I'll do for Codeine etc., just not the whole range of TDE apps.
But there are also these little fixes I included:
Which I wasn't aware of until I signed in to respond to the first part ..
WITH_SASL=ON
I haven't tried a build with this set, but see no reason why it wouldn't work if required. 'Required' being the point.
As mentioned above, I've pulled out the cmake options from the top level CMakeLists.txt and added and set them in the SlackBuild so they're readily visible.
This option is actually set OFF in the source for a default build.
The tdebase build has been proven with the options currently set and it would be impossible to satisfy everyone with a fixed set of options, so there will have to be some user choice.
So I'm more inclined to leave this for any user to change if they require it.
Replace tqca-tls with tqca
For the current R14.0.10, the tqca-tls plugin isn't a part of tqca, so this will have to wait for the R14.0.11 update.
Package tde-common-cmake instead of cloning common cmake repository.
Another 14.0.11 update.
I can see the need to add this to the package list for 14.0.11, but I'm not sure about removing it in get-source.sh for the development builds.
Having it as a package means that it will only be updated if a conscious decision is made to do so, whereas now it is updated when BUILD-TDE.sh is run.
Its common use means that any package build can require, or be affected by, an update to it, as the recent work on translations has demonstrated, and a default update will ensure that link is maintained.
blu256
commented
3 years ago Replace tqca-tls with tqca Package tde-common-cmake instead of cloning common cmake repository.
Those changes were needed to build some packages from the development version from Git though.
Having it as a package means that it will only be updated if a conscious decision is made to do so
Upstream considers the CMake macros stable enough and removing the cmake/ directory (submodule) from all packages and actually encouraging installing them as a package.
Do you want me to create a separate PR for these changes?
Ray-V
commented
3 years ago The main purpose of this repository is to provide a dialog based method for building the current TDE release - 14.0.10 at the moment.
Provision for a development build was added later and to-date has coexisted without problems with that primary build.
I'm not going to break the setup for 14.0.10 builds by adding tde-common-cmake or replacing tqca-tls, so for the moment, there might have to be some user intervention for development builds where they are required.
Those changes were needed to build some packages from the development version from Git though.
Which packages?
At the moment, the cmake pull for the development builds stiil works for me by storing tde-common-cmake to cmake. If it's not working for you, let's have a look at that in more detail.
If adding tqca to the build list, retaining the tqca-tls build option rather than replacing it, would solve your issues then that would be compatible with the current setup.
So no further PR is necessary and I anticipate that everything will be compatible again with the 14.0.11 release.
encouraging installing them as a package.
This refers to TDE build systems where cmake was included as a git submodule. Slackware isn't a part of that system and I include cmake as set up in the build scripts.
It looks as though a tde-common-cmake package might need to be added for the 14.0.11 release - depending on whether each package archive contains the cmake modules, let's wait and see.
By all means keep an eye on what is happening upstream, but don't take it too literally for Slackware where workarounds are often needed.
blu256
commented
3 years ago I'm not going to break the setup for 14.0.10 builds by adding tde-common-cmake or replacing tqca-tls, so for the moment, there might have to be some user intervention for development builds where they are required.
Okay, it makes sense as 14.0.10 is the current release and you focus on it.
If adding tqca to the build list, retaining the tqca-tls build option rather than replacing it, would solve your issues
This was needed to build tdenetwork from git. Providing both packages could be confusing, as tqca-tls is needed for 14.0.10 and tqca is not, and the vice-versa seems to be the case for the git versions (and the next release).
My job was to make some suggestions through this PR (and I'm glad if they were of any use), and it's up to you to accept or reject them. You do have more experience with these SlackBuilds and better what works for them.
Ray-V
commented
3 years ago Added for development builds,
commit 75a4ed744d - tqca
commit 37d055a8ef - codeine, klamav [*], imlib2, kompose, kplayer, twin-style-suse2
[*] includes option to build clamav if it's not installed.