[Bug 445537] New: Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

Summary: Review Request: tightvnc - VNC software
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: medium
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail],[e-mail]

Spec URL: http://people.redhat.com/atkac/tightvnc.spec
SRPM URL: http://people.redhat.com/atkac/tightvnc-1.5.0-0.1.svn2562.fc9.src.rpm
Description: 
Virtual Network Computing (VNC) is a remote display system which
allows you to view a computing 'desktop' environment not only on the
machine where it is running, but from anywhere on the Internet and
from a wide variety of machine architectures

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: tightvnc - VNC software

https://bugzilla.redhat.com/show_bug.cgi?id=445537

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

------- Additional Comments From [e-mail]  2008-05-07 15:26 EST -------
Is this really intended to replace (real)vnc?  (noticed the Obsoletes)

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: tightvnc - VNC software

https://bugzilla.redhat.com/show_bug.cgi?id=445537

------- Additional Comments From [e-mail]  2008-05-07 16:24 EST -------
Just a notice: I've compared 2 weeks ago the behaviour of vnc (from Centos 5)
and tightvnc (version 1.3.9 from rpmforge, still on the same Centos 5). Despite
all our efforts (we tried absolutely all available optimization switches, such
as    -depth 8, -compresslevel 9, -quality 4 ) access via tightvnc to a remote
server (which is completely out of my reach; I assume it runs RHEL, no idea
about the version ) was painfully slow (delays in the terms of seconds). Using
vnc -LowColourLevel 2 brought things closer to normality. RTT were in the
100-110 ms range in all cases.

Does this newer version improve things ?

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: tightvnc - VNC software

https://bugzilla.redhat.com/show_bug.cgi?id=445537

------- Additional Comments From [e-mail]  2008-05-07 18:19 EST -------
Current TightVNC trunk (and upcomming 1.5 version) is forked RealVNC source with
many improvements + bugfixes so behavior should be same as current (Real)vnc.
More details are on http://fedoraproject.org/wiki/Features/TightVNC.

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: tightvnc - VNC software

https://bugzilla.redhat.com/show_bug.cgi?id=445537

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

------- Additional Comments From [e-mail]  2008-07-01 16:24 EST -------
rpmlint returns:
tightvnc.src:164: E: files-attr-not-set
tightvnc.src:165: E: files-attr-not-set
tightvnc.src:166: E: files-attr-not-set
tightvnc.src:167: E: files-attr-not-set
tightvnc.src: W: strange-permission vncserver.init 0755
1 packages and 0 specfiles checked; 4 errors, 1 warnings.

You forgot to do:
%defattr(-,root,root,-)
in the main %files section

The strange permission warning is, I think, because all files in your package
have 0664 (note the group-write permission)

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: tightvnc - VNC software

https://bugzilla.redhat.com/show_bug.cgi?id=445537

------- Additional Comments From [e-mail]  2008-07-02 05:41 EST -------
Ok, I fixed problems written above. Only vncserver.init should be leaved as is.

http://people.redhat.com/atkac/tightvnc-1.5.0-0.2.svn2562.fc10.src.rpm
http://people.redhat.com/atkac/tightvnc.spec

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #6 from Adam Tkac   2008-09-30 04:33:20 EDT ---
I updated source to latest trunk:

http://people.redhat.com/atkac/tightvnc.spec
http://people.redhat.com/atkac/tightvnc-1.5.0-0.3.svn2831.fc10.src.rpm

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

