> On April 22, 2012, 5:28 p.m., Matěj Laitl wrote: > > This makes sense, give me some time to test it and think about it. > > Ralf Engels wrote: > Thought about it? > Should we ship it?
The #include fixes are definitly right, including <taglib/something.h> completely circumvents CMake taglib detection. In fact, we do the same mistake in all the lastfm code that includes <lastfm/something> - that needs to be fixed, too. The strange part is the linking one - why does amarok binary needs to be linked against it? Shouldn't just amaroklib be linked against it? My resolution: a) include fixes: definitly Ship it! b) linking: don't ship, show the error you get while linking first. - Matěj ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104693/#review12799 ----------------------------------------------------------- On April 22, 2012, 5:25 p.m., Mathias Stephan Panzenböck wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104693/ > ----------------------------------------------------------- > > (Updated April 22, 2012, 5:25 p.m.) > > > Review request for Amarok. > > > Description > ------- > > I installed TagLib 1.8-GIT in $HOME so I can try the new features (see also > my other review request #101598). $HOME/bin is in $PATH and thus > "taglib-config" returns all the right settings. Still Amarok did not compile. > The reason is that it adds the taglib include directory to the list of > include directories, but then still accesses the taglib headers with > "#include <taglib/tstring.h>" in some places (instead of using "#include > <tstring.h>"). This is clearly wrong, but coincidentally works if taglib is > installed in /usr. I fixed that. > > For some reason which I can't explain I had to also add the taglib libs to > the amarok binary. I would have thought that adding it to amaroklib would be > enough, but then the linker complained. > > Maybe the patch isn't good like it is (the linking part), but something needs > to be done about this. > > > Diffs > ----- > > shared/tag_helpers/APETagHelper.h c9f23c7 > shared/tag_helpers/ASFTagHelper.h 41b58b2 > shared/tag_helpers/ID3v2TagHelper.h 1bbae70 > shared/tag_helpers/ID3v2TagHelper.cpp 27e0cf0 > shared/tag_helpers/MP4TagHelper.h 9a6beee > shared/tag_helpers/StringHelper.h 3f6b9b8 > shared/tag_helpers/VorbisCommentTagHelper.h 5c5b1bf > shared/tag_helpers/VorbisCommentTagHelper.cpp 1fbb437 > src/CMakeLists.txt 43bda90 > > Diff: http://git.reviewboard.kde.org/r/104693/diff/ > > > Testing > ------- > > > Thanks, > > Mathias Stephan Panzenböck > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
