Bug 616193

Hi Christoph, you might want to review  bug #603481.

I wonder why I didn't find it when I packaged freerdp.

*** This bug has been marked as a duplicate of bug 603481 ***

Some brief comments from reading through the spec follows. Some of it is duplicate of what is said on bug 603481, but some of it might explain something (not) there.

> License:        BSD and GPLv2+

The project as whole is GPLv2+ even though some parts "just" are GPL-compatible. (Just like glibc doesn't mention BSD despite /usr/share/doc/glibc-2.12/LICENSES.)

> %description
> FreeRDP is a X Remote Desktop Protocol Client for Windows Terminal Servers. 
> It is a fork of the rdesktop project that intends to rapidly start moving 
> forward and implement features that rdesktop lacks the most.

Actually, FreeRDP is the project, libfreerdp is the library, and xfreerdp is the X client. (I have tried to convince upstream to create a simple client on each platform and call it freerdp but haven't succeeded yet.) The way it is I think it would be least confusing if the package was called xfreerdp like the binary.

> BuildRequires:  directfb-devel

The directfb port is far from ready yet and shouldn't be packaged

> BuildRequires:  pcsc-lite-devel

configure is lying - that isn't implemented yet / any longer.

> BuildRequires:  libsamplerate-devel
> BuildRequires:  libao-devel

> # not enabled by default cause it does not yet work
> %{?_with_libvncserver:BuildRequires: libvncserver-devel}  

Also old cruft that doesn't apply

> Requires:       pcsc-lite

N/A

> %package        directfb

N/A

> %package        common
> Summary:        Common libraries, keymaps and plugins for %{name}
> Group:          System Environment/Libraries
>
> %description    common
> The %{name}-common package contains common libraries, keymaps and plugins for 
> %{name} and %{name}-directfb.

It is mostly the core freerdp lib, so shouldn't it be called something with lib instead?

I also find it confusing that the -devel package is for a -common package.

> %prep
> %setup -q
>
>
> %build
> %configure --enable-smartcard --with-dfb

N/A N/A

> --disable-static \

good idea

>     %{?_with_libvncserver:--with-libvncserver}
N/A

> sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
> sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool

Are they really needed? AFAICS my packages without this hack don't have any issues.

> %install
> rm -rf $RPM_BUILD_ROOT
> make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p'

AFAIK -p isn't required by the guidelines. Just doing it on the packages where the maintainer cares seems a bit odd.

> %doc AUTHORS COPYING README doc/{ChangeLog,TODO,rdp-keyboard.odg,*.txt}

Most of these files are outdated and no longer applies

I have already answered to your comments in bug 603481, but just for the record. I'm leaving out the parts where I agree.

(In reply to comment #3)
> The project as whole is GPLv2+ even though some parts "just" are
> GPL-compatible. (Just like glibc doesn't mention BSD despite
> /usr/share/doc/glibc-2.12/LICENSES.)

I was under the impression that certain libs were BSD (only). As soon as something BSD licensed builds into a separate library or binary, BSD must be mentioned explicitly. AFAICS this is the case here.

> Actually, FreeRDP is the project, libfreerdp is the library, and xfreerdp is
> the X client. (I have tried to convince upstream to create a simple client on
> each platform and call it freerdp but haven't succeeded yet.) The way it is I
> think it would be least confusing if the package was called xfreerdp like the
> binary.

See my comment in bug 603481.

> It is mostly the core freerdp lib, so shouldn't it be called something with lib
> instead?

Not sure, it also includes the keyboard definitions.

> I also find it confusing that the -devel package is for a -common package.

Huh?
> > sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
> > sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
> 
> Are they really needed? AFAICS my packages without this hack don't have any
> issues.

I'm getting rpaths when I build this locally, this doesn't happen in koji or mock tough.

> > make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p'
> 
> AFAIK -p isn't required by the guidelines. Just doing it on the packages where
> the maintainer cares seems a bit odd.

The guidelines mention to preserve timestamps. Things like the keyboard defintions that are not getting compiled should IMHO have the upstream timestamp.

(In reply to comment #4)

I'm trying to extract as much knowledge and opinion from your packaging as possible ;-)

> I was under the impression that certain libs were BSD (only). As soon as
> something BSD licensed builds into a separate library or binary, BSD must be
> mentioned explicitly. AFAICS this is the case here.

Can you be more specific? Which library/binary, and where are the statements of BSD license?

> > It is mostly the core freerdp lib, so shouldn't it be called something with lib
> > instead?
> 
> Not sure, it also includes the keyboard definitions.

Yes, but that is data for libfreerdpkbd and not intended to be used or seen in other ways.

> > I also find it confusing that the -devel package is for a -common package.
> 
> Huh?

I would expect "foo-devel" to contain the headers etc for libraries in "foo". The devel stuff for "foo-common" should be in "foo-common-devel"?

> > > sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
> > > sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
> > 
> > Are they really needed? AFAICS my packages without this hack don't have any
> > issues.
> 
> I'm getting rpaths when I build this locally, this doesn't happen in koji or
> mock tough.

You don't know why?

I will investigate further.

> > > make install DESTDIR=$RPM_BUILD_ROOT INSTALL='install -p'
> > 
> > AFAIK -p isn't required by the guidelines. Just doing it on the packages where
> > the maintainer cares seems a bit odd.
> 
> The guidelines mention to preserve timestamps. Things like the keyboard
> defintions that are not getting compiled should IMHO have the upstream
> timestamp.    

Yes, "consider" preserving timestamps. Something less vague could be nice. Now it is up to my preference to focus on simple spec and legibility or on preserving timestamps - that is bad for consistency ...

(In reply to comment #5)
> Can you be more specific? Which library/binary, and where are the statements of
> BSD license?

Sorry, it's actually MIT, to be precise https://fedoraproject.org/wiki/Licensing/MIT#Modern_Style_with_sublicense 
* in asn1 everything is BSD
* in channels/cliprdr everything is MIT
* in channels/common everything it MIT except chan_stream.{c,h} (GPLv2+)
* in channels/rdpdr everything is GPLv2+, except rdpdr_main.c (MIT)
* in channels/rdpsnd we have rdpsnd.h/rdpsnd_dsp.h (MIT) and rspsnd_alsa.c/rdpsnd_main.c (GPLv2+)
* in include/freerdp everything is MIT except kbd.h and rdpset.h (GPLv2+)
* in libfreerdp everything is GPLv2+ except frdp.h (MIT)
* libfreerdpchanman is MIT
* in libfreerdpkbd everything is GPLv2+
* in X11 everything is MIT

I think that xfreedrp is MIT, so if there is a separate package for xfreerdp, it should have MIT as license tag.
The rest is a mess, but I think it will come down to GPLv2+. The plugins are compiling against the things from common, right?

> I would expect "foo-devel" to contain the headers etc for libraries in "foo".
> The devel stuff for "foo-common" should be in "foo-common-devel"?

Now I see what you mean, but usually we only have one foo-devel package.

> > I'm getting rpaths when I build this locally, this doesn't happen in koji or
> > mock tough.
> 
> You don't know why?

I'm sorry, I don't know. But I'm having the same problem with other packages too.

> > The guidelines mention to preserve timestamps. Things like the keyboard
> > defintions that are not getting compiled should IMHO have the upstream
> > timestamp.    
> 
> Yes, "consider" preserving timestamps. Something less vague could be nice. Now
> it is up to my preference to focus on simple spec and legibility or on
> preserving timestamps - that is bad for consistency ...    

So how about using -p in general, then it's consistent again? ;) I wouldn't consider this a blocker during the review. Do as you like.

(In reply to comment #6)
> > Can you be more specific? Which library/binary, and where are the statements of
> > BSD license?
> 
> Sorry, it's actually MIT, to be precise
> https://fedoraproject.org/wiki/Licensing/MIT#Modern_Style_with_sublicense 
> * in asn1 everything is BSD
> * in channels/cliprdr everything is MIT
> * in channels/common everything it MIT except chan_stream.{c,h} (GPLv2+)
> * in channels/rdpdr everything is GPLv2+, except rdpdr_main.c (MIT)
> * in channels/rdpsnd we have rdpsnd.h/rdpsnd_dsp.h (MIT) and
> rspsnd_alsa.c/rdpsnd_main.c (GPLv2+)
> * in include/freerdp everything is MIT except kbd.h and rdpset.h (GPLv2+)
> * in libfreerdp everything is GPLv2+ except frdp.h (MIT)
> * libfreerdpchanman is MIT
> * in libfreerdpkbd everything is GPLv2+
> * in X11 everything is MIT
> 
> I think that xfreedrp is MIT, so if there is a separate package for xfreerdp,
> it should have MIT as license tag.
> The rest is a mess, but I think it will come down to GPLv2+. The plugins are
> compiling against the things from common, right?

Yes, so everything is either BSD or MIT or GPLv2+, but the xfreerdp and all libs contains or links against GPLv2+ headers and libraries, so effectively everything is GPLv2+, right?