Dan Horák  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #7 from Dan Horák   2008-10-10 08:37:05 EDT ---
few notes
- you can use "svn export" instead of "svn checkout" (or a even script like
http://fedora.danny.cz/fedora-getsvn), the source archive will be smaller, but
when you are using upstream released snapshots, use them
- the EVRs in Obsoletes/Provides are inconsistent between main package and
-server subpackage and I don't think that the main package should contain
Obsol/Prov for vnc-server
- better use --with-os-name="Fedora" in xserver's %configure (instead of
"Fedora 11")
- replace /etc with %{_sysconfdir}, /etc/rc.d/init.d with %{_initddir} in
%build and %install sections
- don't add X-Red-Hat-Extra into desktop file, fix the file directly instead of
using command line options, use fedora as vendor, missing BR:
desktop-file-utils - more about desktop files at
https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #8 from Adam Tkac   2008-10-13 04:06:01 EDT ---
(In reply to comment #7)
> few notes
> you can use "svn export" instead of "svn checkout" (or a even script like
>
> http://fedora.danny.cz/fedora-getsvn), the source archive will be smaller, but
> when you are using upstream released snapshots, use them

good hint
> the EVRs in Obsoletes/Provides are inconsistent between main package and
>
> -server subpackage and I don't think that the main package should contain
> Obsol/Prov for vnc-server

fixed
> better use --with-os-name="Fedora" in xserver's %configure (instead of
>
> "Fedora 11")

might be
> replace /etc with %{_sysconfdir}, /etc/rc.d/init.d with %{_initddir} in
>
> %build and %install sections

fixed
> don't add X-Red-Hat-Extra into desktop file, fix the file directly instead of
>
> using command line options, use fedora as vendor, missing BR:
> desktop-file-utils - more about desktop files at
> https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files

should be fixed as well.

Updated srpm & spec:
http://people.redhat.com/atkac/tightvnc.spec
http://people.redhat.com/atkac/tightvnc-1.5.0-0.4.svn2975.fc10.src.rpm

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #9 from Dan Horák   2008-10-14 05:15:32 EDT ---
formal review is here, see the notes below:

OK source files match upstream (checked with diff between the included sources
and fresh export from SVN)
BAD package meets naming and versioning guidelines.
OK specfile is properly named, is cleanly written and uses macros consistently.
OK dist tag is present.
OK build root is correct.
OK license field matches the actual license.
OK license is open source-compatible. License text included in package.
OK latest version is being packaged.
OK BuildRequires are proper.
BAD compiler flags are appropriate.
OK %clean is present.
OK package builds in mock (Rawhide/x86_64).
OK debuginfo package looks complete.
BAD rpmlint is silent.
BAD final provides and requires look sane.
N/A %check is present and all tests pass.
OK no shared libraries are added to the regular linker search paths.
BAD owns the directories it creates.
OK doesn't own any directories it shouldn't.
OK no duplicates in %files.
OK file permissions are appropriate.
BAD scriptlets present
OK code, not content.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK no headers.
OK no pkgconfig files.
OK no libtool .la droppings.
OK* is a GUI app, desktop file installed

- include instructions for getting the sources - see
https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control

- snapshot package must contain date in the Release tag - see
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages

- don't disable optimization (export CFLAGS="$RPM_OPT_FLAGS -O0") - drop the
-O0

- rpmlint complains:
tightvnc-debuginfo.x86_64: W: spurious-executable-perm
/usr/src/debug/tightvnc-1.5.0-svn2975/unix/xserver/*/*.[ch] (about 12 files)
 run "chmod a-x unix/xserver/*/*.[ch]" after copying the xorg-x11-server-Xorg
sources into the tightvnc tree
tightvnc-server.x86_64: W: incoherent-init-script-name vncserver
 can be ignored

- the EVR in Obsoletes/Provides must be greater than the EVR of the package we
are replacing (use EVR=4.1.2-36, no dist tag) - see
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Renaming.2Freplaci...

- server subpackage should have "Requires: xorg-x11-server-Xorg" for the
ownership of %{_libdir}/xorg/modules/extensions/

- the "always succeed" clause (" || :") is missing from the server's postun
scriptlet and from gtk-update-icon-cache too - see
https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_in_spec_...
and
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#GTK.2B_icon_cache

- remove the X-Red-Hat-Extra from the desktop file

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #10 from Adam Tkac   2008-10-15 08:13:48 EDT ---
(In reply to comment #9)
> formal review is here, see the notes below:
>
> BAD package meets naming and versioning guidelines.

Fixed
> BAD compiler flags are appropriate.

Fixed, -O0 was (and probably will be during early F11 cycle) temporary option
because it makes core dumps more usable.
> BAD scriptlets present

Fixed
> rpmlint complains:
>
> tightvnc-debuginfo.x86_64: W: spurious-executable-perm
> /usr/src/debug/tightvnc-1.5.0-svn2975/unix/xserver/*/*.[ch] (about 12 files)
>  run "chmod a-x unix/xserver/*/*.[ch]" after copying the xorg-x11-server-Xorg
> sources into the tightvnc tree

Fixed
> the EVR in Obsoletes/Provides must be greater than the EVR of the package we
>
> are replacing (use EVR=4.1.2-36, no dist tag) - see
> https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Renaming.2Freplaci...

Fixed
> server subpackage should have "Requires: xorg-x11-server-Xorg" for the
>
> ownership of %{_libdir}/xorg/modules/extensions/

dependency on xorg-x11-server-Xorg doesn't seem so good for me so I moved
libvnc.so module to server-module subpackage which requires
xorg-x11-server-Xorg
> remove the X-Red-Hat-Extra from the desktop file

Removed

Updated sources:
http://people.redhat.com/atkac/tightvnc.spec
http://people.redhat.com/atkac/tightvnc-1.5.0-0.5.20081015svn3019.fc10.src.r...

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #11 from Dan Horák   2008-10-15 09:18:17 EDT ---
Yet 2 little issues
- the License tag should be GPLv2+ (sorry I missed that in the first round)
- include instructions for getting the sources - see
https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control
(svn export https://vnc-tight.svn.sourceforge.net/svnroot/vnc-tight/trunk -r
%{revision} tightvnc; tar czvf ...)

And one suggestion - can you move this section

cp -r %{_datadir}/xorg-x11-server-source/* unix/xserver
pushd unix/xserver
for all in `find . -type f -perm -001`; do
 chmod -x "$all"
done
patch -p0 < ../xserver.patch

from %build to %prep, because this is in fact source preparation too.

And last thing (not related to the review) could be refreshing the
xserver.patch, because it applies quite unclean.

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #12 from Adam Tkac   2008-10-15 12:17:20 EDT ---
All problems should be fixed:

http://people.redhat.com/atkac/tightvnc-1.5.0-0.6.20081015svn3021.fc10.src.r...
http://people.redhat.com/atkac/tightvnc.spec

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #13 from Adam Tkac   2008-10-15 12:40:39 EDT ---
(In reply to comment #12)
> All problems should be fixed:
>
> http://people.redhat.com/atkac/tightvnc-1.5.0-0.6.20081015svn3021.fc10.src.r...
> http://people.redhat.com/atkac/tightvnc.spec

Well, SRPM written above is broken, updated one is:
http://people.redhat.com/atkac/tightvnc-1.5.0-0.7.20081015svn3022.fc10.src.r...

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

Dan Horák  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #14 from Dan Horák   2008-10-15 12:48:28 EDT ---
All issues are resolved and this package is APPROVED. Great work, Adam.

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

Adam Tkac  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

--- Comment #15 from Adam Tkac   2008-10-16 06:40:29 EDT ---
CVS request is written below, please create early F10 branch and devel:

New Package CVS Request
=======================
Package Name: tightvnc
Short Description: A TightVNC remote display system
Owners: atkac
Branches: F-10

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #16 from Kevin Fenzi   2008-10-16 13:03:05 EDT ---
cvs done.

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

Adam Tkac  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

Jan ONDREJ  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #17 from Jan ONDREJ   2008-12-08 05:26:27 EDT ---
Why there are still no packages of this?

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #18 from Dan Horák   2008-12-08 05:32:55 EDT ---
You are probably looking at wrong place. It is in Rawhide.

[root@localhost etc]# rpm -q tightvnc
tightvnc-1.5.0-0.9.20081120svn3200.fc11.x86_64

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #19 from Jan ONDREJ   2008-12-08 05:43:48 EDT ---
CVS request was for F-10 and it's not same as rawhide.
Why there is no tightvnc for F-10?

If you need help with building this package on F-10, just tell me.

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #20 from Dan Horák   2008-12-08 06:02:07 EDT ---
There is little misunderstanding - Adam's intention was to do the move from vnc
to tightvnc in F-11, but to achieve that in the time of the CVS request, it was
required to ask for F-10 branch whose content will be included in F-10 (but it
will remain empty here) and only the content of devel will go into rawhide even
when usual devel branch goes into F-to-be-10.

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #21 from Jan ONDREJ   2008-12-08 06:52:18 EDT ---
Is it possible to build this package in F-10 branch without "Obsoletes"
attribute?
Or is there a problem with that?
I think an empty folder is not a good idea. Also there is an problem, that this
package is included in pkgdb:
  https://admin.fedoraproject.org/pkgdb/packages/name/tightvnc#Fedora10

I like tightvnc and real-vnc is not what I want.

I know, that I can rebuild this for myself, but It's nicer to have this package
in distribution.

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #22 from Adam Tkac   2008-12-08 06:57:45 EDT ---
(In reply to comment #21)
> Is it possible to build this package in F-10 branch without "Obsoletes"
> attribute?
> Or is there a problem with that?
> I think an empty folder is not a good idea. Also there is an problem, that this
> package is included in pkgdb:
>   https://admin.fedoraproject.org/pkgdb/packages/name/tightvnc#Fedora10
>
> I like tightvnc and real-vnc is not what I want.

Well, I can build tightvnc for F10 but I don't see reason for it. Current trunk
tightvnc is forked RealVNC source - it is completely different from "old"
tightvnc 1.2.9 & 1.3.9.

[Bug 445537] Review Request: tightvnc - VNC software

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=445537

--- Comment #23 from Jan ONDREJ   2008-12-08 07:41:42 EDT ---
Hmm, I am sorry. You right.

What's happened, that this nice project is where they started? Back on realvnc
source.