Skip to content
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

Fedora packaging of clementine #291

Closed
Clementine-Issue-Importer opened this issue Dec 6, 2013 · 32 comments
Closed

Fedora packaging of clementine #291

Clementine-Issue-Importer opened this issue Dec 6, 2013 · 32 comments

Comments

@Clementine-Issue-Importer

From oget.fedora on May 07, 2010 10:13:46

Hi all,
Thanks for the wonderful work. I have been using clementine, and only
clementine, for the last 2 months. I am very satisfied. I was missing my
good old amarok-1.4 badly:)

We are packaging clementine for Fedora: https://bugzilla.redhat.com/show_bug.cgi?id=583327 However we found that clementine bundles external libraries, such as
libqxt, libqtsingleapplication. Fedora doesn't allow bundled libraries with
applications. So I packaged these two, and libqtlockedfile, which is a
dependency of libqtsingleapplication. These have to go through our review
process which is not the fastest beast in the planet. So we are getting
there slowly.

What I would like to ask is, could you guys add a capabililty to your
CMakeLists that it detects and uses system libraries if they are available?
I am currently patching them out.

My second question is, I see that with 0.3, you bundled universalchardet
too. What is this library? Do I need to package this too?

Uhm, is this gonna go this way? We want to have clementine officially in
Fedora, but picking out these external libraries, packaging them separately
and reviewing them takes time. I hope we can catch up with you some day :)

Thanks for the excellent work!
Orcan

Original issue: http://code.google.com/p/clementine-player/issues/detail?id=291

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on May 07, 2010 04:58:38

Hey Orcan, thanks very much for doing this!

The versions of those libraries that are in Clementine have been patched quite a bit to get them to work properly. You
might run into problems if you make these separate libraries - if you use the official versions then Clementine won't
work, and if you use our versions then other apps won't work :)
I've added the patches we've made to those libraries to svn: http://code.google.com/p/clementine-player/source/browse/trunk/3rdparty/qtsingleapplication/qtsingleapplication.patch http://code.google.com/p/clementine-player/source/browse/trunk/3rdparty/qxt/media-keys.patch The qxt changes are to add support for media keys (play, stop, etc.) in the global shortcut library. I've included a
private Qt header in there too, so these probably wouldn't get accepted upstream.
The qtsingleapplication changes are twofold: there's an obscure compilation fix for cross-compilation with mingw in
release mode, which we could send upstream, but the other one is a compatibility-breaking API change.

We had a similar discussion while trying to get clementine into Ubuntu universe: https://lists.ubuntu.com/archives/ubuntu-motu/2010-April/006661.html The reply (it's not showing up properly on the archives for some reason) was that static linking was "ok" for now for
those two libraries, as they're quite small and not really used by anything else.

I'm not sure what we can really do about it - if you like I can rewrite the functionality we get from those two
libraries myself so we're not technically using external libraries any more :p

