Instead of using "share" use "${BIN_INSTALL_DIR}/data" on Windows,
this is the location provided by QStandardPaths for GenericDataLocation
on Windows.
Details
Diff Detail
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage
+1 on the approach (given that I suggested it ;-)
kde-modules/KDEInstallDirs.cmake | ||
---|---|---|
493 | Should this use something like This seems to be the way to define vars that are relative to other vars, and it's already what's done for LIBEXEC on Windows, for instance, so I assume it works :-) |
This breaks KStars on Windows. I'm using the latest emerge/qt/etc.. on Windows 10. While this is indeed the correct behavior, it breaks the normal usage of:
QString filename = QStandardPaths::locate(QStandardPaths::DataLocation, fileToSearch);
On Linux, the location above are:
"~/.local/share/<APPNAME>", "/usr/local/share/<APPNAME>", "/usr/share/<APPNAME>"
e.g. /usr/share/kstars which is where the read-only data are stored. But now under Windows, the locations are:
"C:/Users/<USER>/AppData/Local/<APPNAME>", "C:/ProgramData/<APPNAME>", "<APPDIR>", "<APPDIR>/data"
In my emerge environment, it was searching for the file under $KDEROOT/bin and $KDEROOT/bin/data
KStars data was installed to $KDEROOT/bin/data/kstars. Now, the problem is that QStandardPaths::locate(QStandardPaths::DataLocation, fileToSearch); searches in $KDEROOT/bin/data and NOT $KDEROOT/bin/data/kstars and therefore fails to find any data files.
We use that function extensively throughout KStars, so what's the solution for this?
What does fileToSearch look like? Yes, QStandardPaths::locate(QStandardPaths::DataLocation) will just get you $KDEROOT/bin/data. You need to pass it the component name as well, i.e. QStandardPaths::locate(QStandardPaths::DataLocation, "kstars/mydatafile"), then it will look in the correct folder.
Feel free to check out kdevelop.git, and grep a bit for QStandardPaths::DataLocation to learn how it's supposed to be used.
We use that function extensively throughout KStars, so what's the solution for this?
So how is this suppose to work under Linux then?
QStandardPaths::locate(QStandardPaths::DataLocation, "kstars/mydatafile") will look in /usr/share/kstars and then look for "kstars/mydatafile", it will fail!
First, DataLocation is the deprecated name for AppLocalDataLocation. This means, it points to an app-specific directory (at least on Linux), so indeed passing "kstars/" after it is wrong.
One solution is GenericDataLocation + "kstars/myfile", that works for sure.
The other solution is DataLocation + "myfile", and installing the file in bin/data, NOT bin/data/kstars.
<APPDIR> is assumed to be "app-specific" already on Windows, so there's no need for a "/kstars" subdir in there.
For the record (already discussed on IRC), I was confusing DataLocation with GenericDataLocation. In KDevelop we're using GenericDataLocation + "kdevmanpage/foo", as you hinted. This works for us.
The other solution is DataLocation + "myfile", and installing the file in bin/data, NOT bin/data/kstars.
<APPDIR> is assumed to be "app-specific" already on Windows, so there's no need for a "/kstars" subdir in there.
We use the the following to install the data files:
install(FILES .... DESTINATION ${KDE_INSTALL_DATADIR}/kstars)
install(FILES .... DESTINATION ${KDE_INSTALL_DATADIR}/kstars/textures)
install(FILES .... DESTINATION ${KDE_INSTALL_DATADIR}/kstars/scripts)
So it's already appending kstars to $KDE_INSTALL_DATADIR, so you're suggesting we omit "kstars" under Windows?
Probably it's easier to use GenericDataLocation + "/kstars/" + file
Hurray after upgrading to ecm-5.24.0 I unsuspectingly ran into this.
Something like this is imo a very breaking change as it will require changes in software using kde frameworks. I'm sorry I missed this change before it was released, I would have argued against this. I'm trying to use frameworks as a somewhat stable base in gpg4win. I find that a change like this in a minor release is pretty massive and should have been optional with the old behavior as default.
I thought we learned the lesson that not using FHS layout under Windows is just inviting problems. At least we learned that in Gpg4win and just went back to a complete FHS installation layout. Moving everything into bin is just broken imo. Regardless of whether QStandardPaths is broken in the same way. Especially as I'm not only packaging KDE Software but other things like GTK or GnuPG which expect their data under share.
Now I have some translations under share/locale some under bin/locale... fun!
I'm reverting this change in a patch to gpg4win now. But this will probably break if other Frameworks expect this layout now. I hope though that Frameworks are flexible enough to handle this with QStandardpaths.
Can we make this optional / over-ridable? (Or is it and I'm failing to see the mechanism for overriding this)
I had to update my application to be compatible with this approach so I hope this does NOT get reverted.
I had to update my application to be compatible with this approach so I hope this does NOT get reverted.
Yes what's done (released) is done. After reading more doc I think i can just override it in our general CMake build command and hope that no one hardcordes the expectancy of the default location in some place.
Sorry for the first grumpy comment, I think your intention is, that if we didn't fix QStandardPaths we have to adapt to it's weird / brokenness on windows. But that does not mean I'm happy with it because as you can see from my comments, I think the installation layout "forced" by QStandardPaths is really wrong. QStandardPaths should be configurable at least because there is no "one size fits all layout" on windows. :-)
Regards
QStandardPaths looks into a number of locations (not just one), and more could be added, but to make the additional locations "configurable" rather than hardcoded, there was the idea (and even a patch floating around on the kde-windows list) to make it read additional paths from qt.conf. I'm still open to that idea, it's just that it didn't turn out to be necessary for KDE apps on Windows (kate and other people having chosen the solution in this patch).
"Moving everything into bin" is of course only the way it looks like when your bindir is "bin", the more common layout on Windows is that this is all at the root of the install prefix.