Thanks Orion, for pursuing x2go packaging!

I had the following errors trying to mock build it with fedora-review:

../../config/makedepend/makedepend: warning:  imake.c (reading /usr/include/bits/byteswap-16.h), line 20: # error "Never use <bits/byteswap-16.h> directly; include <byteswap.h> instead."
../../config/makedepend/makedepend: warning:  makestrs.c (reading /usr/include/bits/byteswap-16.h), line 20: # error "Never use <bits/byteswap-16.h> directly; include <byteswap.h> instead."
../../config/makedepend/makedepend: warning:  AuDispose.c (reading /usr/include/bits/byteswap-16.h), line 20: # error "Never use <bits/byteswap-16.h> directly; include <byteswap.h> instead."
../../config/makedepend/makedepend: warning:  A8Eq.c (reading /usr/include/bits/byteswap-16.h), line 20: # error "Never use <bits/byteswap-16.h> directly; include <byteswap.h> instead."
/builddir/build/BUILD/nx-libs-3.5.0.20/my_configure: line 8: syntax error near unexpected token `./nx-X11/lib/Xft/config.guess'

Ah, new configure macro in rawhide was causing issues.  Fixed.
 
http://www.cora.nwra.com/~orion/fedora/nx/nx-libs.spec
http://www.cora.nwra.com/~orion/fedora/nx/nx-libs-3.5.0.20-3.fc18.src.rpm

* Fri May 31 2013 Orion Poplawski <orion.com> - 3.5.0.20-3
- Fix quoting when creating my_configure script

Okay, I point out some potential issues:

* mock fails to install the package:

Error: Package: x2goagent-3.5.0.20-3.fc20.x86_64 (/x2goagent-3.5.0.20-3.fc20.x86_64)
Requires: x2goserver

* rpmlint is not silent:

- Several non standard executable permission (0775L) on libraries. Not sure if it's intentional
- Several only-non-binary-in-usr-lib
- Several devel-file-in-non-devel-package
- Others: 

nx-libs.x86_64: W: self-obsoletion nx <= 3.5.0.20-3.fc20 obsoletes nx = 3.5.0.20-3.fc20
nx-libs.x86_64: W: self-obsoletion nx(x86-64) <= 3.5.0.20-3.fc20 obsoletes nx(x86-64) = 3.5.0.20-3.fc20
nx-libs.x86_64: E: no-binary
nx-libs.x86_64: W: non-conffile-in-etc /etc/ld.so.conf.d/nx-libs-x86_64.conf
nx-libs.x86_64: E: incorrect-fsf-address /usr/share/doc/nx-libs-3.5.0.20/LICENSE
libXcomp.x86_64: E: missing-call-to-setgroups /usr/lib64/nx/libXcomp.so.3.5.0
libXcomp.x86_64: E: incorrect-fsf-address /usr/share/doc/libXcomp-3.5.0.20/LICENSE
libXcompext.x86_64: E: incorrect-fsf-address /usr/share/doc/libXcompext-3.5.0.20/LICENSE
libXcompshad.x86_64: E: incorrect-fsf-address /usr/share/doc/libXcompshad-3.5.0.20/LICENSE
nx-devel.x86_64: W: no-dependency-on nx/nx-libs/libnx
nx-fontconfig-devel.x86_64: W: no-dependency-on nx-fontconfig/nx-fontconfig-libs/libnx-fontconfig
nx-freetype2-devel.x86_64: W: no-dependency-on nx-freetype2/nx-freetype2-libs/libnx-freetype2
nxagent.x86_64: E: missing-call-to-setgroups /usr/lib64/nx/bin/nxagent
x2goagent.x86_64: W: dangling-relative-symlink /usr/lib64/x2go/bin/x2goagent ../../nx/bin/nxagent

* there are unversioned so-files:
libNX_X11: /usr/lib64/nx/X11/libximcp.so
libNX_X11: /usr/lib64/nx/X11/libxlcDef.so
libNX_X11: /usr/lib64/nx/X11/libxlcUTF8Load.so
libNX_X11: /usr/lib64/nx/X11/libxlibi18n.so
libNX_X11: /usr/lib64/nx/X11/libxlocale.so
libNX_X11: /usr/lib64/nx/X11/libxomGeneric.so

However, they seem private, so maybe it is not an issue

I'll also attach the fedora-review output if you need it as a reference

Created attachment 755522 [details]
Initial review

(In reply to Mario Ceresa from comment #3)
> Okay, I point out some potential issues:
> 
> * mock fails to install the package:
> 
> Error: Package: x2goagent-3.5.0.20-3.fc20.x86_64
> (/x2goagent-3.5.0.20-3.fc20.x86_64)
> Requires: x2goserver

x2goserver and x2goagent each require each other at run-time, so this is expected.

> * rpmlint is not silent:
> 
> - Several non standard executable permission (0775L) on libraries. Not sure
> if it's intentional

Nope, fixed.

> - Several only-non-binary-in-usr-lib

Bug in rpmlint, see bug 483199

> - Several devel-file-in-non-devel-package

Fixed

> - Others: 
> 
> nx-libs.x86_64: W: self-obsoletion nx <= 3.5.0.20-3.fc20 obsoletes nx =
> 3.5.0.20-3.fc20
> nx-libs.x86_64: W: self-obsoletion nx(x86-64) <= 3.5.0.20-3.fc20 obsoletes
> nx(x86-64) = 3.5.0.20-3.fc20

Fixed

> nx-libs.x86_64: E: no-binary

nx-libs is kind of just a container.

> nx-libs.x86_64: W: non-conffile-in-etc /etc/ld.so.conf.d/nx-libs-x86_64.conf

Fixed.

> nx-libs.x86_64: E: incorrect-fsf-address
> /usr/share/doc/nx-libs-3.5.0.20/LICENSE

Fixed.

> libXcomp.x86_64: E: missing-call-to-setgroups /usr/lib64/nx/libXcomp.so.3.5.0

Hmm, this is an odd one...

> nx-devel.x86_64: W: no-dependency-on nx/nx-libs/libnx

Fixed.

> nx-fontconfig-devel.x86_64: W: no-dependency-on
> nx-fontconfig/nx-fontconfig-libs/libnx-fontconfig
> nx-freetype2-devel.x86_64: W: no-dependency-on
> nx-freetype2/nx-freetype2-libs/libnx-freetype2

Fixed the naming.

> nxagent.x86_64: E: missing-call-to-setgroups /usr/lib64/nx/bin/nxagent

Ah, okay, looking into it.

> x2goagent.x86_64: W: dangling-relative-symlink /usr/lib64/x2go/bin/x2goagent
> ../../nx/bin/nxagent
> 
> * there are unversioned so-files:
> libNX_X11: /usr/lib64/nx/X11/libximcp.so
> libNX_X11: /usr/lib64/nx/X11/libxlcDef.so
> libNX_X11: /usr/lib64/nx/X11/libxlcUTF8Load.so
> libNX_X11: /usr/lib64/nx/X11/libxlibi18n.so
> libNX_X11: /usr/lib64/nx/X11/libxlocale.so
> libNX_X11: /usr/lib64/nx/X11/libxomGeneric.so
> 
> However, they seem private, so maybe it is not an issue

These are links to versioned files.


http://www.cora.nwra.com/~orion/fedora/nx/nx-libs.spec
http://www.cora.nwra.com/~orion/fedora/nx/nx-libs-3.5.0.20-4.fc18.src.rpm

* Tue Jun 11 2013 Orion Poplawski <orion.com> - 3.5.0.20-4
- Fix 775 library permissions
- Move nx/X11 .so files to -devel
- Fix nx obsoletes
- Mark ld.so.conf.d files config(noreplace)
- Fix requires

Hello Orion!
here there is another round of questions:

* rpmlint:
x2goagent.x86_64: W: dangling-relative-symlink /usr/lib64/x2go/bin/x2goagent ../../nx/bin/nxagent

* has any of the nx packages ever been granted an FPC exception for all the libraries it bundles? I couldn't find any ticket around.

* Not sure about this: "Fully versioned dependency in subpackages, if present."
Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in libNX_ICE-devel , libNX_SM-devel , libNX_X11-devel , libNX_Xau-devel , libNX_Xext-devel , libNX_Xfixes-devel , libNX_Xi-devel , libNX_Xmu-devel , libNX_Xpm-devel , libNX_Xrender-devel , libNX_Xt-devel , libNX_Xv-devel , libNX_fontenc-devel , libNX_xkbfile-devel , libXcomp-devel , libXcompext-devel , libXcompshad-devel , libNX_Mesa-devel , nx-bitmaps-devel , nx-devel , libNX_fontconfig-devel , libNX_freetype-devel , nx-proto-devel , nxagent , nxauth , nxproxy , x2goagent

(In reply to Mario Ceresa from comment #6)
> Hello Orion!
> here there is another round of questions:
> 
> * rpmlint:
> x2goagent.x86_64: W: dangling-relative-symlink /usr/lib64/x2go/bin/x2goagent
> ../../nx/bin/nxagent

Okay because x2goagent requires nxagent.

> * has any of the nx packages ever been granted an FPC exception for all the
> libraries it bundles? I couldn't find any ticket around.

Hmm, I never realized quite the extent of the bundling - let me poke around there some more.  I've gotten rid of some, but not all.

> * Not sure about this: "Fully versioned dependency in subpackages, if
> present."
> Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
> libNX_ICE-devel , libNX_SM-devel , libNX_X11-devel , libNX_Xau-devel ,
> libNX_Xext-devel , libNX_Xfixes-devel , libNX_Xi-devel , libNX_Xmu-devel ,
> libNX_Xpm-devel , libNX_Xrender-devel , libNX_Xt-devel , libNX_Xv-devel ,
> libNX_fontenc-devel , libNX_xkbfile-devel , libXcomp-devel ,
> libXcompext-devel , libXcompshad-devel , libNX_Mesa-devel , nx-bitmaps-devel
> , nx-devel , libNX_fontconfig-devel , libNX_freetype-devel , nx-proto-devel
> , nxagent , nxauth , nxproxy , x2goagent

The libN?X*-devel packages require their corresponding runtime versions (which require %{name}%{?_isa} = %{version}-%{release}), although many did not have %{version}-%{release} - fixed.  Not really needed for the program sub-packages.

nx-bitmaps-devel stands on its own (and not sure if it is needed), and so does nx-proto-devel.

Fixed for nx-devel

* Thu Jun 13 2013 Orion Poplawski <orion.com> - 3.5.0.20-5
- Add more explicit verioned requires
- Drop unnecessary directory ownership by sub-packages
- Remove many bundled libraries


http://www.cora.nwra.com/~orion/fedora/nx/nx-libs.spec
http://www.cora.nwra.com/~orion/fedora/nx/nx-libs-3.5.0.20-5.fc18.src.rpm

I've done a lot more cleanup.

* Thu Jul 11 2013 Orion Poplawski <orion.com> - 3.5.0.20-6
- Drop building and/or shipping a bunch of unneeded libraries

http://www.cora.nwra.com/~orion/fedora/nx/nx-libs.spec
http://www.cora.nwra.com/~orion/fedora/nx/nx-libs-3.5.0.20-6.fc11.src.rpm

I'm still probably shipping more than is strictly needed, but I'd like to err to that side for now.

I think Orion knows that error of missing-call-to-setgroups.

missing-call-to-setgroups has been renamed to missing-call-to-setgroups-before-setuid.

This will be available in the next version.

And the explanation is:

This executable is calling setuid and setgid without setgroups or initgroups.
There is a high probability this mean it didn't relinquish all groups, and this
would be a potential security issue to be fixed. Seek POS36-C on the web for
details about the problem.

Ref POS36-C:

https://www.securecoding.cert.org/confluence/display/seccode/POS36-C.+Observe+correct+revocation+order+while+relinquishing+privileges

The SRPM link in comment 8 appears to be wrong or dead. It appears that it should be .fc19 rather than .fc11.

Oops, yeah:

http://www.cora.nwra.com/~orion/fedora/nx/nx-libs-3.5.0.20-6.fc19.src.rpm

I'm trying to get some info out of upstream.  I'm not sure why it is even calling setuid/setgid in the first place.

We're making great progress reviewing the dependencies :), but not nx-libs itself :(.  Could someone step up and take on this review?  I'm also not sure what is a blocker at this point.  Thanks!

Orion, I'll be back next Monday, if that's okay to you, tell me and I'll take the review.

Best,

Mario

I would just like *somebody* to take the review. :)

Well, looks like we've missed the change deadline for F20.  Sorry.

Upstream's comment on the setgid issue:

Everything in NX runs under the user who launches the X2Go session. IMHO resetting the effective GID prevents us from setgid file permission manipulations, so that the effective group ID always is the primary/real group ID of the current user that is executing the NX binary.

----

We're rapidly approaching the change deadline for F20.  Are there any blockers in this package?  It would be nice to get a review done soon.

http://www.cora.nwra.com/~orion/fedora/nx/nx-libs-3.5.0.20-7.fc19.src.rpm


* Thu Aug 29 2013 Orion Poplawski <orion.com> - 3.5.0.20-7
- Add patch to call initgroups()

I'll give this a go.

- rpmlint checks return:

nx-libs.spec:16: W: macro-in-comment %{name}
There is a unescaped macro after a shell style comment in the specfile. Macros
are expanded everywhere, so check if it can cause a problem in this case and
escape the macro with another leading % if appropriate.

nx-libs.spec:493: W: macro-in-comment %{_libdir}
There is a unescaped macro after a shell style comment in the specfile. Macros
are expanded everywhere, so check if it can cause a problem in this case and
escape the macro with another leading % if appropriate.

nx-libs.spec:493: W: macro-in-comment %{_libexedir}
There is a unescaped macro after a shell style comment in the specfile. Macros
are expanded everywhere, so check if it can cause a problem in this case and
escape the macro with another leading % if appropriate.

W: only-non-binary-in-usr-lib  for the -devel pacakges.  Probably OK for NX.

x-libs.x86_64: E: no-binary
The package should be of the noarch architecture because it doesn't contain
any binaries.

Lots of spurious executable perms in debuginfo, you can probably use find to chmod -x in setup.

Would making this noarch overly complicate things?

And lots of bogus spelling error / no docs/manpages.

- package meets naming guidelines
- package meets packaging guidelines
- license ( GPLv2+ ) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
! post/postun ldconfig ok   Runs on post, but not postun
! devel requires base package n-v-r    -devel pacakges don't Require main lib pacakges

So it look like just commented macros, spurious executables, possibly noarch main package, postun ldconfig, and -devel Requires.

Also, after i replaced nx with nx-libs, I had to manually nxproxy before remmina would connect, but then it was fine, so nx-libs should require nxproxy.

I also replaced nx on my freenx server, and that worked, once nxagent and its deps were installed, so it should be required as well.

(In reply to Jon Ciesla from comment #19)
> - rpmlint checks return:
>
> x-libs.x86_64: E: no-binary
> The package should be of the noarch architecture because it doesn't contain
> any binaries.
> 
> Lots of spurious executable perms in debuginfo, you can probably use find to
> chmod -x in setup.

Yowza.  Fixed.

> Would making this noarch overly complicate things?

Cannot make the main package noarch, sub-packages arch.

> ! post/postun ldconfig ok   Runs on post, but not postun

Fixed.

> ! devel requires base package n-v-r    -devel pacakges don't Require main
> lib pacakges

Fixed by renaming nx-devel to nx-libs-devel

> So it look like just commented macros, spurious executables, possibly noarch
> main package, postun ldconfig, and -devel Requires.

http://www.cora.nwra.com/~orion/fedora/nx/nx-libs-3.5.0.21-1.fc19.src.rpm

* Fri Aug 30 2013 Orion Poplawski <orion.com> - 3.5.0.21-1
- Update to 3.5.0.21
- Many bundled libs removed upstream
- Drop initgroups patch applied upstream
- Fix macro in comments
- Remove execute permissions from source files
- Add %%postun ldconfig scripts
- Rename nx-devel nx-libs-devel

(In reply to Jon Ciesla from comment #20)
> Also, after i replaced nx with nx-libs, I had to manually nxproxy before
> remmina would connect, but then it was fine, so nx-libs should require
> nxproxy.

Actually, remmina-nx needs to require nxproxy once this is imported.

> I also replaced nx on my freenx server, and that worked, once nxagent and
> its deps were installed, so it should be required as well.

And again, freenx-server will need to require nxagent.

The problem is that the current nx package contains both server and client components, while this nx-libs package separates them out.  As such we can't really automatically migrate the dependencies properly.

Looks great, but I'm not sure I agree about the requires.  If this is meant to be a truly drop-in replacement, my expectation would be that upgrading from the old nx to this would Just Work, and I'd think this could be achieved with the right Provides and Requires.  But that's your call.

APPROVED.

Okay, I have nx-libs, nxproxy, and nxagent all obsolete/provide nx so current upgrades will be drop in.  Then we can tweak the requires of remmina-plugins-nx and freenx-server later.

* Mon Sep 3 2013 Orion Poplawski <orion.com> - 3.5.0.21-2
- Have nxagent and nxproxy also obsolete/provide nx


New Package SCM Request
=======================
Package Name: nx-libs
Short Description: NX X11 protocol compression libraries
Owners: orion
Branches: f18 f19 f20 el6
InitialCC:

Git done (by process-git-requests).

Checked in and built.  Thanks all.