-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
port appimage to linuxdeployqt type2 #98
Conversation
There is an automated "continuous build" upload tool we can use easily: Specifically we need it in the travis integration, as described here: Normally that builds also appimages for pull requests, uploads to transfer.sh and links in the PR. |
46a0030
to
1247184
Compare
@probonopd
as can be seen in the travis log: https://api.travis-ci.org/v3/job/394609547/log.txt EDIT: the second line is just the quotes not showing in the txt file...the error is the first line |
Although extracting the appimage, the file is in the right place...so that's not a problem?
|
Thanks for you great work @LyzardKing. Two things should be changed: First, Possibly this is a Second, something pulls in You may need to use something like this. |
I fixed a bug in my build (I didn't change some hardcoded paths, so I believe it was looking at some libraries outside the appimage). |
Do I have to run linuxdeployqt -show-exclude-libs in travis? |
Yes, so that we can learn what might be going wrong in the linuxdeployqt that is being used. Looks like a bug to me given this... |
The ldd I'm using to pull extra libraries does have libc.so.6..so that might be the issue:
Is there a way to compare this with the list generated by linuxdeployqt? |
what I have is this: |
You are right, https://github.com/scribusproject/scribus/pull/98/files#diff-354f30a63fb0907d4ad57269548329e3R69 is likely causing this. Note that What happens if you remove that line and do
instead? Will it deploy the additional dependencies of |
So I don't have to manually copy with the ldd? |
Also, the first linuxdeployqt is without -appimage, right? that can be done only at the end.. |
Yes. That's the plan. I wonder what will happen :-)
Yes. Note that I updated the above script, there was a mistake. |
I pushed the changes...did I do it right? |
33e59a8
to
fdd7e51
Compare
@probonopd It ignores the -extra-plugins=iconengines/libqsvgicon.so |
d6a1560
to
de5bebd
Compare
.travis.yml
Outdated
mv appdir/usr/_tkinter.so appdir/usr/lib/; | ||
( cd appdir/usr/ ; ln -s ./lib/_tkinter.so . ); | ||
./squashfs-root/usr/bin/linuxdeployqt -bundle-non-qt-libs appdir/usr/lib/_tkinter.so; | ||
cp /opt/qt58/plugins/iconengines/libqsvgicon.so appdir/usr/plugins/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never manually copy stuff into the AppImage. This does not properly deploy its dependencies, and does not set the rpath. Instead, pass in -extra-qt-plugins
as described here: https://github.com/probonopd/linuxdeployqt/#adding-extra-qt-plugins
Ok, so I guess it's not a missing plugin, and the sed replacement used in the previous appimage script is no good since this is not a script... |
bd6d861
to
36146a6
Compare
@probonopd |
.travis.yml
Outdated
make -j"$NPROC"; | ||
make install; | ||
bash -ex ./AppImage-package/bundle.sh; | ||
cmake . -DCMAKE_INSTALL_PREFIX=/usr -DWANT_HUNSPELL=1 -DWITH_PODOFO=1 -DWANT_GRAPHICSMAGICK=1 -DWANT_DEBUG=0 -DWANT_SVNVERSION=0 -DWANT_GUI_LANG=en_US -DBUILD_APPIMAGE_BUNDLE=1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@probonopd
Here I added a BUILD_APPIMAGE setting..but I have no idea how it's supposed to work...
I should use this to change the build process, change the paths, and for example the updates integration...
in the paths file I used Q_OS_LINUX...should I use BUILD_APPIMAGE? and how
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is -DBUILD_APPIMAGE_BUNDLE=1
defined? I've never heard of it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I added it in the travis buid command, to then use it as a condition in the build process...
I need a way to select some options only if I'm building an appimage...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea how it's supposed to work
Well if you code something that even you have no idea how it's supposed to work, how should I? No offense, but maybe if you could explain what you want to do then I might be able to help or at least suggest alternative solutions.
Also I think an AppImage should be built for each run of the CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lyzardking's proposal was a not 100% complete patch that needed further work.
it was a best effort to help in a/ fixing the appimage to build again b/ get it to work correctly with python (iirc)
iirc, the -DWANT_APPIMAGE_BUNDLE
variable is probably needed to switch from relative to absolute paths for SHARE.
@probonopd If a docker image is the new solution to dependency issues, |
Yes, but how do you create and host a custom Docker container on Travis CI? I am doing it in https://gitlab.com/probono/scribus-ci-docker/.
Well, the point of AppImages is that they run on older systems too, not just the latest bleeding-edge ones. To achieve this, we generally recommend to build what goes into an AppImage on a build system no newer than the oldest still-supported distribution, which for Ubuntu currently is 14.04. A fallback option (which I am currently testing) if it is not possible to build on the oldest still-supported distribution is to bundle everything inside the AppImage, down to glibc and ld-linux. Then the resulting AppImage should run also on systems older than the build system. But again, it is a fallback.
Because I wanted to have a newer Qt and other dependencies than what is available for Ubuntu 14.04, and I didn't want to build everything from source. And because I wanted to experiment with an AppImage that bundles everything. Still, if we can make it work on 14.04 it's much better...! |
@probonopd |
That's awesome news 👍Why would you want to do that only for AppImage builds? If Scribus loads its resources from a path relative to itself, it will work both for AppImage and for "normal" installs... |
I guess that would be a question for @aoloe |
what do the other projects do? |
Inkscape has a configure option to switch on binreloc, which means relative paths will be used for everything. |
// ones inside the bundle, relative to the executable. | ||
#ifdef Q_OS_LINUX | ||
// Set the application name expliticly. | ||
QCoreApplication::setApplicationName("scribus"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the goal of this? Are you imagining occasions where the default of "executable name" is wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added that because the file configs were set in a different location every time I installed a new version.
I can't remember now (I told @aoloe in irc a while back) but I believe the folders were named scribus-<version>
or similar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this fixed it? It feels weird, I would love to have more details about this… @aoloe ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the details should be somewhere in irc...
The issue is that there was no application name set properly...
we could test this again if you want..
m_templateDir = QString("%1/share/scribus/templates/").arg(pathPtr); | ||
m_libDir = QString("%1/lib/scribus/").arg(pathPtr); | ||
m_pluginDir = QString("%1/lib/scribus/plugins/").arg(pathPtr); | ||
m_qmlDir = QString("%1/share/scribus/qml/").arg(pathPtr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the scribus
in the hardcoded paths there should use whatever C++ const is defined by the cmake variable TAG_VERSION
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why should we add a version number in the folders?
would that not just add a new folder on every update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is TAG_VERSION
defined as. By default it is an empty string, but whoever builds it can use it to be able to build different co-installable builds of scribus.
For example, for the Ubuntu PPA I'm providing 3 builds, 'scribus', 'scribus-ng' and 'scribus-trunk', all of them coinstallable. For example, in the latest TAG_VERSION
is -trunk
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why you would be interested in adding a "number", but I am. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I thought it was a value that was not empty by default...
how would I add the cmake value in the cpp code?
The paths are correct for a regular release, and except from the point I made as an inline comment, it wouldn't affect the Debian and Ubuntu packages. However, I notice that now that all of linux, macos and windows (these are the only platform scribus supports, right?) would be overriding compile-time consts with the value obtained at runtime. Which is probably fine, but at this point you should probably looking also at cleaning up cmake and stop bothering defining such paths there (and while at it, check that all the thoughts cmake is having are reflected in the runtime detection logic). |
Thanks for looking at it @mapreri |
if we are realistic, yes. |
So I guess this PR is useless now, if the builds are now happening on gitlab... |
can anybody have a look at https://gitlab.com/impagina/scribus-patched/compare/master...appimage ? it should implement:
|
Looks good to me 👍 |
submitted as https://bugs.scribus.net/view.php?id=15671 once this is sorted out, do you think i can close this request? |
Yes, I think so. Thanks for your work on this @aoloe |
As discussed on IRC
This PR uses the new linuxdeployqt for the appimage build.
Note that I disabled the irc part temporarily