universalchardetect is from mozilla ( http://www.mozilla.org/projects/intl/detectorsrc.html ), we're not using it yet,
but we will be using it to automatically work out character encodings for ID3 tags.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on May 07, 2010 19:00:06

Hmm, apparently I haven't experienced any the breakage for linking to the system
libqxt and qtsingleapplication. Perhaps I didn't use what you are providing through
these libraries.

In any case, it is hard to convince package reviewers that the bundled libraries
cannot be handled by appropriate patches, but I'll do my best. Is it not possible to
not modify the existing functions of these libraries, but add new functions so that
the original API does not change?

How about universalchardetect? I understand that you didn't implement any of its
functionality but do you expect that it will get modifications?

@Clementine-Issue-Importer
Copy link
Author

From john.maguire on May 08, 2010 05:43:21

Universalchardet has already been modified to remove its dependencies on Mozilla. It's
not currently provided separately so the only solution is to bundle the source with
clementine.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 01, 2010 22:31:36

Hi,
Today I worked on wrapping qtsingleapplication and adding the methods that you are using. But you changed some private members, which means I had to reimplement them. Soon I found myself rewriting 80% of qtsingleapplication and I stopped. This is not the right way to go.

How about we add these new members, instead of replacing the existing ones, and submit this to upstream of qtsingleapplication? I really would like to see clementine in Fedora. It is awesome. But the policies do not allow it as is :(

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 04, 2010 21:14:36

Hi,
Now I see that you forked projectM too, with version 0.4. It is really hard to catch up with you folks, from a linux distribution contributor's point of view. Was there any attempt to upstream your patches? I really really want to have clementine spread widely in the linux world, but this continuous forking is making our lives so hard.

Why do we use and fork qtsingleapplication, qxt instead of kdelibs which provides us all these features conveniently? There are many kde folks in Fedora telling me to give up, but I don't want to. Can we be a little more friendly?

Best'
Orcan

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on July 05, 2010 00:08:19

I can probably change the qtsingleapplication patch to be backwards compatible if that'll help.

I emailed Peter Sperl about getting the projectM patches upstream. He seemed quite keen, but told me it would probably take a little while to merge them and to make a release. The patches are just bugfixes and new functions though so they should be backwards compatible if you just want to patch the fedora package.
The ubuntu packaging of projectM is pretty neat - it splits the large data files into another package: http://packages.ubuntu.com/lucid/libprojectm-data Clementine's ubuntu package depends on that and compiles with -DBUNDLE_PROJECTM_PRESETS=OFF, making the Clementine package much smaller.

I'm not a fan of depending on kdelibs. It's not a small package and bringing it in would upset the gnome users. I'm trying to avoid any dependencies on gnome/kde packages, hopefully making both camps believe that Clementine is made for their desktop ;)

Honestly I find Fedora's policy a bit unhelpful. From a developer's point of view it would have been easier for me to simply reinvent the wheel and rewrite the functionality of these packages myself - something which is obviously bad. Instead Clementine is being punished for reusing other components.
I know it's not your fault and you can't do anything about it though, so I'd like to do something to help. I really can't think of what to do though, short of disabling functionality in Clementine or rewriting bits of the 3rdparty libraries from scratch.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 05, 2010 09:46:25

I can probably change the qtsingleapplication patch to be backwards compatible if that'll help.

That will be a great start. I was thinking of doing this myself. However, this means patching both qtsingleapplication and clementine on every release, assuming that nokia does not accept our patches. They have really bad support for their libraries. It would be great if you could handle this by renaming/overloading functions.

I emailed Peter Sperl about getting the projectM patches upstream.

That's good news. However at this point, we are not sure if he will accept the patches as is, right? I see that you define new functions with presets. I can ping the projectM maintainer at Fedora to ask him to include these patches, but I need to be sure that the new API will not change after a release of projectM.

Let us summarize. What functionality will we lose if we use the standard versions of these libraries:

  • qtsingleapplication: "Open With" on the file browser, and passing filenames as arguments in the command line
  • qxt: Some multimedia key bindings for certain keyboards
  • projectM: presets by using -DBUNDLE_PROJECTM_PRESETS=OFF.

Is there anything else I am missing?

Thank you :)

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on July 10, 2010 08:41:06

Right, I've changed the qtsingleapplication patch so it should be source compatible with the original. If you let me know how to link against Fedora's one (whether it uses pkgconfig, or otherwise the names of the libraries) then I'll add the cmake option for you too.

  • qtsingleapplication: "Open With" on the file browser, and passing filenames as arguments in the command line

Controlling playback with the commandline as well (--pause, --play, etc.)

  • qxt: Some multimedia key bindings for certain keyboards

All keyboards I think. Unless you're using Gnome and selected "Use Gnome's shortcut keys" in the Preferences, in which case it won't use qxt at all.

  • projectM: presets by using -DBUNDLE_PROJECTM_PRESETS=OFF.

The projectM patches let you change the texture resolution and preset duration at runtime without having to restart the app.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 10, 2010 12:00:19

Thanks for the update!

Since qtsinglepplication uses qmake as the build system, we use a .prf file that takes place of a .pc file. This file is placed in $LIBDIR/qt4/mkspecs/features/qtsinglepplication.prf and has the following contents:

INCLUDEPATH *= $$QMAKE_INCDIR_QT/QtSolutions
DEPENDPATH *= $$QMAKE_INCDIR_QT/QtSolutions
LIBS *= -lQtSolutions_SingleApplication-2.6
QT *= network

I am not sure if cmake is capable of reading .prf files. I know the library name is ugly, but that's what we are given. Afaik, Opensuse uses the same .prf file and I want to keep stuff consistent across distributions. I also submitted this file upstream, but you know how responsive they are... :(

By the way, how am I supposed to use the libprojectM that comes with Fedora? You have these lines in your CMakeLists.txt

if(ENABLE_VISUALISATIONS)
add_subdirectory(3rdparty/libprojectm)
endif(ENABLE_VISUALISATIONS)

and your copy of libprojectm gets compiled in whenever I enable visualizations. There are also calls such as
projectm_->changeTextureSize(texture_size_);
projectm_->changePresetDuration(duration_);
in file src/visualisations/projectmvisualisation.cpp. How should these calls be replaced in order to use system libprojectM? Is it possible to wrap this code inside an #if block?

Thanks.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 10, 2010 12:02:31

I forgot to add that this
INCLUDEPATH *= $$QMAKE_INCDIR_QT/QtSolutions
evaluates to
/usr/include/QtSolutions/
in a standard installation.

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on July 10, 2010 12:09:15

I'd rather not #if those lines away and cripple the Fedora package of Clementine... can we apply my patches to Fedora's projectM?

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 10, 2010 12:35:23

I contacted our libprojectM package maintainer, and asked the include the patches. According to his response I'll enable or disable the visualizations.

It would be good to have on the clementine side to have a cmake flag, to use the system copy of libprojetM.

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on July 10, 2010 12:38:54

Ok yeah that's fine :) I'll add the flags in a moment...

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on July 10, 2010 14:06:32

Ok, you should now have -DUSE_SYSTEM_PROJECTM=ON and -DUSE_SYSTEM_QTSINGLEAPPLICATION=ON
I haven't tested them very well so let me know if they don't work :)

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 10, 2010 21:42:55

