Ref: https://fedoraproject.org/wiki/SIGs/KDE/Packaging/Requests

Looks like this is actually Qt-only (Qt 5, to be precise), so qt-reviews is the more appropriate tracker to put this on.

$ LANG=C wget -c http://cicku.me/drawpile.spec
--2014-04-25 17:10:22--  http://cicku.me/drawpile.spec
Resolving cicku.me (cicku.me)... 37.59.54.136
Connecting to cicku.me (cicku.me)|37.59.54.136|:80... connected.
HTTP request sent, awaiting response... 404 Not Found
2014-04-25 17:10:22 ERROR 404: Not Found.

You can assign this bug to you now :)

I will be back in the next week. Feel free to review it.

SPEC URL: http://misc.cicku.me/fedora/drawpile.spec
SRPM URL: http://misc.cicku.me/fedora/drawpile-0.8.6-1.fc21.src.rpm

Start Issues:
=======
1) update-desktop-database is invoked in %post and %postun if package contains
  desktop file(s) with a MimeType: entry.
  Note: desktop file(s) with MimeType entry in drawpile
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-
  database
2) Please add systemd stuff.
3) Report incorrect FSF address.
4) It have bundled libs. For example: src/client/ora/{qzip.cpp,zipreader.h,zipwriter.h} have statement: "This file is part of the QtGui module of the Qt Toolkit."
drawpile-0.8.6/src/server/{qcommandlineoption.cpp,qcommandlineoption.h,qcommandlineparser.cpp,qcommandlineparser.h} - "This file is part of the QtCore module of the Qt Toolkit."

Inquiry: https://github.com/callaa/Drawpile/issues/51

I will wait 0.9.0 release.

All issues addressed.

SPEC URL: http://misc.cicku.me/fedora/drawpile.spec
SRPM URL: http://misc.cicku.me/fedora/drawpile-0.9.0-1.fc21.src.rpm

ALL bundled files still here in tarball. You may try delete in %prep if you insist they are not needed.

(In reply to Pavel Alexeev (aka Pahan-Hubbitus) from comment #8)
> ALL bundled files still here in tarball. You may try delete in %prep if you
> insist they are not needed.

Sure:

SPEC URL: http://misc.cicku.me/fedora/drawpile.spec
SRPM URL: http://misc.cicku.me/fedora/drawpile-0.9.0-2.fc21.src.rpm

It must be similar to:
rm -rfv src/client/ora/zip*.h
rm -rfv src/client/ora/qzip.h
instead of:
rm -rfv ora/zip*.h
rm -rfv ora/qzip.h

Or just (full):
rm -rfv src/client/{ora/zip*.h,ora/qzip.*,widgets/colortriangle/plugin.cpp,widgets/qtcolortriangle.cpp,utils/kis_cubic_curve.cpp,widgets/colortriangle/plugin.cpp,widgets/qtcolortriangle.cpp,widgets/kis_curve_widget.cpp} rm -rfv src/server/qcommandline*

And then build failed with (http://koji.fedoraproject.org/koji/taskinfo?taskID=6841337):
+ /usr/bin/cmake -DCMAKE_C_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_CXX_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_Fortran_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr -DINCLUDE_INSTALL_DIR:PATH=/usr/include -DLIB_INSTALL_DIR:PATH=/usr/lib64 -DSYSCONF_INSTALL_DIR:PATH=/etc -DSHARE_INSTALL_PREFIX:PATH=/usr/share -DLIB_SUFFIX=64 -DBUILD_SHARED_LIBS:BOOL=ON -DCLIENT=ON -DSERVER=ON
CMake Error at src/client/CMakeLists.txt:195 (add_executable):
  Cannot find source file:

    utils/kis_cubic_curve.cpp

Files without copyrights for clarification:
1) ./src/client/widgets/dualcolorbutton/plugin.cpp
2) ./src/client/widgets/gradientslider/plugin.cpp
3) ./src/client/widgets/colorbutton/plugin.cpp
4) ./src/client/widgets/imageselector/plugin.cpp
5) ./src/client/widgets/brushpreview/plugin.cpp

Full delete command (correction):
rm -rfv src/client/{ora/zip*.h,ora/qzip.*,widgets/colortriangle/plugin.cpp,widgets/qtcolortriangle.cpp,utils/kis_cubic_curve.cpp,widgets/colortriangle/plugin.cpp,widgets/qtcolortriangle.cpp,widgets/kis_curve_widget.cpp} src/server/qcommandline*

