Spec URL: http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec SRPM URL: http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-2.fc11.src.rpm Description: x11vnc is to X Window System what WinVNC is to Windows, i.e. a server which serves the current X Window System desktop via RFB (VNC) protocol to the user. Based on the ideas of x0rfbserver and on LibVNCServer, it has evolved into a versatile and performant while still easy to use program. Koji build successful: http://koji.fedoraproject.org/koji/taskinfo?taskID=1465614
You should add openssl-devel as BR, otherwise the package is built without SSL support. Quoting http://koji.fedoraproject.org/koji/getfile?taskID=1465616&name=build.log: checking for crypt... no checking for crypt in -lcrypt... yes checking for RAND_file_name in -lcrypto... no checking for SSL_library_init in -lssl... no configure: WARNING: ========================================================================== *** The openssl encryption library libssl.so was not found. *** An x11vnc built this way will not support SSL encryption. And, if I am not mistaken, your package suffers from the same problem as described under https://bugzilla.redhat.com/show_bug.cgi?id=439772, it is built using the internal minilzo instead of the ones made available by lzo-minilzo
How is this different from vino?
> You should add openssl-devel as BR, otherwise the package is built without SSL > support. Thank you. Added. > And, if I am not mistaken, your package suffers from the same problem as > described under https://bugzilla.redhat.com/show_bug.cgi?id=439772, it is > built using the internal minilzo instead of the ones made available by > lzo-minilzo Hmmmm. Yes, you are right again. I'm fix it. Now used system variant. (In reply to comment #2) > How is this different from vino? First, main difference - it is not DE related, minimum dependencies. Second... so, I'm not use Gnome at all and even wasn't try vino at all :( But x11vnc provide several advantages: 1) It's easy to start (It was first really working solutions, what I found when search how get access to running X11 session at home computer from work, when only SSH was available) 2) It is uses several useful patches like TurboVNC (VNC is very slow protocol, so, any speed improvements is highly appreciated), tightvnc-filetransfer, lzo and others.(In reply to comment #1) 3) It can be used to provide access not only first running X11 session, but really to any (to which auth files you can get access) 4) It can provide access even before X11 session starts, to GDM/KDM/XDM/CDE login screen for example ( http://www.karlrunge.com/x11vnc/faq.html#faq-display-manager ) It would be great, If you can try and compare its comprehensive. http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-3.fc11.src.rpm http://koji.fedoraproject.org/koji/taskinfo?taskID=1466147
> (In reply to comment #2) > > How is this different from vino? > First, main difference - it is not DE related, minimum dependencies. Second... > so, I'm not use Gnome at all and even wasn't try vino at all :( > and even if it would not be different... that would not be a problem, would it? just for the record, I am a big fan of x11vnc. I'm using it quite regularly since 2007 or so.
(In reply to comment #4) > just for the record, I am a big fan of x11vnc. I'm using it quite regularly > since 2007 or so. So, may be you are review it? :)
If no one else steps in a decent period of time, I'll try to. Unfortunately $DAYLIFE occupies almost all my time
(In reply to comment #6) > If no one else steps in a decent period of time, I'll try to. Unfortunately > $DAYLIFE occupies almost all my time If it is OK I step in and do the review within the next days.
I've roughly scanned over the package (http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-3.fc11.src.rpm ) and before I do a complete review please fix the following issues: - lots of rpmlint errors: * README/AUTHORS not UTF-8, please convert them in the %prep section via iconv (please make sure to preserve the timestamp, e.g. by using "touch -c -r old_file" * wrong permissions of some of the *.c files, please change the permission to 644 in the %prep section - the indentation seems to be broken - just open the file with vim or gedit - please delete the commented Packager: and Vendor: tags, it should be sufficient to add to your first changelog entry that this spec file is based on a version from Dag Wieers <dag@...> - delete the old non-Fedora changelog entries - correct the Source: line * use Source0 for the primary source file * for sf.net please use something like this: http://downloads.sourceforge.net/libvncserver/%{name}-%{version}.tar.gz * see also http://fedoraproject.org/wiki/Packaging:SourceURL for details * verify via "spectool -g x11vnc.spec" whether the sources can be downloaded using the provided URL - does not build in rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=1484978 - probably a missing build requirement since X11/extensions/XInput.h can't be found - check for spelling mistakes in the comments ;-) ("pathc") - the source as well as the built package contain some java clients: * only the server components are mentioned in the %description * if the clients should be shipped, then put them into separate sub-package * it is not allowed to use the pre-built JARs * preferably the pre-built JARs should not even be shipped in the sources (see http://fedoraproject.org/wiki/Packaging:Java for details) - "Requires: minilzo" is wrong, since there is no package "minilzo". Since x11vnc is linked against libminilzo.so.0, the dependency should be automatically added to the binary rpm: rpm --requires -qp ~rpmbuild/RPMS/i386/x11vnc-0.9.8-3.fc10.i386.rpm |grep lzo libminilzo.so.0 - would it be an option to use the system's libvncserver library instead of the internal one? (--use-system-libvncserver - when you upload a new version, please also provide also a link to the spec file - this makes it easier to just have a quick look at it
(In reply to bug 439772 comment #21) > I think the choice to make minilzo a shared lib is probably not really what the > authors of lzo or the consumer software intended. The idea was to have a one > file statically swallowed in build procedures w/o any further ties to the lzo > project. Now if the API/ABI of minilzo changes the other software is dependent > on being patched up to work with it. > > Just looking at how this issue is holding up a useful package for 6 months (!) > and the amount of patching one needs to remove the internal minilzo. I hardly > think that upstream will accept any of this patching (did anyone try to send > them upstream). Next updates of libvncserver/x11vnc will probably need to > adjust/extend the minilzo patching again. > > And all that for blindly adhering to guidelines? I'm sure if presented to the > FPC they would probably ack the use of minilzo as is. Fedora is currently even > more upstream closer than 6 months before, so chances are that upstream > proximity will win over other guidelines. > > And yes, I did have a check on how to remove minilzo in x11vnc and just see the > work involved. Trivial OTOH, but not a small workload to maintain on the other. (In reply to comment #3) > > And, if I am not mistaken, your package suffers from the same problem as > > described under https://bugzilla.redhat.com/show_bug.cgi?id=439772, it is > > built using the internal minilzo instead of the ones made available by > > lzo-minilzo > Hmmmm. Yes, you are right again. I'm fix it. Now used system variant. No, please don't. Check the discussion in the old bug. The embedded minilzo is a feature, not a bug. If you don't want the embedded *minilzo*, then you should use lzo, not the forced-shared-minilzo package. minilzo is by upstream's definition for embedded, statically linked applications.
(In reply to comment #9) > No, please don't. Check the discussion in the old bug. The embedded minilzo is > a feature, not a bug. If you don't want the embedded *minilzo*, then you should > use lzo, not the forced-shared-minilzo package. > > minilzo is by upstream's definition for embedded, statically linked > applications. Even if I also don't like the large patch to use the external (mini)lzo library, I still think it is the correct way: a) there are good reasons (security updates, ...) why the guidelines encourage not to embed system libraries: http://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries b) the work has already been done c) libvncserver also uses the system's libminilzo c) in other cases ( https://fedorahosted.org/fesco/ticket/134 ) FESCO did not approve the use of embedded libraries Given these arguments I'm convinced that using the embedded minilzo is not acceptable (the same applied for the embedded libvncserver).
I'm aware of the guidelines and libvncserver, still this is a different matter. The guidelines address the convenience copy of libraries carried in other projects. Here the minilzo is by designed to be embedded. If you really want shared lzo, you need to use the proper lzo library, minilzo was designed for a local static build. Upstream disagrees that minilzo is being used exposed in a shared lib way. If you want to patch the static lib away, OK, but using the forced-to-be-shared-minilzo is the wrong way, patch it to use the full lzo. The fact that libvncserver uses the same flawed approach is no excuse to repeat the same mistakes. In fact the minilzo subpackage should be banned, but if more and more packages use it, beacaue another package did, it will not be extractable anymore. Don't let me fool you - ask libvncserver & x11vnc upstream as well as lzo upstream to confirm or deny the above. BTW AFAIK libvnxserver still uses its internal minilzo. Not by choice, but because the patching proves to be a nightmare, see bug #439979. So on top of everything else, one creates a big unmaintainable patch, that will slow down any furture upstream upgrade as it will need months/years again to be retrofitted.
(In reply to comment #11) > I'm aware of the guidelines and libvncserver, still this is a different matter. > The guidelines address the convenience copy of libraries carried in other > projects. Here the minilzo is by designed to be embedded. I understand your point. But unfortunately the fedora guidelines provide also arguments for the opposite. ;-) Since the whole problem is not only related to this review, but to all packages which are either using an internal minilzo or already use an external minilzo shared lib, I recommend the following: a) Let's bring the issue to the attention of the Fedora Packaging Committee and discuss it on their mailing list. I'll sum up the issue by mail today or tomorrow and send it to FPC. b) At the same time let's continue with the review for now. I think only this specific issue can be put on hold for now until there is an agreement with FPC. All other remarks regarding the package are still valid and needs to be addressed.
(In reply to comment #8) > - lots of rpmlint errors: > * README/AUTHORS not UTF-8, please convert them in the %prep section via iconv > (please make sure to preserve the timestamp, e.g. by using "touch -c -r > old_file" > * wrong permissions of some of the *.c files, please change the permission to > 644 in the %prep section Excuse me - I don't understand you. *.c files it is sources and they only built and do not go in any packages. Why I shoud care about its permissions? And what mean "wrong" in this context? BTW rpmlint offcourse silent about it. > - the indentation seems to be broken - just open the file with vim or gedit No. Just I use tabsize = 5 spaces. > - please delete the commented Packager: and Vendor: tags, it should be > sufficient to add to your first changelog entry that this spec file is based on > a version from Dag Wieers <dag@...> > > - delete the old non-Fedora changelog entries Ok, I slightly modify it - add historical packagers in changelog, and clear it. > - correct the Source: line > * use Source0 for the primary source file Renamed. Is there any difference in it? I remember it may have sense in patches... in sources too? > * for sf.net please use something like this: > http://downloads.sourceforge.net/libvncserver/%{name}-%{version}.tar.gz > * see also http://fedoraproject.org/wiki/Packaging:SourceURL for details > * verify via "spectool -g x11vnc.spec" whether the sources can be downloaded > using the provided URL Sorry, it is historical URL, some time ago it was correct as i remember. Fixed. > - does not build in rawhide: > https://koji.fedoraproject.org/koji/taskinfo?taskID=1484978 - probably a > missing build requirement since X11/extensions/XInput.h can't be found Oops. I was built it only on Fedora 11, my mistake. Add BR: /usr/include/X11/extensions/XInput.h; In F12 it is located in libXi-devel but in previous versions in xorg-x11-proto-devel so, to do not make conditional requires, require explicit file. Now it built successful: http://koji.fedoraproject.org/koji/taskinfo?taskID=1487807 > - check for spelling mistakes in the comments ;-) ("pathc") I'm try, seriously. My English skills is very low. So do not hesitate point me on such errors too. > > - the source as well as the built package contain some java clients: > * only the server components are mentioned in the %description > * if the clients should be shipped, then put them into separate sub-package > * it is not allowed to use the pre-built JARs > * preferably the pre-built JARs should not even be shipped in the sources (see > http://fedoraproject.org/wiki/Packaging:Java for details) Off course! Such binaries removed in %install stage. > - "Requires: minilzo" is wrong, since there is no package "minilzo". Since > x11vnc is linked against libminilzo.so.0, the dependency should be > automatically added to the binary rpm: > rpm --requires -qp ~rpmbuild/RPMS/i386/x11vnc-0.9.8-3.fc10.i386.rpm |grep lzo > libminilzo.so.0 Fixed. > - would it be an option to use the system's libvncserver library instead of the > internal one? (--use-system-libvncserver This is very interesting question. I think it is will be "right" solution. Additionally it completely solve minilzo problem as I can understand. I try use it now (add --with-system-libvncserver). Add BR libvncserver-devel But I have errors ( http://koji.fedoraproject.org/koji/getfile?taskID=1487784&name=build.log ) several "undefined reference" like: /builddir/build/BUILD/x11vnc-0.9.8/x11vnc/connections.c:3161: undefined reference to `rfbUnregisterTightVNCFileTransferExtension' x11vnc-connections.o: In function `client_gone': /builddir/build/BUILD/x11vnc-0.9.8/x11vnc/connections.c:726: undefined reference to `rfbRegisterTightVNCFileTransferExtension' x11vnc-help.o:make[3]: Leaving directory `/builddir/build/BUILD/x11vnc-0.9.8/x11vnc' In function `print_help': /builddir/build/BUILD/x11vnc-0.9.8/x11vnc/help.c:4862: undefined reference to `rfbRegisterTightVNCFileTransferExtension' Disable this option now, but continue investigation there... > > - when you upload a new version, please also provide also a link to the spec > file - this makes it easier to just have a quick look at it I'm each time upload new spec-file with new src.rpm one. But as it is not versioned (have not version in name) I just omit it link each time, because it is present in initial report. Sorry for confusion. http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-4.fc11.src.rpm http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec
My 2 cent in embed/shared minilzo discussion. A also want see all libraries in system only in 1 exemplar (where it is possible off course) to have slender system where all on self places. And, one small note: (In reply to comment #10) > Even if I also don't like the large patch to use the external (mini)lzo > library, I still think it is the correct way: About what large patch you are speaking? There few string cuts, and size only to file deletion. I can delete it in spec by 1 command... or even this is not necessary! So I only ask: It there any real advantages to use static build? Speed? Size? For what we should try change existing guidelines? Why shared linking should be with full lzo, not minimalistic minilzo, if it is enough?
(In reply to comment #13) > > - does not build in rawhide: > > https://koji.fedoraproject.org/koji/taskinfo?taskID=1484978 - probably a > > missing build requirement since X11/extensions/XInput.h can't be found > Oops. I was built it only on Fedora 11, my mistake. > Add BR: /usr/include/X11/extensions/XInput.h; In F12 it is located in > libXi-devel but in previous versions in xorg-x11-proto-devel so, to do not make > conditional requires, require explicit file. Although technically your solution is correct, I think that performance wise it is better to use: %if 0%{?fedora} >= 12 BuildRequires: libXi-devel %else BuildRequires: xorg-x11-proto-devel %endif > > Now it built successful: > http://koji.fedoraproject.org/koji/taskinfo?taskID=1487807 http://koji.fedoraproject.org/koji/taskinfo?taskID=1487918 (F11) http://koji.fedoraproject.org/koji/taskinfo?taskID=1487913 (rawhide) http://koji.fedoraproject.org/koji/taskinfo?taskID=1487923 (EL-5) Mind that EL-5 build fails because of the lzo dependency ( http://koji.fedoraproject.org/koji/getfile?taskID=1487925&name=build.log ): ultra.c:11:21: error: minilzo.h: No such file or directory ultra.c: In function 'rfbSendOneRectEncodingUltra':
(In reply to comment #15) > Although technically your solution is correct, I think that performance wise it > is better to use: Yes, I known how make conditional BR, but it is what I try avoid there. And please note, this is only performance wise on build stage it is not related in any user installation of package. > > Now it built successful: > > http://koji.fedoraproject.org/koji/taskinfo?taskID=1487807 > http://koji.fedoraproject.org/koji/taskinfo?taskID=1487918 (F11) > http://koji.fedoraproject.org/koji/taskinfo?taskID=1487913 (rawhide) > http://koji.fedoraproject.org/koji/taskinfo?taskID=1487923 (EL-5) > > Mind that EL-5 build fails because of the lzo dependency ( > http://koji.fedoraproject.org/koji/getfile?taskID=1487925&name=build.log ): > ultra.c:11:21: error: minilzo.h: No such file or directory > ultra.c: In function 'rfbSendOneRectEncodingUltra': Epel lzo do not contain minilzo variant of package: $ rpm -qpl http://download.fedora.redhat.com/pub/epel/5/i386/lzo-devel-2.02-2.el5.1.i386.rpm /usr/include/lzo /usr/include/lzo/lzo1.h /usr/include/lzo/lzo1a.h /usr/include/lzo/lzo1b.h /usr/include/lzo/lzo1c.h /usr/include/lzo/lzo1f.h /usr/include/lzo/lzo1x.h /usr/include/lzo/lzo1y.h /usr/include/lzo/lzo1z.h /usr/include/lzo/lzo2a.h /usr/include/lzo/lzo_asm.h /usr/include/lzo/lzoconf.h /usr/include/lzo/lzodefs.h /usr/include/lzo/lzoutil.h /usr/lib/liblzo2.so /usr/share/doc/lzo-devel-2.02 /usr/share/doc/lzo-devel-2.02/LZO.FAQ /usr/share/doc/lzo-devel-2.02/LZO.TXT /usr/share/doc/lzo-devel-2.02/LZOAPI.TXT So, should we try patch it use lzo instead?
(In reply to comment #16) > (In reply to comment #15) > > Although technically your solution is correct, I think that performance wise it > > is better to use: > Yes, I known how make conditional BR, but it is what I try avoid there Just for my info: why do you want to avoid it ? > And > please note, this is only performance wise on build stage it is not related in > any user installation of package. You are correct. But why make the builders cry if the solution is so simple ? > Epel lzo do not contain minilzo variant of package: [...] > So, should we try patch it use lzo instead? Yes, pretty please.
(In reply to comment #17) > (In reply to comment #16) > > (In reply to comment #15) > > > Although technically your solution is correct, I think that performance wise it > > > is better to use: > > Yes, I known how make conditional BR, but it is what I try avoid there > Just for my info: why do you want to avoid it ? This is look ugly. I every time try use common solutions if it available. > > And > > please note, this is only performance wise on build stage it is not related in > > any user installation of package. > You are correct. But why make the builders cry if the solution is so simple ? Why cry??? If you build this package at home, its will be satisfied once. Most of builds I do on koji, there also no problem - plus few seconds... What problem you a inspired with it? > > > Epel lzo do not contain minilzo variant of package: > [...] > > So, should we try patch it use lzo instead? > Yes, pretty please. Yeh, ok, I'll try do it.
Works great on f10 at both sides and through ssh.
(In reply to comment #13) > > - would it be an option to use the system's libvncserver library instead of the > > internal one? (--use-system-libvncserver > This is very interesting question. I think it is will be "right" solution. > Additionally it completely solve minilzo problem as I can understand. This is bug #439979. The shared-minilzo patch there has broken --use-system-libvncserver. E.g. one guideline kills the other. If one argues that one needs a shared (mini)lzo, then one needs to continue the logic and require a shared libvncserver. (In reply to comment #18) > > > So, should we try patch it use lzo instead? > > Yes, pretty please. > Yeh, ok, I'll try do it. About a million thanks! And the same applies to libvncserver (actually most of the lzo calls are in the libvncserver part, if not all). So maybe you want to fix libvncserver first? Just for testing, you can build a static-minilzo libvncserver and see whether --use-system-libvncserver works, and whether there is any minilzo code used directly anymore by x11vnc. If not, then this part of the review is done and the shared-lzo patch would need to move completely to libvncserver.
I'm continue work to try link it with system libvncserver and have one more question: When (if) i can do it, we lose tightvnc-filetransfer addon, presented in this version, or it also present in system one?
(In reply to comment #20)> > This is bug #439979. The shared-minilzo patch there has broken > --use-system-libvncserver. I'll look to it. My patch to cut off minilzo from x11vnc work as expected I think. > E.g. one guideline kills the other. > If one argues that one needs a shared (mini)lzo, then one needs to continue the > logic and require a shared libvncserver. About what you speaking? What argues to what? I say what *both* library should be used as shared system libraries, and at this time work to do that (for libvncserver now). > (In reply to comment #18) > About a million thanks! And the same applies to libvncserver (actually most of > the lzo calls are in the libvncserver part, if not all). So maybe you want to > fix libvncserver first? As I say, off course I'll try do it. > Just for testing, you can build a static-minilzo libvncserver and see whether > --use-system-libvncserver works, and whether there is any minilzo code used > directly anymore by x11vnc. If not, then this part of the review is done and > the shared-lzo patch would need to move completely to libvncserver. In my patch to cutoff minilzo I compleately remove even files of it from sources before start process. So, it can be linked only with system (mini)lzo library. But now, as I want link it with system libvncserver it have not many sense...
Thanks for the new package. (In reply to comment #13) > (In reply to comment #8) > > - lots of rpmlint errors: > > * wrong permissions of some of the *.c files, please change the permission to > > 644 in the %prep section > Excuse me - I don't understand you. *.c files it is sources and they only built > and do not go in any packages. Why I shoud care about its permissions? And what > mean "wrong" in this context? BTW rpmlint offcourse silent about it. rpmlint RPMS/i386/x11vnc-debuginfo-0.9.8-3.fc10.i386.rpm x11vnc-debuginfo.i386: W: spurious-executable-perm /usr/src/debug/x11vnc-0.9.8/libvncserver/rfbregion.c x11vnc-debuginfo.i386: W: spurious-executable-perm /usr/src/debug/x11vnc-0.9.8/libvncserver/auth.c [...] > > - the indentation seems to be broken - just open the file with vim or gedit > No. Just I use tabsize = 5 spaces. But IMHO it looks bad for nearly all people since most others use tabwidth = 8. Just try to click on the link in your browser and the the lines will look somehow displaced. I suggest to convert it to spaces since that's a good compromise and it will look good for all people. ;-) > > - correct the Source: line > > * use Source0 for the primary source file > Renamed. Is there any difference in it? I remember it may have sense in > patches... in sources too? No, it's just a convenience / some kind of standard in Fedora. > > - check for spelling mistakes in the comments ;-) ("pathc") > I'm try, seriously. My English skills is very low. So do not hesitate point me > on such errors too. No problem - I'm also not an native English speaker - if in doubt just load the specfile into gedit and turn on the automatic spell checker and roughly scan for mistakes... ;-) > > - would it be an option to use the system's libvncserver library instead of the > > internal one? (--use-system-libvncserver > This is very interesting question. I think it is will be "right" solution. > Additionally it completely solve minilzo problem as I can understand. > I try use it now (add --with-system-libvncserver). Add BR libvncserver-devel > But I have errors ( > http://koji.fedoraproject.org/koji/getfile?taskID=1487784&name=build.log ) > several "undefined reference" like: > /builddir/build/BUILD/x11vnc-0.9.8/x11vnc/connections.c:3161: undefined > reference to `rfbUnregisterTightVNCFileTransferExtension' > x11vnc-connections.o: In function `client_gone': > /builddir/build/BUILD/x11vnc-0.9.8/x11vnc/connections.c:726: undefined > reference to `rfbRegisterTightVNCFileTransferExtension' > x11vnc-help.o:make[3]: Leaving directory > `/builddir/build/BUILD/x11vnc-0.9.8/x11vnc' > In function `print_help': > /builddir/build/BUILD/x11vnc-0.9.8/x11vnc/help.c:4862: undefined reference to > `rfbRegisterTightVNCFileTransferExtension' > > Disable this option now, but continue investigation there... I consider this more important than the lzo issue, since libvncserver is intended to be a shared library. When tightvnc filetransfer is disabled, x11vnc can be compilee without any problems: ./configure --with-system-libvncserver --without-tightvnc-filetransfer > > > Epel lzo do not contain minilzo variant of package: > > [...] > > > So, should we try patch it use lzo instead? > > Yes, pretty please. > Yeh, ok, I'll try do it. I agree this would be the best solution. If you like you can try to implement it using a similar configure option as for libvncserver. This patch could then be sent upstream and hopefully applied so that problem would be gone for further releases. > > > And > > > please note, this is only performance wise on build stage it is not related in > > > any user installation of package. > > You are correct. But why make the builders cry if the solution is so simple ? > Why cry??? If you build this package at home, its will be satisfied once. Most > of builds I do on koji, there also no problem - plus few seconds... > > What problem you a inspired with it? I've asked on #fedora-devel and I got the impression that file based build requires are strongly discouraged. The main reason is the increased build time. I think we must not only consider the increased build time for single developers but also the increased load on the koji machines. Two additional minor remarks: Please don't indent the whole for loop when changing file encoding. ;-) Use the macros consistently - one plain "rm" leaked in although you've used "%{__rm}" in all other places...
(In reply to comment #21) > I'm continue work to try link it with system libvncserver and have one more > question: When (if) i can do it, we lose tightvnc-filetransfer addon, presented > in this version, or it also present in system one? System's libvncserver has explicitely disabled tighvnc-filetransfer, too: http://cvs.fedoraproject.org/viewvc/rpms/libvncserver/devel/libvncserver.spec?revision=1.7&view=markup So if this feature would be needed later, libvncserver must enable it and later x11vnc can enable it as well. But for now I think it is OK to go without it...
(In reply to comment #24) > System's libvncserver has explicitely disabled tighvnc-filetransfer, too: > http://cvs.fedoraproject.org/viewvc/rpms/libvncserver/devel/libvncserver.spec?revision=1.7&view=markup As I can understand - to do not change license only: # NOTE: --with-tightvnc-filetransfer => GPLv2 So, as you see in my spec, I also add GPLv2 by this reason: # Some included components, like tightvnc-filetransfer not available on GPLv2+ License: GPLv2+ and GPLv2 And I think it is very usefull future in any case... > So if this feature would be needed later, libvncserver must enable it and later > x11vnc can enable it as well. But for now I think it is OK to go without it... Should I try request it on libvncserver if I want see it in x11vnc? It's a pity what we can't separate it in another subpackage and it have influence on licensing full package.
I just used this first time in my life and realized, there are many use cases for this and those affects how this should be packaged and how people installing and using it will see it. For now i think it splits to two: - console user wants to share her screen, initiated locally - remote support/administration over network, initiated remotely Former already has a .desktop file in package, so assuming that she knows what she's doing, that's tackled already. My use case was, that I had to give remote support to running host over ssh without local help. So I was a root and had to run it manually with correct options. The biggest trouble was the MIT cookie handling (with -auth switch). Also, this depends on two factors: - user has not logged in, thus the dm cookie must be used. - some user has logged in, the user's own cookie must be used. Finding those cookies from the filesystem took the most of my time and for some new admin it might be a point to ask for help. Cookie location is also dm dependent and I think it would help a lot if there would be some kind of script for this like: /usr/sbin/vnc-share-display [<display num>] which would figure out right options internally and could also take care of authentication/password settings and possibly give example commands to be run at client side to connect to that host (few clients, with and without ssh). IMO this would save countless hours of people's time. What do you think?
Juha Tuomala most times to export current session you should run x11vnc as regular user (you may do it over SSH too) without any parameters, if all environment variables correct (by default). So, it is just working. Off course you must care about firewall rules separately (or use -via option to tunnel it over SSH what is very easy and very useful). If you think what there may be more magic-helpers on any case, I think you should try speak about it with upstream author.
(In reply to comment #27) > most times to export current session you should run x11vnc as > regular user Yes, in that other use case. 'less times' is the other use case when there is nobody at console, or she's effectively (knows nothing) there. > If you think what there may be more magic-helpers on any case, I think you > should try speak about it with upstream author. Why? - we make initscripts and other distro-integration scipts ourselves. - file locations for those MIT cookies falls into distro's configuration. That would make the script unnecessary complex to cover all distros. IMO, that does not belong to upstream.
If it belongs to upstream it's debatable, but for sure it's just a "nice to have", not a mandatory element. But yes, I agree that a tool to automatically find the correct cookie in the [more or less rare] cases when you login as another user would be nice. Myself I had to chase for the correct cookie once or twice and it did take lots of time.
(In reply to comment #28) > Yes, in that other use case. 'less times' is the other use case when there is > nobody at console, or she's effectively (knows nothing) there. Off course. But what about login over ssh as root, became user (su to it) with its environment, and again, just starts x11vnc... Off course we can imagine many other cases where hunter on magic cookie file is needed, but in most of them I think you are known what you do... > Why? Because we must be closer to upstream as much as possible. http://fedoraproject.org/wiki/PackageMaintainers/WhyUpstream > - we make initscripts and other distro-integration scipts ourselves. Key there "distro-integration". So, init-scripts is distro dependent and Fedora specific in most of cases. > - file locations for those MIT cookies falls into distro's configuration. > That would make the script unnecessary complex to cover all distros. File locations for those MIT cookies is also standardized less or more, as I can understand. And include 3 or 4 additional paths in find process for most useful places is not problem. > IMO, that does not belong to upstream. IMHO, if it is useful future, it will be very cool in any case speak with upstream. So, if upstream do not want even speak about it, then we can continue discuss about include it as Fedora-specific part only.
(In reply to comment #30) > (In reply to comment #28) > Off course. But what about login over ssh as root, became user > (su to it) with its environment, and again, just starts x11vnc... Will that inherit the user's regular environment, but not the one that has been set up by xinit? I'd say it just gets the regular which is in this case useless. It won't give any cookie information. > Off course we can imagine many other cases where hunter on magic > cookie file is needed, but in most of them I think you are > known what you do... That's nonsense. I've been hacking Linux/OSS since -93 and spent few hours to solve that support case. If I would have seen a script name i suggested in rpm -ql x11vnc, i've run it and get help for client side command. I'd been done in 20min in such case. > Because we must be closer to upstream as much as possible. > http://fedoraproject.org/wiki/PackageMaintainers/WhyUpstream jada jada jada, everyone here knows that mantra. And we already stated here why it doesn't apply here. > File locations for those MIT cookies is also standardized less or more, as I > can understand. And include 3 or 4 additional paths in find process for most > useful places is not problem. Which is untrue. A misleading point here is, that x11vnc gives site url with FAQ to stdout, which actually helps you with those cookies, but that information, for example in kdm case is not the case in fedora. It just leads one to wrong place and waste time. I'm not saying that some particular person should come up with that script, I can draft it myself to get started with. What I'm saying, that we should make it inside Fedora. I'm also saying, that lack of it should not stop this review to be accepted, it can very well be added once it's working.
Even with --with-system-libvncserver --without-filetransfer x11vnc build failed :( /home/pasha/rpmbuildroot/BUILD/x11vnc-0.9.8/x11vnc/connections.c:3161: undefined reference to `rfbUnregisterTightVNCFileTransferExtension' x11vnc-connections.o: In function `client_gone': /home/pasha/rpmbuildroot/BUILD/x11vnc-0.9.8/x11vnc/connections.c:726: undefined reference to `rfbRegisterTightVNCFileTransferExtension' I dpn't known what to do now.
(In reply to comment #32) > Even with --with-system-libvncserver --without-filetransfer x11vnc build failed > :( ./configure --help seems to be a little bit misleading ;-) Please use the following line: ./configure --with-system-libvncserver --without-tightvnc-filetransfer
(In reply to comment #23) > rpmlint RPMS/i386/x11vnc-debuginfo-0.9.8-3.fc10.i386.rpm > x11vnc-debuginfo.i386: W: spurious-executable-perm > /usr/src/debug/x11vnc-0.9.8/libvncserver/rfbregion.c > x11vnc-debuginfo.i386: W: spurious-executable-perm > /usr/src/debug/x11vnc-0.9.8/libvncserver/auth.c > [...] Yes, I forgot -debuginfo package. Fix it now. > But IMHO it looks bad for nearly all people since most others use tabwidth = 8. > Just try to click on the link in your browser and the the lines will look > somehow displaced. I suggest to convert it to spaces since that's a good > compromise and it will look good for all people. ;-) It is more useful when standard size - you always see where tab used, and where not. I suggest you change settings. I think it is easy in any editor. > No, it's just a convenience / some kind of standard in Fedora. Can I read about similar things anywhere? > I've asked on #fedora-devel and I got the impression that file based build > requires are strongly discouraged. The main reason is the increased build time. > I think we must not only consider the increased build time for single > developers but also the increased load on the koji machines. This issue discussed many times in ml, f.e. https://lists.dulug.duke.edu/pipermail/rpm-devel/2004-October/000097.html but there no 1 thing. So, *correct* way in this case libXi-devel should provide xorg-x11-proto-devel as it is replaced them. Other is hacks. File-based BuildRequire is semed more beautiful and preferred for me. And, I believe what increased build time is very small to count it... > Use the macros consistently - one plain "rm" leaked in although you've used > "%{__rm}" in all other places... Ok. (In reply to comment #31) > That's nonsense. I've been hacking Linux/OSS since -93 and spent few > hours to solve that support case. If I would have seen a script name > i suggested in rpm -ql x11vnc, i've run it and get help for client > side command. I'd been done in 20min in such case. Does not FAQ helps you in your search? > jada jada jada, everyone here knows that mantra. And we already > stated here why it doesn't apply here. WHY?? I can't even imagine why we can't (try) be close to upstream. I try do it when it is possible. > I'm not saying that some particular person should come up with that script, > I can draft it myself to get started with. What I'm saying, that we should > make it inside Fedora. I'm also saying, that lack of it should not stop this > review to be accepted, it can very well be added once it's working. I'm not argue with you startup try doing it is more easy, useful, friendly. I am glad to see it... But why you do not want even try do it for all (in upstream), not only for Fedora? > (In reply to comment #32) > ./configure --help seems to be a little bit misleading ;-) > > Please use the following line: > ./configure --with-system-libvncserver --without-tightvnc-filetransfer Indeed! Thank you. What is very strange with epel. I make src.rpm on my Fedora 11 machin and than try build it on koji and its failed with very strange non-clear fasion. http://koji.fedoraproject.org/koji/taskinfo?taskID=1564059 - build.log is almost empty. http://koji.fedoraproject.org/koji/taskinfo?taskID=1564254 - build.log absolutely absent! Then, I pack src.rpm on CentOS, and its built sucessfull! http://koji.fedoraproject.org/koji/taskinfo?taskID=1564269 On dist-f11 it built without problems too - http://koji.fedoraproject.org/koji/taskinfo?taskID=1564385 http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-6.src.rpm http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec
(In reply to comment #34) > What is very strange with epel. I make src.rpm on my Fedora 11 machin and than > try build it on koji and its failed with very strange non-clear fasion. > http://koji.fedoraproject.org/koji/taskinfo?taskID=1564059 - build.log is > almost empty. check the end of http://koji.fedoraproject.org/koji/getfile?taskID=1564059&name=root.log
(In reply to comment #34) > > i suggested in rpm -ql x11vnc, i've run it and get help for client > > side command. I'd been done in 20min in such case. > Does not FAQ helps you in your search? FAQ misleads people to look those kdm cookies elsewhere. No, it does not help at all. > WHY?? I can't even imagine why we can't (try) be close to upstream. > I try do it when it is possible. Because this is distro specific how the kdm is configured with its file locations.
(In reply to comment #35) > check the end of > http://koji.fedoraproject.org/koji/getfile?taskID=1564059&name=root.log Ehh, it seams epel koji builders still not xz-lzma ready. I hope it will be done near. (In reply to comment #36) > Because this is distro specific how the kdm is configured with its file > locations. Where the problem add 2 or 3,5,10 additional paths to search??
(In reply to comment #37) > Where the problem add 2 or 3,5,10 additional paths to search?? So you're saying that i didn't spend couple hours for 5min job, just because this software is already perfect. Thank you, once that script is ready, I keep it myself.
Juha Tuomala please, do not take offense. Nothing personal. Script ready for Fedora? Good job. Can you offer it to upstream author? Do you want I will do it for you? Or just help? This in any case good start. Upstream developer himself could add in that additional paths, for example from user requests by mail or bugtracker.
(In reply to comment #34) > (In reply to comment #23) > > But IMHO it looks bad for nearly all people since most others use tabwidth = 8. > > Just try to click on the link in your browser and the the lines will look > > somehow displaced. I suggest to convert it to spaces since that's a good > > compromise and it will look good for all people. ;-) > It is more useful when standard size - you always see where tab used, and where > not. I suggest you change settings. I think it is easy in any editor. The spec file should be easily readable without any specific tab width settings. Please use either a standard tab width or convert it to spaces. > > No, it's just a convenience / some kind of standard in Fedora. > Can I read about similar things anywhere? I'm not aware of any explicit documentation which requires "Source0", however this is also some kind of standard in all Fedora packages. E.g. see the examples here: http://fedoraproject.org/wiki/Packaging/SourceURL > > I've asked on #fedora-devel and I got the impression that file based build > > requires are strongly discouraged. The main reason is the increased build time. > > I think we must not only consider the increased build time for single > > developers but also the increased load on the koji machines. > This issue discussed many times in ml, f.e. > https://lists.dulug.duke.edu/pipermail/rpm-devel/2004-October/000097.html but > there no 1 thing. > > So, *correct* way in this case libXi-devel should provide xorg-x11-proto-devel > as it is replaced them. Other is hacks. File-based BuildRequire is semed more > beautiful and preferred for me. > And, I believe what increased build time is very small to count it... Since this seems to be discouraged in Fedora, please don't do it. The guidelines don't explicitly forbid this, but at least for the Requires field it is discouraged: http://fedoraproject.org/wiki/Packaging/Guidelines#Requires > > Use the macros consistently - one plain "rm" leaked in although you've used > > "%{__rm}" in all other places... > Ok. There is one missing where your removed the prebuilt clients. Some more remarks: Would it be possible to link it against the regular liblzo even for the Fedora package? This would save us one condition. Additional if it would be possible to create a patch which would make this a compile option to switch between minilzo (which is designed to be internal) and external lzo then this patch would be hopefully acceptable for upstream. The part of the %prep section which changes the encoding is not correctly indented.
@FE-LEGAL There are 2 legal issues with this package: 1. pre-compiled binaries The upstream tarball [1] contains a bunch of pre-compiled java binaries: ./classes/ssl/VncViewer.jar ./classes/ssl/UltraViewerSSL.jar ./classes/ssl/src/VncViewer.jar ./classes/ssl/src/UltraViewerSSL.jar ./classes/ssl/src/SignedVncViewer.jar ./classes/ssl/src/VncViewerNOSSL.jar ./classes/ssl/src/SignedUltraViewerSSL.jar ./classes/ssl/SignedVncViewer.jar ./classes/ssl/VncViewerNOSSL.jar ./classes/ssl/SignedUltraViewerSSL.jar ./classes/VncViewer.jar ./classes/UltraViewerSSL.jar ./classes/SignedVncViewer.jar ./classes/VncViewerNOSSL.jar ./classes/SignedUltraViewerSSL.jar and ./ssl/src/tight/*.class ./ssl/src/ultra/*.class The binary package will contain neither pre-compiled nor newly-compiled java tools. The complete classes/ directory is deleted in the spec file in the %install section. Please advice, whether the pre-compiled java binaries can be distributed via the src.rpm or whether the upstream tarball should be re-packaged without these files. 2. License: Although the COPYING files indicates GPLv2+, there are a couple of source files which explicitly state GPLv2. Most of these files are in the unused libvncserver/ directory, so they can be ignored. However, the file x11vnc/x11vnc.c which used to build /usr/bin/x11vnc (included in the binary RPM) is licensed under GPLv2. This would lead to the conclusion that the whole package should be GPLv2. Please can you confirm this? Thank you very much in advance! [1] http://downloads.sourceforge.net/libvncserver/x11vnc-0.9.8.tar.gz
Christian Krause, 1) before you Juha Tuomala already pointed me what I forgot delete prebuilt binaries, and I was already remove that. So, nothing there, as you can see: $ rpm -qlp x11vnc-0.9.8-6.fc11.i686.rpm | grep -i jar $ 2) Ok, You are right, if main file x11vnc.c under GPLv2, all package must be under GPLv2. I change package license. BTW it is only review moment, not subject FE-legal, because both licenses is freedom and acceptable for Fedora!
(In reply to comment #40) > The spec file should be easily readable without any specific tab width > settings. Please use either a standard tab width or convert it to spaces. You are first reviewer what don't accept that. File is easy readable. This style of formating not covered any guidelines, as I can understand (kick me, if I wrong) and I want leave it as it is. > I'm not aware of any explicit documentation which requires "Source0", however > this is also some kind of standard in all Fedora packages. E.g. see the > examples here: > http://fedoraproject.org/wiki/Packaging/SourceURL Only examples? I'm make decision what this is some kind of maintainer preferables only such as using $RPM_BUILD_ROOT or %{buildroot}, rm or %{__rm} etc... > Since this seems to be discouraged in Fedora, please don't do it. The > guidelines don't explicitly forbid this, but at least for the Requires field it > is discouraged: http://fedoraproject.org/wiki/Packaging/Guidelines#Requires This covered small later: http://fedoraproject.org/wiki/Packaging/Guidelines#File_Dependencies Cite from it: Rpm gives you the ability to depend on files instead of packages. Whenever possible you should avoid file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin. Using file dependencies outside of those directories requires yum (and other depsolvers using the repomd format) to download and parse a large xml file looking for the dependency. Helping the depsolvers avoid this processing by depending on the package instead of the file saves our end users a lot of time. =====/ End cite ==== So, key there "end users". Small later in this guidelines you can also find useful example when it is preferable. Nothing there about BuildRequires! So I don't see what it would be discouraged. > > > Use the macros consistently - one plain "rm" leaked in although you've used > > > "%{__rm}" in all other places... > > Ok. > > There is one missing where your removed the prebuilt clients. Upps. Sorry, I just don't update spec. Now I also replace by macroses other things like mv, sed, ln... > > Some more remarks: > > Would it be possible to link it against the regular liblzo even for the Fedora > package? This would save us one condition. It is possible - http://koji.fedoraproject.org/koji/taskinfo?taskID=1579635 But it some sort of hack. Are you sure what we should do it? > Additional if it would be possible to create a patch which would make this a > compile option to switch between minilzo (which is designed to be internal) and > external lzo then this patch would be hopefully acceptable for upstream. There I agree with you - such option would be appreciated. But it requires some additional times, and I wasn't planing do that. May be in the future. > The part of the %prep section which changes the encoding is not correctly > indented. Hm... What exactly? Everything seems correctly intended for me. http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-7.fc11.src.rpm http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec
(In reply to comment #43) > (In reply to comment #40) > > The spec file should be easily readable without any specific tab width > > settings. Please use either a standard tab width or convert it to spaces. > You are first reviewer what don't accept that. File is easy readable. This > style of formating not covered any guidelines, as I can understand (kick me, if > I wrong) and I want leave it as it is. I don't want to kick anyone, but I agree with Christian, just click on the specfile you posted and it appears with standard tabsize 8 on my browser looking strange. > > Would it be possible to link it against the regular liblzo even for the Fedora > > package? This would save us one condition. > It is possible - http://koji.fedoraproject.org/koji/taskinfo?taskID=1579635 > But it some sort of hack. Are you sure what we should do it? That would be great, please do it! The lzo developer offers liblzo for shared builds and minilzo for embedded static builds. The current Fedora-only situation with a shared minilzo is awkward to say the least and should be removed. And if you get a patch to do the same with libvncserver you will win all my sympathy points and I will send you a truck full of good karma :) > > Additional if it would be possible to create a patch which would make this a > > compile option to switch between minilzo (which is designed to be internal) and > > external lzo then this patch would be hopefully acceptable for upstream. > There I agree with you - such option would be appreciated. But it requires some > additional times, and I wasn't planing do that. May be in the future.
%Name and %Group needs one more time... ehm tab.
(In reply to comment #44) > I don't want to kick anyone, but I agree with Christian, just click on the > specfile you posted and it appears with standard tabsize 8 on my browser > looking strange. Browser is not best spec-related tool, how you think? > > > Would it be possible to link it against the regular liblzo even for the Fedora > > > package? This would save us one condition. > > It is possible - http://koji.fedoraproject.org/koji/taskinfo?taskID=1579635 > > But it some sort of hack. Are you sure what we should do it? > > That would be great, please do it! The lzo developer offers liblzo for shared > builds and minilzo for embedded static builds. The current Fedora-only > situation with a shared minilzo is awkward to say the least and should be > removed. And if you get a patch to do the same with libvncserver you will win > all my sympathy points and I will send you a truck full of good karma :) Ok, ok, I done that. And I think for libvncserver patch is pretty same. Also x11vnc also shared linked with system libvncserver if you speak about them. Christian Krause, as there appeared patch which must be applied only for Fedora > 11 (this all troubles around separate X11 extensions into several packages) and I had introduce conditionals, I also replace file-based buildrequires, as you request before (I just don't known how omit it now :( ). http://koji.fedoraproject.org/koji/taskinfo?taskID=1592731 http://koji.fedoraproject.org/koji/taskinfo?taskID=1592750 http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-8.fc11.src.rpm
Looks like there are no outstanding issues for FE-Legal here, lifting it. If I'm wrong, just let me know. :)
(In reply to comment #47) > Looks like there are no outstanding issues for FE-Legal here, lifting it. > > If I'm wrong, just let me know. :) Yeah, the licensing issue should be solved. The only remaining issue I'm unsure is about shipping the pre-compiled binaries. Sure, they are not in the binary rpm, but they are still in the source rpm. Is this OK or is there any need to re-package the upstream tgz without them?
(In reply to comment #48) > The only remaining issue I'm unsure is about shipping the pre-compiled > binaries. Sure, they are not in the binary rpm, but they are still in the > source rpm. Is this OK or is there any need to re-package the upstream tgz > without them? Well, if you know the licensing for all of the pre-compiled binaries and you are 100% sure that we have permission to freely redistribute them without restriction, you can leave them in the tarball (and delete the files in %prep). If you're not 100% sure, make a new tarball that doesn't have them. Either way, you should ask upstream to consider removing the pre-built binaries from the source tarball.
Well, I got answer from Karl J. Runge. Since x11vnc 0.9.4 the full java source is available: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=500970 So, I think we can redistribute its in src.rpm and delete binaries at built time. Also I ask him to devide x11vnc to separate packages with viewer applets - he answer what not interested in doing that upstream. I think we may leave all as is now. Tom, thank you for the check and lifting FE-legal.
Pavel, thanks for asking upstream regarding this issue. Since it looks like that most of the issues are addressed, I'll do the complete review on the weekend. Please apologize the delay.
Pavel, Thanks for all your work on this package and also for your patience, here is now the full review of the package. In general it looks good, but there are still some issues left. * rpmlint: OK rpmlint SPECS/x11vnc.spec SRPMS/x11vnc-0.9.8-8.fc10.src.rpm RPMS/i386/x11vnc-* 3 packages and 1 specfiles checked; 0 errors, 0 warnings. * naming: OK - name matches upstream - spec file name matches package name * License: TODO 1. source files - OK The COPYING files contains the regular GPLv2. There are some source files released under GPLv2+ and there are also a couple of source files which explicitly state GPLv2. Most of these files are in the unused libvncserver/ directory, so they can be ignored. However, the file x11vnc/x11vnc.c which used to build /usr/bin/x11vnc (included in the binary RPM) is licensed under GPLv2. This would lead to the conclusion that the whole package should be GPLv2. - License tag: OK - matches actual License: OK - License file packaged: OK - OpenSSL exception: OK In theory GPL code can not be compiled against the openssl library. However, Karl Runge, the upstream author, has explicitely give the permission to do so" In addition, as a special exception, Karl J. Runge gives permission to link the code of its release of x11vnc with the OpenSSL project's "OpenSSL" library (or with modified versions of it that use the same license as the "OpenSSL" library), and distribute the linked executables. You must obey the GNU General Public License in all respects for all of the code used other than "OpenSSL". If you modify this file, you may extend this exception to your version of the file, but you are not obligated to do so. If you do not wish to do so, delete this exception statement from your version. " 2. binary files - TODO The upstream package ships some pre-compiled java binaries: x11vnc-0.9.8/classes/VncViewer.jar x11vnc-0.9.8/classes/ssl/VncViewer.jar x11vnc-0.9.8/classes/ssl/SignedVncViewer.jar x11vnc-0.9.8/classes/ssl/UltraViewerSSL.jar x11vnc-0.9.8/classes/ssl/SignedUltraViewerSSL.jar The jar files can be re-generated using the shipped sources (GPLv2). However, as Tom Callaway pointed out, only if we are 100% sure about the actual license of the binary files we may distribute it. I have checked their web page as well as the various README files in the package and I could not find any specific information regarding the status of the binaries. This means, that I'm not 100% sure about it and so I think we should re-package it: http://fedoraproject.org/wiki/Packaging:SourceURL#When_Upstream_uses_Prohibited_Code However, if Tom or somebody else from the FE-LEGAL team gives their explicit permission for redistribution I would be happily accept this. ;-) * specfile in American English and legible: TODO - indentation: I know this may sound like nitpicking, however I'm still convinced that the spec files should be written in such a way that it can be easily read by any other user with any program. It should not be necessary to guess the tab width for each spec file. ;-) I think using some kind of standard for things like this will certainly help that all Fedora package maintainers can easily work together. - typo: pathes -> paths - wording: IMHO "performant" isn't an Enlish word, probably just use "fast" - spelling: "prebuild clients" -> "prebuilt clients" - spelling: "acording" -> "according" - spelling: "macroses" -> "macros" - spelling: "arount" -> "around" - indentation in %prep: The small code snippet to do the conversion into UTF-8 is not indented correctly. The begin ("for file....") and the end ("done") of the for loop should be one tab more to the left than the body of the for loop. - please order the patches by number * %description: TODO (minor) - Although "x11vnc" is a name, I would start it with a capital "C" at the beginning of the sentence. - In the second sentence I think the comma can be omitted. * Sources: OK - Source0 URL ok - spectool -g x11vnc.spec works - sources matches upstream - md5sum: 13e41380fe9ba2581db180061d1cbd22 x11vnc-0.9.8.tar.gz * Patch0: TODO During the review I was wondering that x11vnc is neither linked against the lzo library nor the minilzo library. Indeed since we neither build or use libvncclient nor libvncserver from the x11vnc package, there is no need at all to take care of lzo here. Just to be on the save side I would still delete the minilzo.[ch] in the spec file, but I think the lzo patch as well as the usage of __ln_s/__sed can be omitted. * Patch1: OK * Patch2: OK * Compilation: TODO - the first 2 lines which exports the CFLAGS and the LDFLAGS should not be necessary Otherwise, the compilation is OK - package builds correctly in koji for F12, F11 and F10 - RPMOPTFLAGS used - parallel build supported via _smp_mflags * debuginfo sub-package: OK - non-empty - debuginfo file works together with gdb * BuildRequires: TODO - most likely lzo-devel can be omitted * Locales handling: OK (n/a) * shared/static libs, pkgconfig/header/*.la files: OK (n/a) * packages must own all directories: OK * packaged files and directories: TODO It looks like that /usr/share/x11vnc is packaged although it is empty. Please don't package it. * files not listed twice: OK * permissions of files: OK - %defattr used - final file permissions OK * %clean section: OK * macro usage: OK * code vs. content: OK (only code) * large documentation into sub-package: OK It may be debatable whether 1MB README file is too large, but since it describes the command line parameters in detail etc. I think it is quite useful even in the main package and since the size is not that big, it seems to be OK. * GUI application needs %{name}.desktop: OK * no directories owned which are already owned by other packages: OK * rm -rf %{buildroot} at the beginning of %{install}: OK * all filenames UTF8: OK * functional test: OK
(In reply to comment #52) > 2. binary files - TODO > The upstream package ships some pre-compiled java binaries: > x11vnc-0.9.8/classes/VncViewer.jar > x11vnc-0.9.8/classes/ssl/VncViewer.jar > x11vnc-0.9.8/classes/ssl/SignedVncViewer.jar > x11vnc-0.9.8/classes/ssl/UltraViewerSSL.jar > x11vnc-0.9.8/classes/ssl/SignedUltraViewerSSL.jar > > The jar files can be re-generated using the shipped sources (GPLv2). > However, as Tom Callaway pointed out, only if we are 100% sure about the > actual license of the binary files we may distribute it. > I have checked their web page as well as the various README files in the > package and I could not find any specific information regarding the status > of the binaries. This means, that I'm not 100% sure about it and so I think > we should re-package it: As I already say before, Karl Runge (upstream developer) answer me what there no non-free code, and all sources also included there. I have not check it. But have we any foundation to do not trust him?? > http://fedoraproject.org/wiki/Packaging:SourceURL#When_Upstream_uses_Prohibited_Code If you very want, off course I may repack it. > However, if Tom or somebody else from the FE-LEGAL team gives their explicit > permission for redistribution I would be happily accept this. ;-) Tom lifting FE-legal early :) > * specfile in American English and legible: TODO > - indentation: I know this may sound like nitpicking, however I'm still > convinced that the spec files should be written in such a way that it can be > easily read by any other user with any program. It should not be necessary to > guess the tab width for each spec file. ;-) I think using some kind of standard > for things like this will certainly help that all Fedora package maintainers > can easily work together. There was discussion on this theme - http://thread.gmane.org/gmane.linux.redhat.fedora.extras.packaging/6214/focus=6224 And accordingly it proposed draft guidelines change - https://fedoraproject.org/wiki/PackagingDrafts/Tabs So, there many people argue with your arguments. So, I think until it is only draft and FESCO do not approve that, it can't be as required item. > - typo: pathes -> paths > - wording: IMHO "performant" isn't an Enlish word, probably just use "fast" Sure? http://dictionary.reference.com/browse/performant But if you want, I replace it by productive. Is it ok? > - spelling: "prebuild clients" -> "prebuilt clients" > - spelling: "acording" -> "according" > - spelling: "macroses" -> "macros" There I don't known (replaced as you say). Macros have not multiple form at all? > - spelling: "arount" -> "around" Thank you. All fixed. > - indentation in %prep: > The small code snippet to do the conversion into UTF-8 is not indented > correctly. The begin ("for file....") and the end ("done") of the for loop > should be one tab more to the left than the body of the for loop. Again... I'm change it as you like see it for only do not start new long flame-war. But this is very similar to tab-width space. In Fedora we even not have (as I known) some recommended "coding standards" or similar documents/guidelines/policies. So, than it can't be error at all! > > - please order the patches by number Ok. > > * %description: TODO (minor) > - Although "x11vnc" is a name, I would start it with a capital "C" at the > beginning of the sentence. Capital "X" I think? I'm unsure. BTW, I reorder sentence, please see. > - In the second sentence I think the comma can be omitted. Ok. > * Patch0: TODO > During the review I was wondering that x11vnc is neither linked against the > lzo library nor the minilzo library. Indeed since we neither build or use > libvncclient nor libvncserver from the x11vnc package, there is no need at all > to take care of lzo here. > Just to be on the save side I would still delete the minilzo.[ch] in the > spec file, but I think the lzo patch as well as the usage of __ln_s/__sed can > be omitted. Yes, you are right. Today, after linking with system libvncserver. > * Compilation: TODO > - the first 2 lines which exports the CFLAGS and the LDFLAGS should not be > necessary Ok. > * BuildRequires: TODO > - most likely lzo-devel can be omitted Ok. > * packaged files and directories: TODO > It looks like that /usr/share/x11vnc is packaged although it is empty. > Please don't package it. Ok, excluded. http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-10.fc11.src.rpm http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec
At a minimum, you must rebuild those jars from source. Assuming that you have the source and are able to do that, I think you are safe to simply delete the bundled prebuilt jars during %prep, and do not need to make a custom tarball.
(In reply to comment #54) > At a minimum, you must rebuild those jars from source. Assuming that you have > the source and are able to do that, I think you are safe to simply delete the > bundled prebuilt jars during %prep, and do not need to make a custom tarball. Thanks for doing the final call. Completely fine with me and easier for packaging. ;-)
Thanks for the new package. There are just some smaller cosmetic changes left. (In reply to comment #53) > > * specfile in American English and legible: TODO > > - indentation: I know this may sound like nitpicking, however I'm still > > convinced that the spec files should be written in such a way that it can be > > easily read by any other user with any program. It should not be necessary to > > guess the tab width for each spec file. ;-) I think using some kind of standard > > for things like this will certainly help that all Fedora package maintainers > > can easily work together. > There was discussion on this theme - > http://thread.gmane.org/gmane.linux.redhat.fedora.extras.packaging/6214/focus=6224 > And accordingly it proposed draft guidelines change - > https://fedoraproject.org/wiki/PackagingDrafts/Tabs > > So, there many people argue with your arguments. So, I think until it is only > draft and FESCO do not approve that, it can't be as required item. Yes, I've also read this thread. However, it is for sure not wrong to use a standard tab width. And you would do nearly all other packagers the favor that they could easily read your spec file with a proper formatting. The alternative would be to use just spaces. This would work for everyone and since there is usually not that many changes in a spec file, it should not have that much overhead. Would you agree on this solution? > > - typo: pathes -> paths > > - wording: IMHO "performant" isn't an Enlish word, probably just use "fast" > Sure? http://dictionary.reference.com/browse/performant Not any more. ;-) I did some more research: In the mentioned URL the meaning of "performant" is described as "a performer" but not as an adjective with the meaning of "fast". However, just searching for "performant" with google revealed lots of people debating about it. ;-) Even if it is not in the standard dictionaries, it can be found here: http://www.urbandictionary.com/define.php?term=Performant > But if you want, I replace it by productive. Is it ok? Either way is fine with me by now. ;-) > > - spelling: "prebuild clients" -> "prebuilt clients" > > - spelling: "acording" -> "according" > > - spelling: "macroses" -> "macros" > There I don't known (replaced as you say). Macros have not multiple form at > all? Yes, "macros" is in widespread use. > > - indentation in %prep: > > The small code snippet to do the conversion into UTF-8 is not indented > > correctly. The begin ("for file....") and the end ("done") of the for loop > > should be one tab more to the left than the body of the for loop. > Again... > I'm change it as you like see it for only do not start new long flame-war. > But this is very similar to tab-width space. In Fedora we even not have (as I > known) some recommended "coding standards" or similar > documents/guidelines/policies. So, than it can't be error at all! I think basic indentation is a well-accepted coding standard for nearly all languages. Especially the inner blocks of constructs like "if () then ... else ..." or "for" loops should be indented one step more than the surrounding code. Here are the missing minor pieces: 1. tab width: it would really be nice if you could use either a tab width of 8 or just convert the spec file to spaces 2. According to Tom it is not necessary to re-package the tarball. However, he suggests that the pre-built binaries are deleted in the %prep section. My suggestion: - add "find -name '*.jar' -exec rm {} \;" to the end of the %prep section - add the attached patch to prevent that "make" will enter the "classes" directory 3. Please make sure that the comments in the spec file and the %changelog entries don't exceed 80 chars per line. Sure, that's also not strictly required but nearly all spec files do it this way. ;-)
Created attachment 358976 [details] patch to prevent building of the clients in the classes directory
(In reply to comment #56) > 2. According to Tom it is not necessary to re-package the tarball. However, he > suggests that the pre-built binaries are deleted in the %prep section. My > suggestion: > - add "find -name '*.jar' -exec rm {} \;" to the end of the %prep section > - add the attached patch to prevent that "make" will enter the "classes" > directory I don't think Tom implies to not have any java bits. Just don't use prebuilt ones as shipped. If the Makefile supports rebuilding the jars then perhaps all that is needed is to add some java build dependencies (and remove the java build targets to trigger the builds). The problem with prebuilt software is the lack of possible review and the possibility of a Trojan import in case the upstream project has been compromised. The shipment of jar files by the upstream authors is meant as a convenience, just use the source and rebuild the java support.
(In reply to comment #55) > (In reply to comment #54) > > At a minimum, you must rebuild those jars from source. Assuming that you have > > the source and are able to do that, I think you are safe to simply delete the > > bundled prebuilt jars during %prep, and do not need to make a custom tarball. > > Thanks for doing the final call. Completely fine with me and easier for > packaging. ;-) Hm... I must rebuild it before make decision what it may be excluded?? In this case, will be best solution include it also in package... Sic, it require additional time in any case.
(In reply to comment #58) > (In reply to comment #56) > I don't think Tom implies to not have any java bits. Just don't use prebuilt > ones as shipped. If the Makefile supports rebuilding the jars then perhaps all > that is needed is to add some java build dependencies (and remove the java > build targets to trigger the builds). Yes and no. ;-) 1. The java client doesn't build out of the box just by by the global "make". There are some manual steps necessary. However, the pre-built binaries would be installed when using "make install". 2. On the other hand, I was under the expression that we don't want to ship the clients at all. If this is the case, we must make sure that they don't go into the binary rpm. The safest way to achieve this is to delete them as early as possible in the %prep section. If we decide later to add them, we can just remove the patch and add probably some commands in the %build section to build them, too. (In reply to comment #59) > (In reply to comment #55) > > (In reply to comment #54) > > > At a minimum, you must rebuild those jars from source. Assuming that you have > > > the source and are able to do that, I think you are safe to simply delete the > > > bundled prebuilt jars during %prep, and do not need to make a custom tarball. > > > > Thanks for doing the final call. Completely fine with me and easier for > > packaging. ;-) > > Hm... I must rebuild it before make decision what it may be excluded?? In this > case, will be best solution include it also in package... Sic, it require > additional time in any case. I think there is a misunderstanding. If we don't want to ship the java client, we can just delete the binaries and we don't need to rebuild them. The only minor request I had was to move the deletion from the %install section to the %prep section to be on the safe side, that no any change in the build system of upstream would leak them accidentely in later...
(In reply to comment #60) > 1. The java client doesn't build out of the box just by by the global "make". > There are some manual steps necessary. Well, can't these manual steps be written in a specfile? > 2. On the other hand, I was under the expression that we don't want to ship the > clients at all. Whether the java applet is desired or not is a different question depending on its usefulness (the license question has already been checked). What is rather certain and would require an exception by the FPC otherwise is that if there are any java bits to be packaged they have to have been built by the package. So, it's either a) Rebuild the java support and the first step would be to remove the existing targets and add build support to the specfile or the upstream Makefile(s), or b) don't add any java support, but you don't need to actually delete them during the build if they are not used in packaging, removing the folders from the main Makefile is enough (like in a diff further up the report).
(In reply to comment #61) > Whether the java applet is desired or not is a different question depending on > its usefulness (the license question has already been checked). What is rather > certain and would require an exception by the FPC otherwise is that if there > are any java bits to be packaged they have to have been built by the package. I fully agree. It is up to the packager whether we wants to add the java client or not. I'm fine with either way. > So, it's either > > a) Rebuild the java support and the first step would be to remove the existing > targets and add build support to the specfile or the upstream Makefile(s), or Yes. > b) don't add any java support, but you don't need to actually delete them > during the build if they are not used in packaging, removing the folders from > the main Makefile is enough (like in a diff further up the report). I agree that this may be debatable. Personally think explicit deleting of the pre-built binaries in spec file in the %prep section will make it quite obvious for anyone who looks at the spec file later, that there is an issue with pre-built binaries which must be kept in mind. That's all. ;-)
Sorry for delay. As rebuild is requirement to ensure of all binaries born from longtime sources, I done this. And now, I think it good idea also include it. I pack it in subpackage. (In reply to comment #56) > Yes, I've also read this thread. However, it is for sure not wrong to use a > standard tab width. And you would do nearly all other packagers the favor that > they could easily read your spec file with a proper formatting. As you can read no single "all other packagers thing" about it question. > The alternative would be to use just spaces. This would work for everyone and > since there is usually not that many changes in a spec file, it should not have > that much overhead. Would you agree on this solution? No. Spaces off course always was second solution there, but (IMHO off course) add only mesh. The main reason of it - easy distinguish leaved space in formatting. But, you may wish do it. I think you favorite editor can do it. If not, I wrote little script for that on PHP: http://hubbitus.net.ru/rpm/Fedora11/x11vnc/tab-convert.phps Yo may use it like: $ cat x11vnc.spec | ./tab-convert.php > x11vnc.spec.spaces Or, with power of UNIX-WAY, off course directly in shell f.e: $ cat x11vnc.spec | php -r 'define("TAB_WIDTH", 5);foreach(file("php://stdin") as $l){preg_match_all("/\t+/", $l, $m, PREG_OFFSET_CAPTURE);foreach($m[0] as $mm){$l = str_replace($mm[0],str_repeat(" ", (TAB_WIDTH - ($mm[1] % TAB_WIDTH)) + ( TAB_WIDTH * ( strlen($mm[0]) - 1 ) )), $l);}echo $l;}' > x11vnc.spec.spaces > I think basic indentation is a well-accepted coding standard for nearly all > languages. Especially the inner blocks of constructs like "if () then ... else > ..." or "for" loops should be indented one step more than the surrounding code. May be you will surprised, but even in one language different projects has own "coding standards". > Here are the missing minor pieces: > > 1. tab width: it would really be nice if you could use either a tab width of 8 > or just convert the spec file to spaces I provide script and command before - with it you can easy convert tabs to spaces if you wish see it. > 2. According to Tom it is not necessary to re-package the tarball. However, he > suggests that the pre-built binaries are deleted in the %prep section. My > suggestion: > - add "find -name '*.jar' -exec rm {} \;" to the end of the %prep section > - add the attached patch to prevent that "make" will enter the "classes" > directory I add string to remove all prebuilt jars and built it again. So, I think there no more to discuss. > 3. Please make sure that the comments in the spec file and the %changelog > entries don't exceed 80 chars per line. Sure, that's also not strictly required > but nearly all spec files do it this way. ;-) As I remember rpmlint fire warning at one time if string exceeded 79 characters in description. But our days it is not (I especially check). And additionally I have for example Source0 string which is greater. So, it is not impossible in common case. Meantime I truncate some very long lines especially for you. http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-11.fc11.src.rpm
The license of the DES cipher, which goes into the jar files looks suspicious to me: THIS SOFTWARE IS NOT DESIGNED OR INTENDED FOR USE OR RESALE AS ON-LINE CONTROL EQUIPMENT IN HAZARDOUS ENVIRONMENTS REQUIRING FAIL-SAFE PERFORMANCE, SUCH AS IN THE OPERATION OF NUCLEAR FACILITIES, AIRCRAFT NAVIGATION OR COMMUNICATION SYSTEMS, AIR TRAFFIC CONTROL, DIRECT LIFE SUPPORT MACHINES, OR WEAPONS SYSTEMS, IN WHICH THE FAILURE OF THE SOFTWARE COULD LEAD DIRECTLY TO DEATH, PERSONAL INJURY, OR SEVERE PHYSICAL OR ENVIRONMENTAL DAMAGE ("HIGH RISK ACTIVITIES"). WIDGET WORKSHOP SPECIFICALLY DISCLAIMS ANY EXPRESS OR IMPLIED WARRANTY OF FITNESS FOR HIGH RISK ACTIVITIES. This might be nonfree. I am blocking FE-Legal. For references, please see the files: classes/ssl/src/ultra/README classes/ssl/src/ultra/DesCipher.java classes/ssl/src/tight/README classes/ssl/src/tight/DesCipher.java The full license statement can be found at the bottom of the above 2 README files. If FE-Legal decides that the DES cipher code is nonfree, these files (the corresponding java and jar files) cannot go into the SRPM.
It's not non-free, because of how it is worded. You may remember that we had a similar license from Sun which was non-free, because it said: "You acknowledge that Software is not designed, licensed or intended for use in the design, construction, operation or maintenance of any nuclear facility." The reason it was non-free was because of the use of the term "licensed". This DES cipher license does not use the term "licensed", it simply uses "DESIGNED OR INTENDED". The license does not forbid use in any of these conditions, it merely states that the software was not designed or intended for those conditions. Lifting FE-Legal. P.S. Orcan, thanks for your continued due diligence here. :)
Thanks spot! So what would be the license of the javaviewers subpackage then? In any case, I believe that the classes/ssl/src/tight/README classes/ssl/src/ultra/README files need to be added to the %doc
I've looked at the newest package (and especially at the new subpackage) and unfortunately there are some new issues: TODO: The directory %{_datadir}/%{name} is not owned by the javaviewers subpackage. Using %{_datadir}/%{name}/ instead of %{_datadir}/%{name}/classes should fix it. TODO: The subpackage should require the fully-versioned main package: Requires: %{name} = %{version}-%{release} TODO: Regarding the BR for the build of the java parts please follow these guidelines: http://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires It may be debatable whether the "Requires:" are need, but since the java clients may also be executed outside of the browser, it would be OK to use them. TODO: There are some minor wording/spelling mistakes in the description of the javaviewers sub-package. I suggest the following: Summary: VNC clients (java applets) Description: The package contains the corresponding java clients for %{name}. They can be used with any java-enabled browser and provide an easy access to the server without the need to install software on the client machine. TODO: Man pages should not be marked as %doc. (sorry, I've overseen this in the first review) TODO: Please use consistently: %defattr(-,root,root,-) TODO: In general the java packaging guidelines encourage the packagers to build the GCJ AOT bits: http://fedoraproject.org/wiki/Packaging/GCJGuidelines I've had a quick look and it looks like that it is possible to activate it. TODO: Please add the files mentioned by Orcan in #66 to the subpackage as well. MINOR: please replace "ln -s" by %%{__ln_s}
(In reply to comment #66) > Thanks spot! So what would be the license of the javaviewers subpackage then? GPLv2+ The license you pointed out is an MIT variant, I've added it to the MIT list as the "Nuclear" variant. :) MIT + GPLv2+ = GPLv2+
Sorry for delay guys. (In reply to comment #67) > I've looked at the newest package (and especially at the new > subpackage) and unfortunately there are some new issues: > > TODO: > The directory %{_datadir}/%{name} is not owned by the javaviewers > subpackage. > Using %{_datadir}/%{name}/ instead of %{_datadir}/%{name}/classes should > fix it. Ups, sorry. Fixed. > TODO: > The subpackage should require the fully-versioned main package: > Requires: %{name} = %{version}-%{release} Why? I think in this case, when its built in separate source release of main package have no matter. > TODO: > Regarding the BR for the build of the java parts please follow these > guidelines: > http://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires > > It may be debatable whether the "Requires:" are need, but since > the java clients may also be executed outside of the browser, it would > be OK to use them. I add only BuildRequires: java-devel >= 1:1.6.0 BuildRequires: jpackage-utils but you are right, Java applets intended even on remote machine execution, so, it do not Require in current. > TODO: > There are some minor wording/spelling mistakes in the description > of the javaviewers sub-package. I suggest the following: > > Summary: VNC clients (java applets) VNC clients (browser java applets) ok? > > Description: > The package contains the corresponding java clients for %{name}. They > can be used with any java-enabled browser and provide an easy access to > the server without the need to install software on the client machine. Ok. > TODO: > Man pages should not be marked as %doc. (sorry, I've overseen this in > the first review) Ok, thanks. > TODO: > Please use consistently: %defattr(-,root,root,-) Ok. > TODO: > In general the java packaging guidelines encourage the packagers > to build the GCJ AOT bits: > http://fedoraproject.org/wiki/Packaging/GCJGuidelines > I've had a quick look and it looks like that it is possible to activate it. Is it really required? What advantage for that in nowadays? Only add mesh into spec legibility... > TODO: > Please add the files mentioned by Orcan in #66 to the subpackage as well. Off course. > MINOR: > please replace "ln -s" by %%{__ln_s} Ok. Additionally I change License: GPLv2+ for javaviewers subpackage as Spot say in post #68. http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-12.fc11.src.rpm http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec
I've reviewed the new spec file and the new src.rpm: You've stated in the %changelog of the new spec file, that the following changes were done: - Own %%{_datadir}/%%{name} instead of %%{_datadir}/%%{name}/classes - Add Requires: %%{name} = %%{version}-%%{release} in subpackage. Unfortunately the fixes are not done in this spec file. Please can you verify, whether you've uploaded the wrong version of the spec file? Regarding the packaging of the GCJ AOT bits I've asked in #fedora-java whether it is really necessary in this specific case (jar files mostly used in a browser on the client) and they've agreed that it is not needed here. So once all changes are done which are stated in the spec file, I think all issues are solved. ;-)
(In reply to comment #70) > I've reviewed the new spec file and the new src.rpm: > > You've stated in the %changelog of the new spec file, that the following > changes were done: > - Own %%{_datadir}/%%{name} instead of %%{_datadir}/%%{name}/classes > - Add Requires: %%{name} = %%{version}-%%{release} in subpackage. > Unfortunately the fixes are not done in this spec file. Please can you verify, > whether you've uploaded the wrong version of the spec file? Sorry-sorry! Off course it is my mistake. I fix it (Note, without release increase) http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec > Regarding the packaging of the GCJ AOT bits I've asked in #fedora-java whether > it is really necessary in this specific case (jar files mostly used in a > browser on the client) and they've agreed that it is not needed here. > > So once all changes are done which are stated in the spec file, I think all > issues are solved. ;-) Fine! Christian Krause, very thanks for that long and detailed review.
Unfortunately 2 minor issues remained: 1. There is still a small inconsistency between the changelog and the actual spec file: "- Add Requires: %%{name} = %%{version}-%%{release} in subpackage." but Requires: %{name} = %{version} 2. At least in my F10 installation the install step fails since the %doc line of the subpackage: %doc classes/ssl/README classes/ssl/src/tight/README classes/ssl/src/ultra/README tries to copy all README files to the same location (so that finally the package would contain only one README file): Executing(%doc): /bin/sh -e /home/rpmbuild/tmp/rpm-tmp.zbaPMi + umask 022 + cd /home/rpmbuild/BUILD + cd x11vnc-0.9.8 + DOCDIR=/home/rpmbuild/BUILDROOT/x11vnc-0.9.8-12.fc10.i386/usr/share/doc/x11vnc-javaviewers-0.9.8 + export DOCDIR + rm -rf /home/rpmbuild/BUILDROOT/x11vnc-0.9.8-12.fc10.i386/usr/share/doc/x11vnc-javaviewers-0.9.8 + /bin/mkdir -p /home/rpmbuild/BUILDROOT/x11vnc-0.9.8-12.fc10.i386/usr/share/doc/x11vnc-javaviewers-0.9.8 + cp -pr classes/ssl/README classes/ssl/src/tight/README classes/ssl/src/ultra/README /home/rpmbuild/BUILDROOT/x11vnc-0.9.8-12.fc10.i386/usr/share/doc/x11vnc-javaviewers-0.9.8 cp: will not overwrite just-created `/home/rpmbuild/BUILDROOT/x11vnc-0.9.8-12.fc10.i386/usr/share/doc/x11vnc-javaviewers-0.9.8/README' with `classes/ssl/src/tight/README' cp: will not overwrite just-created `/home/rpmbuild/BUILDROOT/x11vnc-0.9.8-12.fc10.i386/usr/share/doc/x11vnc-javaviewers-0.9.8/README' with `classes/ssl/src/ultra/README' error: Bad exit status from /home/rpmbuild/tmp/rpm-tmp.zbaPMi (%doc) Most likely it is necessary to rename to README files which are in the %doc section of the subpackage so that they have distinct names.
Thanks, fixed. And one small question - should be -javawievers sub-package noarch? http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-13.fc11.src.rpm
(In reply to comment #73) > Thanks, fixed. Ok, good. > And one small question - should be -javawievers sub-package noarch? > > http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-13.fc11.src.rpm Good point. Certainly it would be better since the sub-package only contains arch-independent files. There is no hard requirement for this, but if you would still change it, it would be great! ;-)
Ok, ready http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-14.fc11.src.rpm
I've reviewed your latest package and all issues are solved. Thank you very much for all your work on this package. -> APPROVED
New Package CVS Request ======================= Package Name: x11vnc Short Description: VNC server for the current X11 session Owners: hubbitus Branches: F-10 F-11 EL-5 InitialCC:
cvs done with F-12 added.
x11vnc-0.9.8-14.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/x11vnc-0.9.8-14.fc11
x11vnc-0.9.8-14.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/x11vnc-0.9.8-14.fc12
F-11 and F-12 build successful below, but for EPEL strange error occurred: http://koji.fedoraproject.org/koji/getfile?taskID=1757845&name=build.log Checking for unpackaged file(s): /usr/lib/rpm/check-files /var/tmp/x11vnc-0.9.8-14.el5-root-mockbuild error: Installed (but unpackaged) file(s) found: /usr/lib/debug/usr/bin/x11vnc.debug I really don't understand why - debuginfo extracted auomatically, and it's work properly on Fedora builders...
https://fedorahosted.org/fedora-infrastructure/ticket/1772
Strange, "FedoraHosted" item in select box of external bugs referred to cobbler project ( https://fedorahosted.org/cobbler/ticket/1772 ) not to Fedora-Infrastructure as I expect ( https://fedorahosted.org/fedora-infrastructure/ticket/1772 ). Is there way set Fedora-Infrastructure bug as external?
x11vnc-0.9.8-16.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/x11vnc-0.9.8-16.el5
x11vnc-0.9.8-16.el5 has been pushed to the Fedora EPEL 5 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update x11vnc'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/EL-5/FEDORA-EPEL-2009-0777
x11vnc-0.9.8-14.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
x11vnc-0.9.8-16.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.