Thanks,
I have one more request. Can you do the same thing (adding cmake flags) with qtiocompressor? It uses
LIBS *= -lQtSolutions_IOCompressor-2.3

By the way, this one is not patched, right?

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on July 11, 2010 05:03:08

Correct - it's not patched, so I've made it use the system library be default if it finds one, otherwise it'll fall back on the one in 3rdparty.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 16, 2010 22:35:22

Hi,
Today I built clementine against the system libraries. We are almost there. I have a few minor issues:

  • The line in CMakeLists.txt
    pkg_check_modules(LIBPROJECTM projectM)
    needs to be
    pkg_check_modules(LIBPROJECTM libprojectM)

  • In the file dist/clementine.desktop
    The MimeType line needs to end with a semi-colon ";"
    This is required by the freedesktop standards. http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#value-types I also have a not-so-minor issue:

    We included your libprojectM patches in Fedora's libprojectM package. clementine compiles fine against it now. But when I select Visualizations from the menu, clementine crashes. I checked the backtrace. The crash occurs inside a function in ftgl library. I see that you don't compile the bundled sources against ftgl. Why? Are you aware of this bug? I am attaching the backtrace. I doubt that it is a bug in clementine though. I recompiled the libprojectM without ftgl support. Then I don't get any crash. Unfortunately I don't think we can drop the ftgl support from the Fedora package.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 16, 2010 22:38:44

Attachment: clementine-visual.crash.txt

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 16, 2010 23:26:12

Nevermind the crash. It is an unrelated Fedora packaging bug. I am getting that fixed.

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on July 17, 2010 04:57:05

I've just fixed those two issues.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 17, 2010 10:25:58

Great. So the last remaining library is qxt. Looking at the patches, I see there is no straightforward way to upstream them. Perhaps, can we modify the patches, in a way that they are upstreamable, yet we can inherit qxt classes and do our implementation independently?

I also see gmock in the 3rdparty/ directory. Since we don't have gmock in Fedora yet (it is pending a review), I am not building the tests. Is it easy to switch to system gmock, when we have it in Fedora?

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 17, 2010 19:33:07

Actually, the crash was not only a packaging bug. Let me explain, as sooner or later you will hit this bug.