(In reply to Pavel Alexeev (aka Pahan-Hubbitus) from comment #10)
> It must be similar to:
> rm -rfv src/client/ora/zip*.h
> rm -rfv src/client/ora/qzip.h
> instead of:
> rm -rfv ora/zip*.h
> rm -rfv ora/qzip.h

My bad, I was very busy and hadn't saved the changes before uploading.


> And then build failed with
> (http://koji.fedoraproject.org/koji/taskinfo?taskID=6841337):
> + /usr/bin/cmake -DCMAKE_C_FLAGS_RELEASE:STRING=-DNDEBUG
> -DCMAKE_CXX_FLAGS_RELEASE:STRING=-DNDEBUG
> -DCMAKE_Fortran_FLAGS_RELEASE:STRING=-DNDEBUG
> -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr
> -DINCLUDE_INSTALL_DIR:PATH=/usr/include -DLIB_INSTALL_DIR:PATH=/usr/lib64
> -DSYSCONF_INSTALL_DIR:PATH=/etc -DSHARE_INSTALL_PREFIX:PATH=/usr/share
> -DLIB_SUFFIX=64 -DBUILD_SHARED_LIBS:BOOL=ON -DCLIENT=ON -DSERVER=ON
> CMake Error at src/client/CMakeLists.txt:195 (add_executable):
>   Cannot find source file:
> 
>     utils/kis_cubic_curve.cpp
> 
> Files without copyrights for clarification:
> 1) ./src/client/widgets/dualcolorbutton/plugin.cpp
> 2) ./src/client/widgets/gradientslider/plugin.cpp
> 3) ./src/client/widgets/colorbutton/plugin.cpp
> 4) ./src/client/widgets/imageselector/plugin.cpp
> 5) ./src/client/widgets/brushpreview/plugin.cpp

Yes, because you are deleting source files. These files have no licensing issues. Don't get muddled.

Quoted(https://github.com/callaa/Drawpile/issues/51#issuecomment-42953986):

************************************************************
Here's a little more detail about the bundled files:

qcommandlineparser: safe to delete (used only for Qt 5.0 compatibility)
qzip: these are Qt internals, which are not exported. Applications must bundle them to use them directly.
kis_cubic_curve: this is taken from Krita's source code and is not a part of Qt or any library Drawpile depends on.
qtcolortriangle: this is not part of Qt itself, but from the (now defunct) Qt Solutions package.
I'll add the copyright headers to the plugin files. (The plugins are not normally installed. They are used only during development and must be installed manually if needed.)
************************************************************

Therefore I will only do this:

rm -rfv src/server/qcommandline*

SPEC URL: http://misc.cicku.me/fedora/drawpile.spec
SRPM URL: http://misc.cicku.me/fedora/drawpile-0.9.0-2.fc21.src.rpm

(In reply to Christopher Meng from comment #12)
> (In reply to Pavel Alexeev (aka Pahan-Hubbitus) from comment #10)
> Yes, because you are deleting source files. These files have no licensing
> issues. Don't get muddled.
> 
> Quoted(https://github.com/callaa/Drawpile/issues/51#issuecomment-42953986):
Yes, no licensing consideration at all. That files have bundling issue.
http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

I have seen author answer.

> ************************************************************
> Here's a little more detail about the bundled files:
> 
> qcommandlineparser: safe to delete (used only for Qt 5.0 compatibility)
Then delete.
> qzip: these are Qt internals, which are not exported. Applications must
> bundle them to use them directly.
You may work with upstream to export need symbols or request exception: http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Exceptions

> kis_cubic_curve: this is taken from Krita's source code and is not a part of
In this case Krita should be packaged, imported in Fedora and then appeared there as Dependency. If you are speak about calibra-krita it is already in Fedora and calligra-krita-libs should be used for shared linking.

Etc...

(In reply to Pavel Alexeev (aka Pahan-Hubbitus) from comment #13)
> (In reply to Christopher Meng from comment #12)
> > (In reply to Pavel Alexeev (aka Pahan-Hubbitus) from comment #10)
> > Yes, because you are deleting source files. These files have no licensing
> > issues. Don't get muddled.
> > 
> > Quoted(https://github.com/callaa/Drawpile/issues/51#issuecomment-42953986):
> Yes, no licensing consideration at all. That files have bundling issue.
> http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

No.

> I have seen author answer.
> 
> > ************************************************************
> > Here's a little more detail about the bundled files:
> > 
> > qcommandlineparser: safe to delete (used only for Qt 5.0 compatibility)
> Then delete.
> > qzip: these are Qt internals, which are not exported. Applications must
> > bundle them to use them directly.
> You may work with upstream to export need symbols or request exception:
> http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Exceptions

You should ask Qqt upstream but not me to do that. I can't control this.

> > kis_cubic_curve: this is taken from Krita's source code and is not a part of
> In this case Krita should be packaged, imported in Fedora and then appeared
> there as Dependency. If you are speak about calibra-krita it is already in
> Fedora and calligra-krita-libs should be used for shared linking.
> 
> Etc...

I don't think it's a bundled library, it's just a piece of code taken from other projects. Krita is already packaged, but I don't agree with your opinion of hacking and depending on calligra-krita-libs.

I don't want to waste any time here anymore, if you still consider that it has bundled libraries, I will close this review.

(In reply to Christopher Meng from comment #14)
> (In reply to Pavel Alexeev (aka Pahan-Hubbitus) from comment #13)
> > (In reply to Christopher Meng from comment #12)
> > > (In reply to Pavel Alexeev (aka Pahan-Hubbitus) from comment #10)
> > > Yes, because you are deleting source files. These files have no licensing
> > > issues. Don't get muddled.
> > > 
> > > Quoted(https://github.com/callaa/Drawpile/issues/51#issuecomment-42953986):
> > Yes, no licensing consideration at all. That files have bundling issue.
> > http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
> 
> No.
Why not??
> 
> > I have seen author answer.
> > 
> > > ************************************************************
> > > Here's a little more detail about the bundled files:
> > > 
> > > qcommandlineparser: safe to delete (used only for Qt 5.0 compatibility)
> > Then delete.
> > > qzip: these are Qt internals, which are not exported. Applications must
> > > bundle them to use them directly.
> > You may work with upstream to export need symbols or request exception:
> > http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Exceptions
> 
> You should ask Qqt upstream but not me to do that. I can't control this.
I shouldn't anything. If you prefer help author to do so you may be involved in that process.

> > > kis_cubic_curve: this is taken from Krita's source code and is not a part of
> > In this case Krita should be packaged, imported in Fedora and then appeared
> > there as Dependency. If you are speak about calibra-krita it is already in
> > Fedora and calligra-krita-libs should be used for shared linking.
> > 
> > Etc...
> 
> I don't think it's a bundled library, it's just a piece of code taken from
> other projects. Krita is already packaged, but I don't agree with your
> opinion of hacking and depending on calligra-krita-libs.

There nothing to agree or disagree with me. It is Fedora guidelines. I can't say I fully agree with it (but mostly yes). If you disagree with it, you may start process of changing it.

> I don't want to waste any time here anymore, if you still consider that it
> has bundled libraries, I will close this review.
Yes, I think. If you doubt I'm right in conclusion you may forward that question in ML of FPC ticket to resolve issue.

Krita is an application, not a library, so calling code from Krita a "bundled library" is a bit of a stretch, even if it happens to be somewhere in calligra-krita-libs. I don't think depending on Krita makes any sense for this package.

I'm also busy with building a Drawpile package. The current version 0.9.6 ships a bundled version of Qt-Color-Picker [1]. I was able to build a package thereof, it is available from a Copr repository [2]. The Drawpile package also provided there has been built without the bundled kf5-karchive, but after adding Qt-Color-Picker-devel to BuildRequires and removing the directory ./src/client/bundled I get an error message while trying to build Drawpile:

--------------------------------------------------------------

CMake Error at src/client/CMakeLists.txt:240 (add_executable):
  Cannot find source file:

    bundled/kis_curve_widget.cpp

--------------------------------------------------------------

After only removing ./src/client/bundled/QtColorPicker, it gets built. Well, the package still lacks of icon installation and systemd integration.

It is that mentioned Krita thing again... However, the Qt-Color-Picker package could be one step further to get Drawpile into Fedora. Would it be helpful to file a review ticket for this? If yes, any comments about the package name (capitalization etc.)?

[1] https://github.com/mbasaglia/Qt-Color-Picker
[2] https://copr.fedoraproject.org/coprs/mariobl/drawpile/

Since our bundling policy changed we may proceed without unbundling.

Christopehr, do you plan update package?

Can we close here? No response from requester since monthes, also to others of its requests.

Closing. Please feel free to reopen if you're still interested in this package or use it as a base for another review request.