Do you see the null pointer "face" in the above backtrace in function FTSize::CharSize()? That is because the font paths s.titleFontURL and s.menuFontURL are not set in ProjectMVisualisation::InitProjectM() in file src/visualisations/projectmvisualisation.cpp.

This font path is passed as _titlefontURL to Renderer::Renderer() constructor in file Renderer/Renderer.cpp of libprojectM. There is a
#ifdef USE_FTGL
clause in that constructor where the font path is passed to the ftgl library. Unfortunately, ftgl library blows when it sees a nonexisting font path.

Since you are not using ftgl, you didn't hit this bug before. But after your patches are included in libprojectM, you might get bug reports from distros which link projectM to ftgl. I wish projectM was using fontconfig so that we do not have to give absolute font paths.

I added a patch to the Fedora package which adds the lines
s.titleFontURL = "/usr/share/fonts/bitstream-vera/Vera.ttf";
s.menuFontURL = "/usr/share/fonts/bitstream-vera/VeraMono.ttf";
to src/visualisations/projectmvisualisation.cpp. But obviously this is a Fedora specific patch and you can't include it.

Just wanted to give you a heads up. (By the way, projectM with ftgl is a lot better and smoother than without.)

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on July 18, 2010 06:40:12

Ah yes I had actually encountered that before, my "solution" was turning off FTGL in libprojectM :) Clementine doesn't actually use libprojectM to draw text - the overlay is done by Qt - so I've bundled a blank font and made Clementine write it out to /tmp and pass the path to libprojectM. It's a bit nasty but it stops the crash when FTGL is on.

I think both the qxt patches are upstreamable. It looks like qxt is still being maintained too, so they might actually get accepted!

Switching to system gtest and gmock should be easy enough once the packages are there - I haven't patched them at all (they're actually svn:externals). But there's not much point in you building the tests when you make the package, so I'd leave them out.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 18, 2010 10:43:16

I have 1 good, 1 bad news.
The meta-modifier.patch is accepted by upstream: http://dev.libqxt.org/libqxt/changeset/1d7ab756c1e8 However media-keys.patch is not accepted as is, since it contains large amount of nokia code. Upstream qxt says the qt code that you borrowed is also wrong, and they need to reimplement this cleanly: http://dev.libqxt.org/libqxt/issue/75 I will include this patch in the Fedora qxt package until it is implemented properly by upstream qxt.

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 18, 2010 12:08:06

I made this patch for using system qxt. It is pretty much copied from what you did for libprojectM, qtsingleapplication etc. I tested it both with and without the USE_SYSTEM_QXT flag. Things seem to work as expected.

Please double check.

Attachment: clementine-system-qxt.patch

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on July 18, 2010 12:10:22

Sorry, here is the final patch

Attachment: clementine-system-qxt.patch

@Clementine-Issue-Importer
Copy link
Author

From davidsansome on July 24, 2010 05:36:33

Thanks, committed!

I've added another projectm patch that fixes visualisations on locales that use , as the decimal-separator, see issue #455 .

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on August 04, 2010 21:32:58

Hello,
We got our first clementine crash report: https://bugzilla.redhat.com/show_bug.cgi?id=618474 It looks to me like a bug in glibc. The user uses rawhide, the unstable Fedora branch. What is your take on it?

@Clementine-Issue-Importer
Copy link
Author

From john.maguire on August 04, 2010 23:36:44

It's issue #563 and it's fixed by r1569 :-)

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on August 06, 2010 08:14:22

Thanks, patch applied to our package and the user says it works. Now we got another crash report: https://bugzilla.redhat.com/show_bug.cgi?id=621913 Seems to me like the visualization destructor, but can be something else. Is this issue resolved?

@Clementine-Issue-Importer
Copy link
Author

From oget.fedora on August 07, 2010 09:19:48

I think we can close this ticket now since clementine is in Fedora. Thanks for your assistance.

I opened a new ticket ( issue #612 ) for the crash I mentioned in the previous comment.

@Clementine-Issue-Importer
Copy link
Author

From john.maguire on August 07, 2010 10:04:54

Cool :-)

Status: Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant