Bug 470696

Updated the SPEC and SRPM for better placement of the mod_passenger.so file (in %{_libdir}/httpd/modules instead of %{ruby_sitearch}/apache2/mod_passenger.so) so that the configuration file %{_sysconfdir}/httpd/conf.d/mod_passenger.so can actually load it independent of the architecture.

New SPEC: http://www.kanarip.com/custom/SPECS/rubygem-passenger.spec
New SRPM: http://www.kanarip.com/custom/f9/SRPMS/rubygem-passenger-2.0.3-2.fc9.src.rpm

Umm...
spot, would you answer my question below?

! First of all please unpack passenger-2.0.3.gem in the srpm by below:
  $ mkdir TMP ; cd TMP
  $ tar xf ../*gem (gem can be unpacked by tar)
  $ mkdir TMP ; cd TMP
  $ tar xzf ../data.tar.gz
  Then:
First of all, the overall license this package is GPLv2 (not GPLv2+)
Then ext/apache2/LICENSE-CNRI.TXT says:
/////////////////////////////////////////////////////////////////////////
A few functions in ext/apache2/Hooks.cpp are based on the source code of
mod_scgi version 1.9. Its license is included in this file.
Please note that these licensing terms *only* encompass those few
functions, and not Passenger as a whole.
------------------------------------------------------------------------
CNRI OPEN SOURCE LICENSE AGREEMENT

(CNRI = Python 1.6 i.e. GPL incompatible license follows)
/////////////////////////////////////////////////////////////////////////

What I am in trouble is that
- What functions in ext/apache2/Hooks.cpp are actually based on mod_scgi codes
- And I don't know for now how these functions are used in the other parts
  of passenger source codes
- So I am not sure if the code in Hooks.cpp under CNRI license won't conflict
  with GPL.

spot, how do you think about this. For me the current status seems very obscure.

By the way some pre-remarks

* BuildRequires
  - This package won't build without "BR: rubygem(fastthread)".
  - "BR: gcc-c++ findutils" are redundant.

* Requires
  - Please check if all needed Requires are correctly added.
    It seems that at least "Requires: rubygem(rack)
    rubygem(fastthread)" are needed.

* boost dependency
  - Well, when I try below to make build log more verbose
    (please consider this)
-------------------------------------------------------
%prep
%setup -q -c -T

mkdir BINDIR
cat > BINDIR/rake <<EOF
#!/bin/bash
%{_bindir}/rake -v \$@
EOF
chmod 0755 BINDIR/rake

%build
export CONFIGURE_ARGS="--with-cflags='%{optflags}'"
export PATH=$(pwd)/BINDIR:$PATH
gem install \
......
-------------------------------------------------------
      build log shows (attached)
-------------------------------------------------------
  1525  DEBUG: cd /builddir/build/BUILD/rubygem-passenger-2.0.3/usr/lib/ruby/gems/1.8/gems/passenger-2.0.3
  1526  DEBUG: rake clean apache2
  1527  DEBUG: rm -rf Utils.o Bucket.o Logging.o System.o Configuration.o Hooks.o mod_passenger.o mod_passenger.so ApplicationPoolSe
rverExecutable
  1528  DEBUG: (in /builddir/build/BUILD/rubygem-passenger-2.0.3/usr/lib/ruby/gems/1.8/gems/passenger-2.0.3)
  1529  DEBUG: ### In ext/apache2:
  1530  DEBUG: rm -r pkg
  1531  DEBUG: make clean
  1532  DEBUG: ### In ext/passenger:
  1533  DEBUG: rm -f Makefile
  1534  DEBUG: rm -f libboost_thread.a *.o
  1535  DEBUG: ### In ext/boost/src:
  1536  DEBUG: rm -f Apache2ModuleTests *.o
  1537  DEBUG: ### In test:
  1538  DEBUG: rm -f DummyRequestHandler ApplicationPool
  1539  DEBUG: ### In benchmark:
  1540  DEBUG: g++ -g -DPASSENGER_DEBUG -fPIC -I../.. -D_REENTRANT -DNDEBUG  -c *.cpp
  1541  DEBUG: ### In ext/boost/src:
  1542  DEBUG: g++ -g -DPASSENGER_DEBUG -fPIC -I../.. -D_REENTRANT -DNDEBUG  -c pthread/exceptions.cpp
  1543  DEBUG: g++ -g -DPASSENGER_DEBUG -fPIC -I../.. -D_REENTRANT -DNDEBUG  -c pthread/once.cpp
  1544  DEBUG: g++ -g -DPASSENGER_DEBUG -fPIC -I../.. -D_REENTRANT -DNDEBUG  -c pthread/thread.cpp
  1545  DEBUG: ar cru libboost_thread.a *.o
  1546  DEBUG: ranlib libboost_thread.a
-------------------------------------------------------
     Here
     - This package seem to be using internal libboost_thread library.
       This should be changed so that mod_passenger.so uses external (system-
       widely provided) libboost_thread-mt.so library
     - Anyway Fedora specific compilation flags are not correctly honored.

* Redundant output
  - I guess the "-v" option of "chmod -v 644 $script" is not needed...
    (Actually you are not using -v option for chmod on the below lines)
  - Also I guess rm -r"v"f is redundant...

* Document files
  - You don't have to write %doc attribute for files under %_mandir
    (as these are automatically regarded as %doc)

* Directory ownership issue
  - Please own %{ruby_sitearch}/passenger/
  - Please also check the directory ownership issues between subpackages.
    ! For example
      - The directory %{geminstdir}/doc is owned by -doc subpackage
      - -devel subpackage has %{geminstdir}/doc/definitions.h
      - -devel subpackage does not have "Requires: -doc"
      - So when -devel subpackage is installed with_out_ -doc subpackage
        installed, %{geminstdir}/doc is not owned by any packages

* %defattr
  - is missing on mod_passenger subpackage.

Created attachment 323017 [details]
mock build log for dist-f11

Forgot to attach mock build log...

(In reply to comment #2)
> First of all, the overall license this package is GPLv2 (not GPLv2+)

The license tag actually says GPLv2, not GPLv2+

(In reply to comment #3)
> By the way some pre-remarks
> 
> * BuildRequires
>   - This package won't build without "BR: rubygem(fastthread)".

Added BuildRequires: rubygem(fastthread) >= 1.0.1

>   - "BR: gcc-c++ findutils" are redundant.
> 

Removed these.

> * Requires
>   - Please check if all needed Requires are correctly added.
>     It seems that at least "Requires: rubygem(rack)
>     rubygem(fastthread)" are needed.
> 

Added these requires

> * boost dependency
>   - Well, when I try below to make build log more verbose
>     (please consider this)
(...snip...)
> -------------------------------------------------------
>      Here
>      - This package seem to be using internal libboost_thread library.
>        This should be changed so that mod_passenger.so uses external (system-
>        widely provided) libboost_thread-mt.so library
>      - Anyway Fedora specific compilation flags are not correctly honored.
> 

I seem unable to do this. Logfile attached

> * Redundant output
>   - I guess the "-v" option of "chmod -v 644 $script" is not needed...
>     (Actually you are not using -v option for chmod on the below lines)
>   - Also I guess rm -r"v"f is redundant...
> 

It's redundant, but it shows which files are chmod'ed or rm'ed; since it's a find with a couple of parameters I'd like to be able to track down what happens.

> * Document files
>   - You don't have to write %doc attribute for files under %_mandir
>     (as these are automatically regarded as %doc)
> 

Fixed.

> * Directory ownership issue
>   - Please own %{ruby_sitearch}/passenger/
>   - Please also check the directory ownership issues between subpackages.
>     ! For example
>       - The directory %{geminstdir}/doc is owned by -doc subpackage
>       - -devel subpackage has %{geminstdir}/doc/definitions.h
>       - -devel subpackage does not have "Requires: -doc"
>       - So when -devel subpackage is installed with_out_ -doc subpackage
>         installed, %{geminstdir}/doc is not owned by any packages
> 

Made %{geminstdir} shared between -devel and -doc package.

> * %defattr
>   - is missing on mod_passenger subpackage.

Fixed.

New SPEC: http://www.kanarip.com/custom/SPECS/rubygem-passenger.spec
New SRPM: http://www.kanarip.com/custom/f9/SRPMS/rubygem-passenger-2.0.3-3.fc9.src.rpm

(In reply to comment #5)

> > * boost dependency
> >   - Well, when I try below to make build log more verbose
> >     (please consider this)
> (...snip...)
> > -------------------------------------------------------
> >      Here
> >      - This package seem to be using internal libboost_thread library.
> >        This should be changed so that mod_passenger.so uses external (system-
> >        widely provided) libboost_thread-mt.so library
> >      - Anyway Fedora specific compilation flags are not correctly honored.
> > 
> 
> I seem unable to do this. Logfile attached

Well, would you attach the log?

(In reply to comment #2)

> What I am in trouble is that
> - What functions in ext/apache2/Hooks.cpp are actually based on mod_scgi codes
> - And I don't know for now how these functions are used in the other parts
>   of passenger source codes
> - So I am not sure if the code in Hooks.cpp under CNRI license won't conflict
>   with GPL.
> 
> spot, how do you think about this. For me the current status seems very
> obscure.

Problem 1: 
The functions which are copied are not marked, thus, the code is being reused without proper attribution. This means that the upstream for rubygem-passenger is in violation of the terms of the CNRI License:

"3. In the event Licensee prepares a derivative work that is based on
   or incorporates scgi-1.9 or any part thereof, and wants to make
   the derivative work available to others as provided herein, then
   Licensee hereby agrees to include in any such work a brief
   summary of the changes made to scgi-1.9."

No such summary exists.

Problem 2: 
You cannot copy code which is under a GPLv2-incompatible license into a source file which is marked as GPLv2, then compile it into a larger GPLv2 program. So, even if we knew which functions were copied, it almost certainly wouldn't matter. This code is non-distributable.

Unfortunately, there seems to be no easy way to fix Problem 2 (Problem 1 is easy enough to fix by the rubygem-passenger upstream). All releases of the mod_scgi code are under the CNRI license (although, changes made after 1.10 are under MIT). Some methods of fixing this issue would be:

1. Removing all of the copied code from mod_scgi 1.09, then replacing it either with clean-room written code (aka, code written by someone who has never looked at mod_scgi) or restructuring the rubygem-passenger code so that it is not necessary. 

2. Getting permission from CNRI to use that code under different (GPLv2 compatible) terms. They seem to use MIT for changes to that codebase these days, perhaps they would give permission for the copied code to be used under those terms?

However, until this issue is resolved, this one can't go any farther, sorry.

(In reply to comment #7)
> (In reply to comment #2)
>
> (...snip...)
>
> Unfortunately, there seems to be no easy way to fix Problem 2 (Problem 1 is
> easy enough to fix by the rubygem-passenger upstream). All releases of the
> mod_scgi code are under the CNRI license (although, changes made after 1.10 are
> under MIT). Some methods of fixing this issue would be:
> 
> 1. Removing all of the copied code from mod_scgi 1.09, then replacing it either
> with clean-room written code (aka, code written by someone who has never looked
> at mod_scgi) or restructuring the rubygem-passenger code so that it is not
> necessary. 
> 
> 2. Getting permission from CNRI to use that code under different (GPLv2
> compatible) terms. They seem to use MIT for changes to that codebase these
> days, perhaps they would give permission for the copied code to be used under
> those terms?
> 
> However, until this issue is resolved, this one can't go any farther, sorry.

Would another solution be to use mod_scgi >= 1.10?

Created attachment 323134 [details]
output on a compile with attached diff (1)

Attach output from a rake -v when compiling against the system-wide provided boost-static libraries

Created attachment 323135 [details]
diff to Rakefile to compile against %{_libdir}/libboost_thread-mt.a

Attach diff for Rakefile

(In reply to comment #9)
> Created an attachment (id=323134) [details]
> output on a compile with attached diff (1)
> 
> Attach output from a rake -v when compiling against the system-wide provided
> boost-static libraries

You should link against libboost_thread-mt.so, not against static
archive libboost_thread-mt.a.
But anyway linkage fails by other reasons like
-----------------------------------------------------------------
ApplicationPoolServerExecutable.cpp:86: undefined reference to `boost::this_thread::interruption_requested()'
-----------------------------------------------------------------
(and many errors). It seems that this is because Fedora ships boot 1.34
while these symbols are introduced on 1.36+.
So until Fedora upgrades boost we have to use internal boost.
In this case Fedora compilation flags must be treated correctly.
The following seems to fix this issue:
------------------------------------------------------------------
--- Rakefile.orig       2008-11-11 16:23:45.000000000 +0900
+++ Rakefile    2008-11-11 17:23:32.000000000 +0900
@@ -88,7 +88,7 @@
 
 subdir 'ext/boost/src' do
        file 'libboost_thread.a' => Dir['*.cpp'] + Dir['pthread/*.cpp'] do
-               flags = "#{OPTIMIZATION_FLAGS} -fPIC -I../.. #{THREADING_FLAGS} -DNDEBUG #{MULTI_ARCH_FLAGS}"
+               flags = "#{OPTIMIZATION_FLAGS} #{APACHE2::CXXFLAGS} -fPIC -I../.. #{THREADING_FLAGS} -DNDEBUG #{MULTI_ARCH_FLAGS}"
                compile_cxx "*.cpp", flags
                # NOTE: 'compile_cxx "pthread/*.cpp", flags' doesn't work on some systems,
                # so we do this instead.
-------------------------------------------------------------------

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #2)
> >
> > (...snip...)
> >
> > Unfortunately, there seems to be no easy way to fix Problem 2 (Problem 1 is
> > easy enough to fix by the rubygem-passenger upstream). All releases of the
> > mod_scgi code are under the CNRI license (although, changes made after 1.10 are
> > under MIT). Some methods of fixing this issue would be:
> > 
> > 1. Removing all of the copied code from mod_scgi 1.09, then replacing it either
> > with clean-room written code (aka, code written by someone who has never looked
> > at mod_scgi) or restructuring the rubygem-passenger code so that it is not
> > necessary. 
> > 
> Would another solution be to use mod_scgi >= 1.10?

Current mod_scgi seems 1.13.
http://python.ca/scgi/releases/scgi-1.13.tar.gz

However LICENSE.txt of mod_scgi 1.13 contains
--------------------------------------------------------------------
This version of the SCGI package is derived from scgi 1.10, released by
CNRI.  See doc/LICENSE_110.txt for the licensing terms of that release.
Changes made since that release are summarized in the CHANGES.txt file
along with a list of authors. Those changes are made available under the
following terms (commonly known as the MIT/X license).
--------------------------------------------------------------------
This reads that the codes derived from scgi 1.10 codes are still under
CNRI license, and only what is changed compared to 1.10 are under
MIT. So I don't think using latest mod_scgi will solve this issue.

Any news from upstream?

(In reply to comment #13)
> Any news from upstream?

I'm not sure what I can ask them to do?

Talk to upstream and CNRI about getting permission to use the mod_scgi derived code under different (GPL compatible terms), perhaps MIT?

I've contacted both upstream for CNRI as well as Phusion Passenger;

= CNRI =

I've requested taking into consideration re-licensing the original mod_scgi code-base to a GPLv2-compatible license, and they promised me to have their CEO look into it. Should they choose to do so, I think that would kill both birds with one stone, wouldn't it?

= Phusion Passenger =

They are considering re-licensing Phusion Passenger (thus rubygem-passenger) to the MIT license. Would that solve the incompatibility issue? And, would that make the package's license acceptable for inclusion in Fedora/EPEL?

So, if CNRI gives permission for rubygem-passenger to use mod_scgi 1.10 under GPLv2, that would resolve the licensing issue. Note that I said 1.10. If they only relicense their latest version, that won't do the trick.

If rubygem-passenger is relicensed to MIT, they're no longer incompatible with the mod_scgi 1.10 licensing, but they're _still_ in violation of the mod_scgi 1.10 licensing terms, which state:

"3. In the event Licensee prepares a derivative work that is based on
   or incorporates scgi-1.9 or any part thereof, and wants to make
   the derivative work available to others as provided herein, then
   Licensee hereby agrees to include in any such work a brief
   summary of the changes made to scgi-1.9."

They need to add a summary of the changes made to the mod_scgi code used, and then they'd be in the clear.

(In reply to comment #17)
> So, if CNRI gives permission for rubygem-passenger to use mod_scgi 1.10 under
> GPLv2, that would resolve the licensing issue. Note that I said 1.10. If they
> only relicense their latest version, that won't do the trick.
> 

I've asked for re-licensing of mod_scgi-1.09 and later versions (including mod_scgi-1.10)

> If rubygem-passenger is relicensed to MIT, they're no longer incompatible with
> the mod_scgi 1.10 licensing, but they're _still_ in violation of the mod_scgi
> 1.10 licensing terms, which state:
> 
> "3. In the event Licensee prepares a derivative work that is based on
>    or incorporates scgi-1.9 or any part thereof, and wants to make
>    the derivative work available to others as provided herein, then
>    Licensee hereby agrees to include in any such work a brief
>    summary of the changes made to scgi-1.9."
> 
> They need to add a summary of the changes made to the mod_scgi code used, and
> then they'd be in the clear.

Phusion Passenger's upstream told me they would list the changes anyway.

Please let me know if there is any news from the upstream.

(In reply to comment #18)
>
> Phusion Passenger's upstream told me they would list the changes anyway.

Jeroen, do you have any news on licensing and/or listing changes from upstream? 

After a quick look at the Phusion Passenger 2.1.1 (beta, released this week), I can't find a new license and/or a listing of those changes to the mod_scgi code in there.

phusion.nl, the upstream for mod_passenger promised me they would include a list of changes.

The CEO of CNRI (owner of the license and original mod_scgi code) has not responded to my inquiries but some of their employees have; long story short, no result from that angle either.

Would it be allowed to drop mod_passenger from the package (and thus not use the CNRI licensed code in the package for Fedora)?

Could we then continue the review just for rubygem-passenger?

(In reply to comment #22)
> Would it be allowed to drop mod_passenger from the package (and thus not use
> the CNRI licensed code in the package for Fedora)?
> 
> Could we then continue the review just for rubygem-passenger?  

If this package is still useful even if mod_passenger part
is dropped, you can just do it.

I'll explain my plan a little further because it may be a little controversial.

Since this package is a little hard to package (with or without mod_passenger), I was thinking I could ship rubygem-passenger, and out-comment all the parts that have to do with mod_passenger (and thus not ship, compile or include mod_passenger itself, just the .spec semantics, out-commented). That way, downstream users that want mod_passenger can derive from the .spec and .srpm already in Fedora.

However, my primary concern is that we would be shipping an intolerable SRPM (since the conflicting licenses prevent the sources from being shipped together as one).

Could you let me know what you think? Thanks!

Sorry for late reply.

(In reply to comment #24)
> I'll explain my plan a little further because it may be a little controversial.
> 
> Since this package is a little hard to package (with or without mod_passenger),
> I was thinking I could ship rubygem-passenger, and out-comment all the parts
> that have to do with mod_passenger (and thus not ship, compile or include
> mod_passenger itself, just the .spec semantics, out-commented). That way,
> downstream users that want mod_passenger can derive from the .spec and .srpm
> already in Fedora.
> 
> However, my primary concern is that we would be shipping an intolerable SRPM
> (since the conflicting licenses prevent the sources from being shipped together
> as one).
> 
> Could you let me know what you think? Thanks!  

I don't think we can provide srpm which cannot build within Fedora's
policy and which needs fixing to compile.

*** Bug 496941 has been marked as a duplicate of this bug. ***

Looking at the 2.2.2 release it looks like they've gone to to the MIT license and carry attribution for the functions.

When I see a 2.2.2 SRPM, I'll audit it to make sure everything looks okay from a licensing perspective.

New SPEC: http://www.kanarip.com/custom/SPECS/rubygem-passenger.spec
New SRPM: http://www.kanarip.com/custom/f11/SRPMS/rubygem-passenger-2.2.2-1.fc11.src.rpm

The source looks clean of licensing issues. You should really use:

License: MIT and CNRI

Lifting FE-Legal.

ell, for 2.2.2-1:

* %define -> %global
  - Please use %global instead of %define:
    https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

* Gem guidelines
  - First of all, this does not follow RubyGem packaging
    guidelines
    A This srpm is not created from original gem file
    B And actually the binary "rubygem-passenger"
      rpm does not work as gem.
      For example, "$ gem list passenger returns nothing.
      This is because specification .gemspec file is missing.
      Also, cached gem file is not installed.

    Well, for A:
    - When using gem as source, applying patches may be difficult.
      However if you want gem file can be expanded as tar archive.
      Also, usually I guess rake file supports "rake gem" to create
      modified gem file
    The issue B is more critical than A.

* Functionality
  - Note that some of (maybe all) scripts under %_bindir don't
    work. For example:
----------------------------------------------------------------
$ passenger-config --version
/usr/bin/passenger-config:27:in `require': no such file to load -- phusion_passenger/constants (LoadError)
	from /usr/bin/passenger-config:27
----------------------------------------------------------------


* Issues from build.log
----------------------------------------------------------------
    92  mkdir -p ext/nginx/libboost_oxt/boost
    93  g++ -Iext  -D_REENTRANT -I/usr/local/include -Wall -g -DPASSENGER_DEBUG -DBOOST_DISABLE_ASSERTS -o ext/nginx/libboost_oxt/boost/exceptions.o -c ext/boost/src/pthread/exceptions.cpp
    94  g++ -Iext  -D_REENTRANT -I/usr/local/include -Wall -g -DPASSENGER_DEBUG -DBOOST_DISABLE_ASSERTS -o ext/nginx/libboost_oxt/boost/once.o -c ext/boost/src/pthread/once.cpp
----------------------------------------------------------------
  - This still uses internal boost library. Please try to
    use system-wide boost library.
    Note that F-11/12 boost is 1.37.

   And:
----------------------------------------------------------------
   106  g++ -Iext -Iext/common  -D_REENTRANT -I/usr/local/include -Wall -g -DPASSENGER_DEBUG -DBOOST_DISABLE_ASSERTS -o ext/nginx/libpassenger_common/Utils.o -c ext/common/Utils.cpp
   107  g++ -Iext -Iext/common  -D_REENTRANT -I/usr/local/include -Wall -g -DPASSENGER_DEBUG -DBOOST_DISABLE_ASSERTS -o ext/nginx/libpassenger_common/Logging.o -c ext/common/Logging.cpp
----------------------------------------------------------------
  - Fedora specific compilation flags are not correctly honored.

* Misc cleanups
  ! Note:
    Before fixing below, please change to use gem file as
    source first.
----------------------------------------------------------------
mkdir -p %{buildroot}/%{geminstdir}
mkdir -p %{buildroot}/%{geminstdir} %{buildroot}/%{geminstdir}/lib/ext/
cp -a * %{buildroot}/%{geminstdir}
----------------------------------------------------------------
    - The middle line is not needed.

    - What is the following line for?
----------------------------------------------------------------
cp -a ext/phusion_passenger/native_support.so %{buildroot}/%{geminstdir}/lib/ext/.
----------------------------------------------------------------

* Directory ownership issue
  - The following directories should be owned by main package,
    not by -devel subpackage:
----------------------------------------------------------------
%{geminstdir}/ext/
%{geminstdir}/ext/apache2/
%{geminstdir}/ext/boost/
----------------------------------------------------------------

  - The ownership of the following directories is unclear:
----------------------------------------------------------------
%{geminstdir}/test/
%{geminstdir}/test/support/
----------------------------------------------------------------

ping?

ping again?

Hi Mamoru. I am one of the authors of Phusion Passenger, and I want to comment on several issues here:

== Vendoring Boost

We actually use a modified version of Boost, so Phusion Passenger will not compile against a stock Boost. I understand your concerns with regard to security, but please rest assured because we fully understand the security implications of vendoring a library. We will take full responsibilities for any security problems; that is, we will either backport security fixes or upgrade our vendored Boost to a newer version.

I hope this addresses any concerns that you might have with regard to vendoring Boost. If not then I'm eager to hear your thoughts on it.

== native_support.so

I don't think the cp command there is correct. Phusion Passenger loads native_support.so by calling "require 'phusion_passenger/native_support'", so the file must be inside a 'phusion_passenger' directory.

== -devel subpackage

Phusion Passenger should not be split into a regular and a -devel subpackage. We do not provide developer headers or libraries.

The source files in the ext subdirectory are not used during runtime, so they can be safely omitted from the RPMs. The lib subdirectory however must be present.

== test subdirectory

The test subdirectory contains unit tests and integration tests, and are only useful for Phusion Passenger developers, not for Phusion Passenger users. You can safely omit this subdirectory in the RPMs.

Also, could you wait for the 2.2.3 release, which is scheduled for next week? *A lot* of bugs have been fixed in this release.

(In reply to comment #34)
> == Vendoring Boost
> 
> We actually use a modified version of Boost, so Phusion Passenger will not
> compile against a stock Boost. I understand your concerns with regard to
> security, but please rest assured because we fully understand the security
> implications of vendoring a library. We will take full responsibilities for any
> security problems; that is, we will either backport security fixes or upgrade
> our vendored Boost to a newer version.
> 
> I hope this addresses any concerns that you might have with regard to vendoring
> Boost. If not then I'm eager to hear your thoughts on it.


CC-ing to Toshio. Would you comment on this issue?

Short answer: vendoring or bundling of libraries is not allowed.

Long answer: you can request an exception from FESCo but it's not likely to be granted with just the reasons you've given here.  When you bundle a library and, on top of that, modify it so it no longer matches upstream, you are starting the process of forking the library.  This may not seem like much of an issue to a developer.  (Hey, I just have one or two forked libraries, no problem for me to track security issues.)  But it makes life much more difficult for distributions.  There are places that this shows up.

* Security.  You touched on the fact that you've heard the security arguments but I'll go ahead on specify them here since there's many different places that this touches:
  - When a security flaw is discovered in a library and bundling is not allowed, The library can be fixed in a single package, that package rebuilt, and when users download it, all the applications that use it are immediately protected.  When bundling is allowed, the distribution has to find all the packages that the library occurs in by auditing source code or running a special tool over all elf files in all packages, then all of those packages have to be fixed, all of those packages have to be built, and users have to download and update each of the ones that they are using on their system before they are protected.  There is much more work involved when bundled libraries are involved.
  - With security issues, people want to remove as much lag as they can between announcement of a problem and the fix being available for users.  When libraries are unbundled, tools like vendor-sec can be used to alert distributions of problems that need patching in their packages before the announcement is made and then they can fix them with zero days of vulnerability.  If bundling of libraries occurs, then the problem becomes how to get fixes out to all affected packages.  If the distribution patches those packages, they must be careful to not leak the fact that there is a security vulnerability before they are allowed (which means they need to be careful who they share the information and what information they share with others).  OTOH, if they do not patch the packages bundling libraries, then those packages are not protected on zero day, but only afterwards.
  - When a security flaw appears, the program has to either update to a non-affected version of the library or backport a fix.  This can be problematic when the code of the library has undergone many API and code changes since the version that is being bundled and the security fixing patch is very widespread.  Many conflicts can arise that need time to fix when trying to backport the fixes but porting the application code to the new API version can also take a lot of time.
  - We cannot implicitly trust an upstream application to be on top of security issues that are released in the packages that they care about.  What happens if you are not following boost development and don't know that a security release has been made?  What happens if the developer that is responsible for watching boost development goes on vacation or quits your project?  What happens if your application ceases active development?  What happens if boost stops active development and security fixes start originating with distro patches?
* Forking is occurring.  Once an application starts bundling libraries, it's easy for the project to include local patches to the library to add features that upstream doesn't have or fix bugs that upstream hasn't addressed.  This has several negative effects.
  - When a security issue appears, it becomes harder to fix the application bundling the library.  If you attempt to upgrade to a newer version, you have to make sure your important local modifications get ported to the new version.  If you attempt to backport, you have to merge the upstream fix to your own code-base which may have conflicts with the local modifications.
  - When working with the library that comes from upstream, there is a community of people who are interested in that library to fall back on for help.  When working on your own private copy that community may not be interested in helping you work on your modified sources since they don't have control or knowledge of what your modified sources do.
  - Forking dilutes one of the strengths of open-source development.  Instead of a project getting stronger with more people supplying patches to help drive the project and build a bigger community, the community of people interested in it are splintering, developing more and more divergent code-bases, solving the same problem over and over in different ways in different private copies of the library.  Instead of everyone benefiting, everyone has to pay.
* Bugfixes.  Are usually of lesser importance from security issues but share the same issues arise.
* Old versions of code linger on.  If the application can bundle its own version of a library, the incentive to port to newer versions of the library are reduced.  This exacerbates the problems of security and bugfix issues.  Instead of progressively porting to newer versions of a library as time goes on, porting to newer versions becomes a chore that has to be performed at the same time as addressing a security flaw.  This puts time pressure on the project when the work could have been spread out over a longer period if only the porting had been done all along.
* Licensing.  Although licensing issues can crop up in any project, projects which bundle code from different sources together are a special source of concern.  They make auditing for license issues a larger project.

One last note related more to what you wrote than to the reasons that bundling is not allowed in Fedora.  You write that you are willing to:

"take full responsibilities for any security problems; that is, we will either backport security fixes or upgrade our vendored Boost to a newer version."

However, I wonder what that means to you.  Are you guaranteeing that you will be able to produce zero day updates of your packages that include the fixes to your bundled boost at the same time as boost itself is updated?  Are you saying that you will always be around to handle the problems that can arise?  What happens if your responses are not timely enough to meet the needs of our users?  Are you making a monetary guarantee?  I think that attempting to say that you take full responsibility in this context should be approached with caution on your part as what it entails could be interpreted in many different ways.

Toshio, I totally understand your points. We are well aware of all of those disadvantages of bundling/forking that you mentioned, but given our circumstances we decided that bundling/forking Boost is the best solution, despite all the aforementioned disadvantages. We made this decision a year ago, and to date we still think that it's the right decision.

Regarding responsibility: what I meant is that we take security very seriously, and that we will do our best to address any security problems, including those in Boost, as opposed to neglecting Boost security issues and happily keeping on vendoring the old, insecure version. We treat any problems in Boost as if they are problems in our own code. Is this explanation sufficient for you? If not, what are your concerns?

(In reply to comment #35)
> Also, could you wait for the 2.2.3 release, which is scheduled for next week?
> *A lot* of bugs have been fixed in this release.  

This is a package review for admitting the package in Fedora, not a bug-tracker ;-)

(In reply to comment #38)
> Toshio, I totally understand your points. We are well aware of all of those
> disadvantages of bundling/forking that you mentioned, but given our
> circumstances we decided that bundling/forking Boost is the best solution,
> despite all the aforementioned disadvantages. We made this decision a year ago,
> and to date we still think that it's the right decision.
> 

Note that what we're saying is that forking (and forming another upstream) actually is better then bundling (whether forked or not).

Also, we'd like to see an effort made to have the changes applied to boost be sent and accepted upstream even though that would include more work (*NIX -> win32 compat). Upstream would then at least stand a chance in whether they want to make stuff win32 compat. or whether to ignore/reject the patch at all. If you decide to do so, please keep me in the loop.

(In reply to comment #38)
> Toshio, I totally understand your points. We are well aware of all of those
> disadvantages of bundling/forking that you mentioned, but given our
> circumstances we decided that bundling/forking Boost is the best solution,
> despite all the aforementioned disadvantages. We made this decision a year ago,
> and to date we still think that it's the right decision.
> 
Bundling may be the right decision for an upstream development shop.  However,
it is definitely not the right decision for software that is shipped in many
distributions or for the conscientious system administrators.  Bundling shifts
the cost of forking so that developers bear less up front costs but
distributions and system administrators bear more costs later on.  In the case
of embargoed security fixes the cost is paid when there is time pressure to get
a fix created before a certain deadline is reached. In the case of bug-fixes
and enhancements (features, performance), the cost penalty is less urgent but
still occurs in the form of duplicate bugs, analysis, and debugging against the
bundled version of the library when that work has already gone into the system
library and resolved there via a patch or upgrade.

Not only is the cost shifted around but the benefits are decreased when the
forked library is bundled instead of released separately.  If the changes that
prompt you to bundle the library are useful for you, why won't they be useful
for others as well?  If they're useful for others, then bundling the library
forces them to reinvent the same types of changes you've made instead of
working with you to get their make a better product overall.

What is the status of this bug?

Toshio, Mamoru, as a startup company with finite resources we cannot afford to spend lots of resources on getting our changes accepted into Boost. If this means that Phusion Passenger will not be packaged by Fedora, then so be it; our users will find us and will use our software regardless.

Of course, if Red Hat can provide man power to get the changes accepted into Boost for us, or if Red Hat can hire us to get it done, then it would be a different story.

(In reply to comment #43)
> Toshio, Mamoru, as a startup company with finite resources we cannot afford to
> spend lots of resources on getting our changes accepted into Boost. If this
> means that Phusion Passenger will not be packaged by Fedora, then so be it; our
> users will find us and will use our software regardless.
> 
> Of course, if Red Hat can provide man power to get the changes accepted into
> Boost for us, or if Red Hat can hire us to get it done, then it would be a
> different story.  

You're so close to understanding the power of open source software, you just need to take a few more steps! :-)  Here's how I would have phrased it as an open source developer trying to leverage his community:

"""
I can see how bundling libraries can be a problem for distributions.  We just went through similar issues with Debian as well.  Unfortunately, that's not the market that we're actively pursuing at the moment so we're spending our time working on other areas of the code rather than trying to get our changes integrated into boost.  If you want to get those changes integrated into boost, the current code is here: URL_TO_GIT_REPO.  The current code is based on boost BOOST_VERSION.  The diff between the two will show you that we're basically doing: OVERVIEW_OF_CHANGES.  This is essential because WHY_IS_THIS_AN_ESSENTIAL_BUGFIX,_FEATURE,_ETC.  This has to touch MANY_FILES/FEW_FILES/INVASIVE_CHANGE/UNOBTRUSIVE_CHANGE.  The changes OTHER_NOTES_THAT_WILL_HELP_US_UNDERSTAND_THE_CODE_WELL_ENOUGH_TO_EXPLAIN_IT_TO_BOOST_UPSTREAM.

We've tried to get this into upstream boost before.  Here's a link to the mailing list discussion of that where boost upstream outlines the issues they had with our present code: URL_TO_PREVIOUS_DISCUSSION_WITH_BOOST.

If you can get this into boost's upstream we'd be happy to TAKE_PATCHES/MAKE_OUR_OWN_CHANGES to phusion passenger to UNBUNDLE/ONLY_BUNDLE_IF_SYSTEM_BOOST_IS_UNAVAILABLE boost.

Hope that helps!
"""

In open source, people help write code because they have an itch to scratch.  Red Hat doesn't ship phusion passenger so you need to address the people who actually have an itch instead.  Jeroen and a number of people CC'd to this bug fit that bill as they want to see phusion passenger shipped on Fedora and understand that bundling libraries is a drawback for their deployments.  Since they didn't write the modifications to boost, be sure to give them the tools and knowledge they need to successfully discuss the changes with boost upstream so that there's a chance of getting the changes integrated.  Whether phusion passenger gets into Fedora will depend on whether you get one of them to pick up the task and get the changes merged upstream.

Also remember that getting changes integrated upstream is often like a negotiating session. Since you're trying to get someone else to do your negotiating for you, be sure they understand what your requirements are before sending them off to work.  Maybe boost will give you the feature you want but it can't use the API you wrote.  Maybe the feature is unacceptable but boost will make changes so you can write the feature into your application layer instead.  It's important for the person who takes up this task to know what things are negotiable and which are not.

Toshio, you are absolutely right. I apologize for the short reply that I sent; I hadn't given it a lot of time and thought because my hands are full with client work right now. Lately I'm not even able keep up with providing community support for Phusion Passenger, let alone going through the negotiation process of getting changes or similar features accepted into Boost.

It is possible for me to provide an overview of changes and rationale for the changes, but that takes time as well, and I cannot spare any. Even my weekends and evenings are fully scheduled. I estimate to have more time by October so if you can wait until then then perhaps we can work something out.

Look, I perfectly understand your challenges, and I myself as undoubtedly many other people as well are in quite the same situation.

Like Toshio suggested, I'm doing this because I have an itch to scratch. Even though of particular benefit to myself (I don't have time to run in circles supporting weird versions of boost), it could result in huge benefit to you as well as your customers.

So, if you could please take the time to press "Print" on the documentation, I can come and pick it up next week in Apeldoorn.

Thanks for the offer, but there is no documentation at this time that describes what the Boost changes are and why they were made, except for the Git commit logs. Writing one will take some time and the earliest date at which I can do that is probably October.

Is there any other way I can help you, besides going renegade and attempting to fix this by working on the code myself, without help?

Created attachment 355185 [details]
diff between boost in passenger/master and boost/tags/Version_1_35_0

Attached a diff between ext/boost in the master branch of passenger and the boost/ directory in boost's tags/Version_1_35_0

Sorry, I haven't had the time to post a proper reply.

Please feel free to go renegade if you can't wait until October.

I can wait until October, but I do want to understand in full what changes we're talking about so that we may actually get some work done in October ;-)

I'll give a few quick hints then.
- You see I modified the boost::thread class. The modifications are necessary in order to implement oxt::thread.
- Some exception classes like thread_resource_error are modified to inherit from oxt::tracable_exception.

oxt::thread and oxt::tracable_exception and their purposes are documented in their respective header files.

So, October is over, and there still a ruby community eager to have passenger on Fedora/RHEL.  Are we any closer to reaching an end to this?

My status is I'm slowly growing some more C coding skills to fold oxt/boost/passenger back onto what is available outside of the passenger realm ;-)

Sorry, still too busy.

And now?

Right now I'm working on a client project with a tight deadline.

Now, since this issue has been opened for about a year now I'll just be honest with you. As part of a bootstrapping company I find it very hard to invest time in getting my changes merged upstream. You mentioned that by submitting changes upstream it'll alleviate myself of having to maintain the changes. This is correct, but the flip side of the coin is that I'll have to get the changes accepted in the first place. Some of the changes we made to Boost are quite intrusive and change source and binary compatibility (we've modified some basic classes to derive from classes in our own library, for example) and getting all changes accepted into Boost will be a long, hard process. So what's easier, maintaining the changes ourselves or going through the trouble of submitting the changes upstream? I still think the former is easier.

Debian is already packaging Phusion Passenger and they know about our usage of Boost. We already have contacts with people who are willing to maintain third party yum repositories. Unlike the official Fedora repository, these third party repositories can collaborate closely with us and won't lag behind official releases for weeks or months.

So all in all I do not see any benefits in getting the Boost changes submitted upstream; in fact, I only see drawbacks. If you want to do it I won't stop you, but without some sort of financial compensation for my time I won't do it. I have to work to pay for my food after all.

Is there any chance that this will ever get packaged into an rpm?  It is disappointing that there are packages for Ubuntu but not for RedHat.

You can only be as disappointed as much as you're willing to sink your teeth into solving the actual problems stated above.

That said, before such aforementioned work is actually done, chances are this package will not be included in the regular Fedora distribution or EPEL add-on repository.

That said, there are packages;

http://mirror.nl.ergo-project.org/repositories/

(In reply to comment #59)
> You can only be as disappointed as much as you're willing to sink your teeth
> into solving the actual problems stated above.

That would be very difficult since I know absolutely nothing about ruby.  My only interest is in setting up a scalable puppet system and as a sysadmin, for obvious reasons, I would prefer to use the native OS package manager, rather than a package management system built-in to and only useful for one specific software tool.

> That said, before such aforementioned work is actually done, chances are this
> package will not be included in the regular Fedora distribution or EPEL add-on
> repository.

Why does this have no chance of being included in a RedHat distribution?  Is it a licensing issue or something else?

> That said, there are packages;
> 
> http://mirror.nl.ergo-project.org/repositories/

Thanks, I will take a look at this next week.

(In reply to comment #60)
> (In reply to comment #59)
> > You can only be as disappointed as much as you're willing to sink your teeth
> > into solving the actual problems stated above.
> 
> That would be very difficult since I know absolutely nothing about ruby.  My
> only interest is in setting up a scalable puppet system and as a sysadmin, for
> obvious reasons, I would prefer to use the native OS package manager, rather
> than a package management system built-in to and only useful for one specific
> software tool.
> 

Then maybe reconsider declaring yourself disappointed -others do in fact (attempt to) sink their teeth into solving the actual problem, so that you can sleep at night without having to worry whether there is a new problem on the horizon.

> > That said, before such aforementioned work is actually done, chances are this
> > package will not be included in the regular Fedora distribution or EPEL add-on
> > repository.
> 
> Why does this have no chance of being included in a RedHat distribution?  Is it
> a licensing issue or something else?
> 

I said Fedora distribution and EPEL add-on repository, and I did not say Red Hat distribution. These are completely different. The Fedora Project will not accept it as part of it's Fedora distribution or EPEL add-on repository, and Red Hat can do as it wishes -however it is still very much unlikely they'll ship it as it is.

As to the reason this package is not / can not / will not be included; Please read the comments in this bug/review request. Long story short, Passenger ships a forked version of the C++ Boost libraries -and lags behind a dozen versions or so.

I just spent a little while going through this and trying to figure out what exactly would need to be done to get passenger working with the stock boost in Fedora. The following are issues we may run into:


Issues which shouldn't post a problem:
* Additional 'defined' checks simply check for netbsd or solaris, neither of which is applicable here

* elif BOOST_PP_INTERATION_DEPTH changes, made for gcc 4.4 compatibility. Since boost is already working against gcc in Fedora, this change isn't needed (correct me if I'm wrong) 

* whitespace changes in non_type.hpp, which I'm guessing shouldn't be there as nothing else is different



Issues which might pose a problem:
* the vendorized boost is at version 1.35 while in Fedora 14 we are going to be at 1.44. I believe this just about summarizes the issue Fedora has with vendorized libs, and I'm not sure if the thread library between these two versions are compatible.

* the condition_varaible and mutex destructors and the condition_variable::wait method try to repeat the necessary pthread destruction code until it is successful, as opposed to attempting it once and verifying the result with BOOST_VERIFY. As of right now I'm not sure why it does this, it seems to me that if the pthread destruction call fails once, it'll fail everytime, and if this is necessary on Linux. It may not be, this change maybe only for solaris or something; some additional clarification would be great. If it is required this is going to be very hard if not impossible to remove as if assertions in boost are enabled, BOOST_VERIFY will terminate a running program (if the condition being verified is false) and there isn't much we can do about it.

* changes to the exception hierarchy and additional exception information has been added to thread_exception, thread_resource_error, and thread_interrupted, to add backtrace and system_error_code support. Unless I'm mistaken these are purely informational only (for debugging) and thus this shouldn't pose an issue (once again correct me if I'm wrong). The apache hooks may have to be modified though to remove the TRACE_POINTs (or perhaps not, still needs more exploration)

* the thread constructor and start_thread method take an optional stack size. From my current understanding this is platform specific, and since boost is already working on Fedora Linux x86 and x86_64 as is, this is unneeded.



That about covers it, I am going to attempt to see if I can passenger working against boost in Fedora, but might not be able to for the aforementioned reasons. If anyone knows any fault in my analysis above, or some other reason why I won't be able to, please correct me, so that I don't invest a ton of time into this. Also if I am successful, it may come at the price of a heavily modified passenger source, but we'll tackle that if/when we get to that point.


P.S. would creating a separate upstream project resolve this issue without any of this? Obviously someone would have to maintain this, but would something like a passenger-boost project work?

Passenger 3's Boost is actually based on 1.42.0. A lot of API breakages have happened between Boost 1.35 and 1.42. I believe it's impossible to support multiple Boost versions without making our code a mess with #ifdefs everywhere. We only support one specific version of Boost for the time being: the one we use during development.

The condition_variable and mutex error checks do not retry on all errors: they only retry on EINTR. BOOST_VERIFY aborts even on EINTR but EINTR is usually not fatal, it just means you need to try later. This issue bit us on one of our customers' production systems.

Exception hierarchy change: yes they're informational. If a thread fails to spawn (i.e. thread_resource_error is thrown) then we'll want to show a backtrace. This is not possible without patching Boost and changing the ABI.
Please do not remove TRACE_POINTs. They're an essential part of our system inspection features. Right now it's possible to query the backtrace of all threads in Phusion Passenger during runtime thanks to the TRACE_POINTs. If you get rid of that then our users will lose their ability to see what's going on and will introduce a lot of headaches when things go wrong.

Thread constructor: it is not unnecessary, we do this intentionally to reduce the VM size. We create a lot of threads. The default thread stack size on Linux is 8 MB, so unless we reduce the stack sizes we can't create many threads and the VM size will grow very large, giving users the impression that we're memory hungry.

(In reply to comment #63)
> Passenger 3's Boost is actually based on 1.42.0. A lot of API breakages have
> happened between Boost 1.35 and 1.42. I believe it's impossible to support
> multiple Boost versions without making our code a mess with #ifdefs everywhere.
> We only support one specific version of Boost for the time being: the one we
> use during development.

Can you please elaborate on this. Looking at the passenger repository, it seems boost is still marked as being on version 1.35

http://github.com/FooBarWidget/passenger/blob/master/ext/boost/VERSION.TXT

and the last commit I see relating to 'updating boost' is the update to 1.35

http://github.com/FooBarWidget/passenger/commits/master/ext/boost

At what point did you update to 1.42? In any case for this submission, boost is still at 1.35 which I hope isn't so incompatible with 1.44 (shipping on F14) that it won't compile against it.


> 
> The condition_variable and mutex error checks do not retry on all errors: they
> only retry on EINTR. BOOST_VERIFY aborts even on EINTR but EINTR is usually not
> fatal, it just means you need to try later. This issue bit us on one of our
> customers' production systems.

This is good to know that this was to solve an edge case on a production system. Which OS was this customer running btw? If it something other than Linux (and maybe even if it's another distro) we might not run into this onto Fedora and thus this can be omitted.


> 
> Exception hierarchy change: yes they're informational. If a thread fails to
> spawn (i.e. thread_resource_error is thrown) then we'll want to show a
> backtrace. This is not possible without patching Boost and changing the ABI.
> Please do not remove TRACE_POINTs. They're an essential part of our system
> inspection features. Right now it's possible to query the backtrace of all
> threads in Phusion Passenger during runtime thanks to the TRACE_POINTs. If you
> get rid of that then our users will lose their ability to see what's going on
> and will introduce a lot of headaches when things go wrong.

I would like to include this (and everything else), but it looks like there is no way to include passenger in Fedora without this being removed (eg passenger needs to work against the stock boost). So its either remove this debugging stuff or not ship passenger at all. Like you said in a previous comment, if anyone wants any of these features I'm removing they can simply get passenger via gem.


> 
> Thread constructor: it is not unnecessary, we do this intentionally to reduce
> the VM size. We create a lot of threads. The default thread stack size on Linux
> is 8 MB, so unless we reduce the stack sizes we can't create many threads and
> the VM size will grow very large, giving users the impression that we're memory
> hungry.

Have you guys ever considered using thread pools? Launching many threads indiscriminately is often considered bad practice, and you may get a performance boost by making use of a pool. Regardless its good to know that this is simply to make the memory footprint smaller, and it should work without this. Just curious is there any way to reduce the default thread stack size, systemwide lets say, without modifying/recompiling every program?


I'm starting to hack at this to see if I can get it working. Based on your response the only thing that makes me not sure if this will work is the discrepancy between boost versions, but we'll just see if that poses and issue.

> Can you please elaborate on this. Looking at the passenger repository, it seems
> boost is still marked as being on version 1.35

Passenger 3 will be based on 1.42. Passenger 3 isn't out yet but will soon be.


> Which OS was this customer running btw?

Debian. But even if it was a different OS, the checks should remain. Returning EINTR is POSIX compliant behavior I believe, so apps should take care of it properly.


> I would like to include this (and everything else), but it looks like there is
no way to include passenger in Fedora without this being removed (eg passenger
> needs to work against the stock boost). So its either remove this debugging
stuff or not ship passenger at all.

In that case I prefer to decline packaging. We cannot agree on having an important feature like that removed. We want every user who uses Phusion Passenger to experience the same quality, and we will find it unacceptable if quality differs among different OS packages.


> Have you guys ever considered using thread pools?

Yes. Thread pools will not do. We really need all those threads, they're used for I/O concurrency so limiting them will not solve the problem.

The default stack size can be changed through ulimit. However I disagree with the notion that the default stack size should be respected. There's a reason why pthread_attr_setstacksize() exists, and we use it for a good reason.


It is certainly possible to patch Phusion Passenger to work with normal Boost. However doing so degrades the user experience in such a way that we cannot approve of such changes. Please note that Phusion Passenger is a registered trademark in the European Union and a pending trademark in the United States. If you want to ship a patched version of Phusion Passenger with changes that we do not approve of, we have to ask you to rename it something else.

I am putting Benjamin Kosnick on cc since he maintains the boost package in Fedora, and hopefully can shed some light on some of the points that seem to be shortcomings in the boost API (not being able to set stack size for threads, handling of EINTR after waiting)

Benjamin can you weigh in on the discussion starting with comment #62 ? I suspect that some of these things would be welcome enhancements to boost.

(In reply to comment #65)
> Please note that Phusion Passenger is a registered
> trademark in the European Union and a pending trademark in the United States.
> If you want to ship a patched version of Phusion Passenger with changes that we
> do not approve of, we have to ask you to rename it something else.

???
Then would you clarify the license?
If Phusion Passenger upstream has such policy, I guess we will anyway
has to rename such software on Fedora
- Note that Debian does not accept "firefox" or so due to the same reason.
  And there are so many discussion also on Fedora about whether we should
  rename "firefox" to something else.

And if we decide the license is "not free", we won't accept such software
anyway.

The license is just MIT license. It has got nothing to do with any trademark rights. I find it interesting that you see this as an issue. Red Hat does the same thing: one cannot modify RHEL and call it Red Hat, which is why Fedora and CentOS are named Fedora and CentOS and not "Red Hat Community Edition" or whatever.

It is fine if you do not accept it. We will work with third party packagers instead.

Hongli, I believe he is asking if you have a separate license for use of the "Phusion Passenger" trademark. For example, Fedora's trademark guidelines are here:

https://fedoraproject.org/wiki/Legal:Trademark_guidelines

Also, here is Mozilla's trademark policy:

http://www.mozilla.org/foundation/trademarks/policy.html

It would clarify things for us significantly if you could put into writing the terms under which use of your trademark is permitted.

I believe we have a fundamental disagreement in packaging policies. I believe it's easier for both of us if you close this issue and that we work with third party packagers instead.

In regards to #62, from my analysis of the modified boost diff, the major sticking point is the hack to boost::thread_exception.

I suggest a different error handling design. Instead of making thread_exception into what phusion passenger wants, derive

oxt::traceable_thread_exception from the un-molested boost::thread_exception and your custom tracing bits add your changes there. This will cleanly separate out the client code (phusion passenger) from the library code (boost), remove the need to deal with upstream boost, help with packaging your software by removing your need to ship a custom boost,  and allow you to use C++0x threads in the future. Win, win, win. Please consider it.

The point of the modification was to have Boost.Thread itself throw exceptions that are derived from thread_exception. Making a seperate tracable_thread_exception won't solve that problem. But, as I've stated earlier, there are fundamental packaging policy disagreements between us and I believe it's easier to maintain the status quo.

Wow, this is amazing how this has degenerated.  Ruby has to be one of the only popular languages without a standard apache module.  Do the Phusion people not see the value in getting their software packaged into a major distribution like RedHat?  And getting it into Fedora would be the first step towards that happening.  Do they not realize the wide acceptance and popularity they would get (at least in the US and maybe worldwide) by being bundled with a large vendor's OS out of the box?  Instead they want their customers to jump through hoops to install their software.

Why do software tools have to reinvent packaging systems outside of the OS package management system anyway, like Ruby's Gems, Perl's CPAN & Python's Eggs.  From a sysadmin perspective, this is horrible and should be avoided at all costs.

Anyway, since my main objective was to install a scalable puppet server, and since puppet recently released 2.6.1 which supports JRuby, I guess I will now be forced to look into that as an alternative, with tomcat.

I'm sorry I reopened this can of worms.

(In reply to comment #72)
> The point of the modification was to have Boost.Thread itself throw exceptions
> that are derived from thread_exception. Making a seperate
> tracable_thread_exception won't solve that problem. But, as I've stated
> earlier, there are fundamental packaging policy disagreements between us and I
> believe it's easier to maintain the status quo.

Can't you simply catch the Boost Thread exceptions, and in the error handler
throw your own custom exceptions? You can interpret the thread errors in your
catch blocks and add as much debugging information as you need there. I'm
actually surprised that hacking boost took less time than doing it this way. I
suppose if you don't know all the boost thread exceptions, you'd want the
backtraces all the way though boost, but this doesn't seem like the best
practice.

Also what environments are you passenger on. It may be more than Linux/g++ but
if it isn't doesn't gcc have some non-standard means of obtaining backtrace
information for c++ exceptions?

I think everyone loses out with the status quo. There are Linux users who
_must_ use rpm / yum to install software on their systems, and using gem is not
an option. Thus you lose potential users of your software and we lose another
supported / tested deployment scenario. It would be great if we can all work
together to find and develop and acceptable solution that works for everyone. I
understand that time is very tight nowadays, but this is why we're volunteering
to help come up with and develop a solution.

(btw no flamewars here please, everyone is right for their own reasons, just need to come up with the right solution)

I'm one of those users who *must* use yum/rpm to install software.  Custom compiled software, be they gems, perl modules, c programs, etc, are forbidden.  I'd love to see this included in Fedora, because then I could request it for the RHEL 6 EPEL repo.  Are there still these two issues outstanding?

1.  Vendored boost library
2.  License incompatibility with scgi functions

Thanks,
Nathan

Nathan, Jason: yes, the license incompatibility has been solved. As for RPM/YUM packages, we are working on setting up a third party YUM repository as we speak, and we expect packages to be available in the near future. Please keep an eye on our blog for updates.

I'm working on getting the necessary people to understand this package needs an exception in order to get Real Life Problems fixed.

We are pleased to announce third party RPMs for Phusion Passenger: http://blog.phusion.nl/2011/01/04/phusion-passenger-native-packages-for-redhatfedoracentos/

Can someone please clarify why this review is stuck?

Does the packager need to take action?
Or does the reviewer need to analyze again the package after the licencing issue has been resolved?

Thanks,

The package contains a bundled library and upstream is unwilling to have it unbundled and still use the passenger trademark.  No one has put in a request to the FPC to grant an exception for this package to bundle libraries so that's still a possibility to resolve the bundled library issue but there doesn't seem to be any good exception material here so I'm unsure that it would pass.  Another option would be to fork the code and unbundle using a non-trademarked name.

Who has the technology to "put in a request to the FPC to grant an exception for this package to bundle libraries" ?

Can I do it, if so can someone explain how?

Things are getting to the point that Fedora is going down the drain ... we cannot even purchase VPS servers with Fedora on them due to the lack of long term support, fast cycles and lack of proper support for things like RoR, in most part due to this very issue with passenger. Thanks the unwise people upstream that are dragging their feet.

The instructions are at 
http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
Please be sure you answer all of the questions presented there.  It is wise to avoid hyperbole such as "Fedora is going down the drain".

cpg, we now provide third party packages so I believe it's no longer necessary for Fedora to ship Phusion Passenger. Our packages are updated very quickly against official releases too.
http://blog.phusion.nl/2011/01/04/phusion-passenger-native-packages-for-redhatfedoracentos/

Hongli,

those of us that need this for clean RoR installations in our RH based environment will take those and suffer through, but at the end of the day I'm sure many of us would rather the packages were actually capable of being provided by Fedora/CentOS/EPEL.  The points behind the No Bundled Libraries restriction are very valid, and speaking from experience I can say we've been burned over the long run by vendors with good intentions and poor follow through on staying up with changes.

Greg, if you look at our reputation we have a very strong follow through of staying up with security updates. Organizations like New York Times, Wikipedia, Pixar etc don't use our stuff for no reason.
On top of that, trying seeing our use of Boost in perspective. We pretty much only use the threading and smart pointer stuff. The former is a thin wrapper around the OS APIs. Neither of them are responsible for processing user or network input. I would be very surprised if they even make vulnerabilities possible, ever.

(In reply to comment #83)
> cpg, we now provide third party packages so I believe it's no longer necessary
> for Fedora to ship Phusion Passenger. Our packages are updated very quickly
> against official releases too.
> http://blog.phusion.nl/2011/01/04/phusion-passenger-native-packages-for-redhatfedoracentos/

Thanks for maintaining these packages externally, but I think Fedora users would be served better if passenger were distributed along with the rest of the OS.

Maintaining a system using multiple disparate repositories is a PITA, especially with yum.

What about keep Passanger as it is and provide boost-passenger for fedora, i.e. fork of boost? It may be even just subset of boost required by passenger? Of course I expcet that this boost fork would be maintained by Phussion, since they have to do it anyway IMO.

Don't take me wrong, I know it is workarond. However forking of project is very natural in opensource, nobody will say "Oh, we cannot have LibreOffice in Fedora, because it is fork of OpenOffice.org". The same would apply for boost-passenger.

another reason to get it in Fedora, Hongli, is that your rpms are broken (bad signature).  This was reported on your blog entry about the repos in January and still happens for me as of last week.

Well, passenger 3 is out and none of the srpm or spec files listed above, so I modified the latest srpm release to get something close to what we want I think.  Since this original packaging request, it looks like the passenger available through phusion's repos also bundles a version of nginx.  My changes to the spec file basically are to remove that (as a consequence, it only builds the apache plugin, not the nginx one as well).  The srpm is available at
http://dev.laptop.org/~bobbyp/passenger/rubygem-passenger-3.0.4-2.fc14.src.rpm
I'll attach the modified specfile (and a diff between the original and it) to this bug.

It still has the vendored boost, but I'm hoping we can apply to get an exemption as api and functionality has changed.  The biggest problem I see is this note on [1]: 'If no attempt has been made to push the changes upstream, we shouldn't be supporting people forking out of laziness'.

And if phusion won't allow us to use passenger, we can probably rename the library to 'straphanger' or some such fairly easily.

Regards.


1 - http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

Created attachment 485557 [details]
passenger 3.0 specfile without nginx support

Created attachment 485558 [details]
diff between attached 3.0 spec and original

one last thing, there are 2 deps for passenger 3.0 that are not in Fedora (14, at least). rubygem-file-tail and rubygem-spruz.  I used the versions from Phusion's repos, but these would need to be accepted into the fedora repos too

Our Boost changes are highly Unix-specific. Phusion Passenger makes no attempt to support Windows. However Boost must support Windows. I do not want to waste my time on writing Windows support for our patches.

Furthermore we're getting rid of the file-tail (and, indirectly, spurz) dependency for the next release.

Is it just me or does the RPM lay down source files and expect the resulting system to compile the module?  I've had this happen on 3 systems now.  I would think this would not be expected behavior for an RPM.

Hongli -

Like many others, I'd like to see Passenger be accepted into Fedora (and EPEL). I am willing to put some time and effort into making it happen.

In comment #43, it seems that you are/were amicable to your modifications being pushed upstream. I'd be happy to take on the task of working with the Boost community to get your changes merged. So, let me ask you directly: If I can get your modifications upstream, will you be willing to drop your bundled version of Boost? 

I will caveat this somewhat by saying that, after having read over your changes, the Boost folks may want to recommend some changes to your implementation. I will do my best to make sure your code gets upstreamed with as few changes as possible, but I think it's reasonable to expect that it won't be perfect. 

I'd like to have you be involved in that dialogue as much as possible. I realize you're a busy person and can't really dedicate much time to something that doesn't provide you with immediate benefits. However, if you're willing to exchange a few e-mails with me through this process, it will help me work through any issues that come up that much more quickly.

I have started discussion on the boost mailing list about upstreaming the two biggest portions of passenger's boost fork:  the optional stack_size argument in thread::start_thread and the additional stack tracing and exception handling.

Initial response to these potential features is very positive. The thread is archived here, among other places: http://marc.info/?t=131722868200003&r=1&w=2 

The patches will require some clean-up and adjustments to support Boot's requirement for cross-platform compatibility. However, I don't think that's a show stopper.

very encouraging!  Thanks for getting this rolling again Brett.

Hi Brett. If you can get the changes bundled in Boost then what we can do is adding an option to disable compiling against the bundled Boost, and instead compile against the system Boost. We'll still want to continue bundling Boost for the sake of platforms on which installing Boost is a hassle (e.g. OS X), but I believe adding such a compile option should solve your issue as well.

I do not mind it if the API that the Boost folks develop is slightly different than ours.

It should be noted that our backtrace support is not automatic. It relies on the user manually specifying backtrace points with TRACE_POINT() macros. It's documented here: https://github.com/FooBarWidget/passenger/blob/master/ext/oxt/backtrace.hpp
The backtrace system is built in such a way that it can be integrated with oxt::thread. oxt::thread is like boost::thread but also registers each thread on a global thread list, each of which has its own thread-local backtrace list. This way, one can obtain the backtraces of all threads during runtime.

Hongli - A disable option works for me. Do you need me to report a bug/feature
request, or can I just look for it in an upcoming release?

Thanks for the heads-up about the macros. I know being able to opt-in is a
concern for the Boost folks, so I'll make sure to highlight that.

Lastly, if you have the time to follow the conversation on the boost-list,
please let me know if there are any proposals that you have any objection to or
will have difficulty working with. Also, if I misrepresent your code at all,
don't hesitate to ping me either here or directly at blentz.

You can just look for it.

Thanks for the effort.

I've got a couple patches that I'm pushing the Boost folks to merge. I'm still unravelling the stack trace exception patch into something workable. 

Discussion on EINTR patch:
http://marc.info/?t=132269010900002&r=1&w=2\

Discussion on configurable stack size patch:
http://marc.info/?t=132266986500004&r=1&w=2

Hongli - If you have a moment to take a look at the discussion around your EINTR modifications. According to the Boost folks, POSIX specifically forbids pthreads from returning EINTR. You already mentioned that this happened on a customer's system in production, do you recall what OS it was?

It might make that particular patch a bit more palatable if we can couch it in some ifdefs, so that it's only necessary on the broken platform. 

Additionally, it might allow us to file a bug with the upstream OS if the behavior still exists.

If I remember correctly it was Debian. It's very important to me that lock() handles EINTR. I don't think couching it with #ifdefs is a good idea though because compiling a whitelist or blacklist is just too much work. Even in the same OS, some versions of the threading library may be broken while some versions may not, and later versions may introduce regressions. It's just easier to always check for EINTR.

While it is technically correct that returning EINTR indicates a bug in the OS, relying on the OS vendor to fix the bug is not a good practical solution.
- Different vendors fix bugs at different paces. Fixing the bug can take months or years but the user wants the software to work yesterday. He does not care which party is to blame.
- Some users use old versions of an OS by choice.
- Some users are part of an organization whose IT department does not allow OS upgrades, for whatever reason.
- Upgrading Phusion Passenger is far easier than upgrading the entire OS and is less risky.

Ah, it was some flavor of Linux, not OS X or some other non-Linux OS?

Among other things, I am trying to anticipate next steps if the Boost folks decide to not accept that portion of your fork. Being that it's a violation of POSIX, it's totally fair for them to say that it's not within the scope of their library to workaround a broken threading library. The "correct" fix would be to fix the threading library itself. (I don't like this possibility, but I must acknowledge that it exists.)

However, I completely agree with you about all of the implications of what a decision like that means.

For now, I'm hopeful that they'll still accept it because it makes the whole library's threading support more robust. 

In the meantime, if you saw this behavior on Linux, I will look into whether this issue has already been reported and fixed in pthread. That information would be good to know, regardless.

Update on the stack size patch:

It looks like the Boost folks are going with a more general solution, which is detailed in this ticket:
https://svn.boost.org/trac/boost/ticket/2741

I'm pleased to report that Boost has accepted the EINTR patch in its entirety.

https://svn.boost.org/trac/boost/ticket/6200


This just leaves the stack trace bits as the last major portion to be upstreamed, AFAIK.

Glad to hear.

Just FYI, the stack trace bits appear to have been commited to be part of boost 1.49.0 (at least the more generic version as mentioned by Brett Lentz):

https://svn.boost.org/trac/boost/ticket/2741#comment:9

I'm glad to hear progress is being made here on this as I'm really hoping Passenger will make my puppet server scale to bigger loads better.

Can someone here that is more familiar with all the gory details of getting Passenger into Fedora give me some kind of estimate of how close we are now?  Are these recent gains that have been announced just a few of many more that will have to be gained or do they represent some of the final bits falling into place?  Are we 25%, 50%, 75% or 99.8% the way there?

John -

I believe there are three modifications that Passenger has done to the base Boost libraries. Two of these three have been upstreamed, and we're actively working on getting the third modification ready for upstreaming.

However, one of the two upstreamed changes was addressed with a more generic patch, and will require modifications to how Passenger handles that particular detail.

I can't provide any specific estimates on when Passenger will land in Fedora. However, I can say that we're actively working on it.

Brett,

Thanks for that update and I really appreciate all the hours you and others are putting in to make this happen.  I'm anxious to see how it will improve things .... but not so anxious that I have a need to wander outside of yum repos to find out.

It sounds as though Boost is the big challenge ATM, although I also recall reading somewhere that there was a whole slew of other packages that would need to be brought into Fedora as well; hopefully those are all moving along well too.

Okay, so...  

After the Boost community has raised some valid concerns about certain portions of the changes in Passenger's fork, I've applied for a bundling exception with the FPC. 

The bundling exception was GRANTED, conditional that I would be the package's maintainer. (See: https://fedorahosted.org/fpc/ticket/160 )

With that, it appears that we're finally clear to resume the package review.

I will be posting a new spec and srpm sometime in the next few days.

Good to hear that things have been solved.

I'll update these to the later version and the new Ruby ABI requirements for rawhide/f17 later today:

New SPEC: http://git.kolabsys.com/rpm/rubygem-passenger/plain/rubygem-passenger.spec
New SRPM: http://mirror.kolabsys.com/pub/redhat/kolab-2.4/el6/development/SRPMS/rubygem-passenger-2.2.15-1.el6.kolab_2.4.src.rpm

Still a work in progress, and only shipping the gem and apache module.

Builds using mock for el5, el6, f16, f17 and rawhide.

New SPEC: http://kanarip.fedorapeople.org/rubygem-passenger.spec
New SRPM: http://kanarip.fedorapeople.org/rubygem-passenger-3.0.12-1.fc16.src.rpm

Jeroen -

Thanks for posting your specs. I'll look them over and incorporate them into my final version on Monday. I've got a few things I want to do a bit differently.

Now including nginx-passenger as well:

New SPEC: http://kanarip.fedorapeople.org/rubygem-passenger.spec
New SRPM: http://kanarip.fedorapeople.org/rubygem-passenger-3.0.12-1.fc16.src.rpm

Guys,

could you please update the spec according to the new Ruby packaging guidelines, i.e. use rubygems-devel, there is no need for declaration of 

gemdir
geminstdir
ruby_sitearch
ruby_sitelib

and more over the ruby_vendorarch and ruby_vendorlib should be used instead of their "site" equivalents.

Thank you.

I submitted some bug fixes upstream which hadn't been submitted before: https://svn.boost.org/trac/boost/ticket/6796

Here's the spec I've been working on, with several portions of Jeroen's spec incorporated. This should comply with the current Ruby packaging guidelines. 

SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.12-1.src.rpm
SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec

There's a few things in the built package that rpmlint isn't too happy about, including a few false warnings about misspellings. None of the warnings rpmlint throws appear to be critical.

I'm currently not including the nginx bits, as there doesn't appear to be any way to ship a module for nginx in the same way we ship apache modules, and shipping a statically compiled version of nginx in the passenger package isn't really acceptable.

I have few notes again:

* rubygem-fastthread was deprecated and removed from F17. The functionality was integrated into Ruby

* I'd like to see removed the "Requires: rubygem(rake)". It is typically not required for runtime. However, I am not passenger specialist, so it might be needed at the end.

Updated my spec and SRPM to fix the Requires. URLs are unchanged.

(In reply to comment #120)
> Updated my spec and SRPM to fix the Requires. URLs are unchanged.

Hi Bret, the condition for fastthread should be "%if 0%{?rhel} <= 6 && 0%{?fedora} <= 16". Thank you.

Updated my spec and SRPM to fix the fastthread logic. URLs are unchanged.

- First of all, please use gem as source and use current
  ruby gems based packaging guidelines revised recently.
- Please don't use Epoch.
- Please use %{?dist}
- Please remove unneeded stuff, unless you intend to ship this into
  EPEL below.
- By the way, is this issue cleared?
(In reply to comment #67)
> (In reply to comment #65)
> > Please note that Phusion Passenger is a registered
> > trademark in the European Union and a pending trademark in the United States.
> > If you want to ship a patched version of Phusion Passenger with changes that we
> > do not approve of, we have to ask you to rename it something else.
> 
> ???
> Then would you clarify the license?
> If Phusion Passenger upstream has such policy, I guess we will anyway
> has to rename such software on Fedora
> - Note that Debian does not accept "firefox" or so due to the same reason.
>   And there are so many discussion also on Fedora about whether we should
>   rename "firefox" to something else.
> 
> And if we decide the license is "not free", we won't accept such software
> anyway.
  This affects not only bundled boost issue but other things.

(In reply to comment #122)
> Updated my spec and SRPM to fix the fastthread logic. URLs are unchanged.

Please change release number every time you modify your spec file
to avoid confusion, even during review request process.

(In reply to comment #123)
> - Please remove unneeded stuff, unless you intend to ship this into
>   EPEL below.

This is "EPEL 5 and below".

On the trademark issue stuff... 

The issue, as I read it, was *if* we were attempting to distribute a package without the bundled boost, that would have been unacceptable because it was deemed an inferior and/or substantially different product. We're not attempting that, so I don't believe there's an issue with using the "Phusion Passenger" name to describe the contents of the package.

On the request to use the gem source...

That's not possible. There hasn't been a gem posted to rubyforge since version 2.2.11. The current 3.0.x series is only published as a tar.gz. See: http://rubyforge.org/frs/?group_id=5873&release_id=46694

My plan is to target EPEL 5 and 6, and F16+, so the conditional statements are valid. The only potentially unnecessary stuff is the commented-out nginx bits, which I talked about in Comment #118.

Here's a SPEC and SRPM with the epoch and dist issues corrected:

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.12-2.fc16.src.rpm

Brett's interpretation is correct.

As for the gem, we 'gem push' the gem to rubygems.org these days. It's there, it's just not on RubyForge.

And in any case, the gem and tar.gz content are the exact same. There is no advantage in using the gem as the source.

Brett, reading over the old comments, it appears that the trademark issue is still a problem.  Hongli Lai says that Phusion Passenger is trademarked and that patches need to be approved: https://bugzilla.redhat.com/show_bug.cgi?id=470696#c65

spot asks for the trademark license so we can evaluate how to handle it: https://bugzilla.redhat.com/show_bug.cgi?id=470696#c69

No reply pointing to the responsibilities we would have in order to use the trademark was subsequently given.  Note that once we know what the terms for using the trademark are, whether to use the trademark might have to pass through FESCo or another body.  The mozilla trademarks sound similar to what Hongli is describing and those were discussed in FESCo (I believe twice -- once in regards to closing the package to provenpackager and once wrt getting rid of bundled libraries).

There was a message about trademarks regarding another package recently so I've opened a ticket for FPC https://fedorahosted.org/fpc/ticket/170 to look at Trademark Guidelines.  There's a good possibility that the decision on policy will have to be made by someone else (Fedora Legal or FESCo) but this should get the ball rolling.

A written trademark license agreement would likely be of help in seeing where upstream wants things to end up and whether that can be accommodated or not.

Thanks Toshio!  I've also got an e-mail out to Hongli to see if he'd like to put together any specific terms. 

I think I understand why the naming issue was brought up in this case, but I also think it'd be better for everyone if we had some clear guidelines for what situations Phusion will allow the use of their trademarks, if any, to label the package in Fedora.

I'm committed to ensuring that Fedora's package doesn't deviate significantly from what Phusion is shipping, but I'd like some clear guidance on whether the patching of a package is acceptable and, if it is, how extensively a package can be patched before it needs to be called something else. I'm also curious if the normal work flow of "ship a patched package while pushing the patch upstream" work flow is acceptable in this case.

We're still waiting to hear back from Hongli Lai (or someone else at Phusion) as to what the trademark usage terms are for the Phusion/Passenger marks. As far as I can tell, this is the only blocker holding up this package.

Passenger doesn't work without the -devel package installed as 

passenger_native_support.so

(which seems to be required to run) is shipped in the -devel rpm.

Sven - Yep. That's been fixed in my local spec file.

Unfortunately, it's been more than a month and my inquiries to the Phusion folks about obtaining a license for Fedora to use their trademarks for purposes of including Passenger in Fedora have gone unanswered.

Until that issue is resolved, Fedora can't ship Passenger in its current form. 

In the near future, I'll be investigating if we have other options here.

To any knowledable devs:

My build keeps failing at:

+ rm -rf selinux
+ mkdir selinux
+ cd selinux
+ cp /root/rpmbuild/SOURCES/rubygem-passenger.fc .
cp: cannot stat `/root/rpmbuild/SOURCES/rubygem-passenger.fc': No such file or directory

when I run:

rpmbuild -bb rubygem-passenger.spec

what am I doing wrong?

Any hints would be greatly appreciated.

Thanks,
gb

P.S.:

yum-builddep rubygem-passenger-3.0.12-2.fc16.src.rpm

faild with

Getting requirements for rubygem-passenger-3.0.12-2.fc16.src
 --> Already installed : libcurl-devel-7.24.0-4.fc17.x86_64
Error: No Package found for rubygem(fastthread) >= 1.0.1

so I just installed the deps that were listed by rpmbuild the first time I ran it against the tweaked spec file that was provided in comment #126 of this thread.  I cannot seem to generate a new .src file with "rpmbuild -bs rubygem-passenger.spec" as that too returns:

error: Bad file: /root/rpmbuild/SOURCES/rubygem-passenger.te: No such file or directory
error: Bad file: /root/rpmbuild/SOURCES/rubygem-passenger.if: No such file or directory
error: Bad file: /root/rpmbuild/SOURCES/rubygem-passenger.fc: No such file or directory


RPM build errors:
    Bad file: /root/rpmbuild/SOURCES/rubygem-passenger.te: No such file or directory
    Bad file: /root/rpmbuild/SOURCES/rubygem-passenger.if: No such file or directory
    Bad file: /root/rpmbuild/SOURCES/rubygem-passenger.fc: No such file or directory

What am I missing?  I've tried "rpmbuild ... --without selinux" and I've tried enabling and disabling selinux and installing selinux-policy-devel.

Gregory - 

Right now, my fedorapeople space has a mismatch between the srpm and spec file that's up there. 

I'll see about fixing that so that the srpms I've published will build successfully.

That said, this particular bug is not the appropriate place to receive support for building your own packages. Please try on the relevant mailing lists or IRC channels.

I am very tired of this game. Unless Hongli Lai (or someone else at Phusion) confirms within the next two weeks (August 10, 2012) that there are specific (and public) trademark usage terms that all users/developers/distributors must obey for the Phusion/Passenger marks, we will move forward on the assumptions that there are no special usage rules, lift the FE-Legal block here, and treat this package identically to the rest of Fedora (which is to say, in compliance with US Trademark Law).

Lifting FE-Legal.

Now that the FE-Legal block is lifted, here's my current spec and srpm for review.

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-1.fc17.src.rpm


Here's what rpmlint tells me. All of its complaints are wrong in passenger's case.


$ rpmlint /home/blentz/rpmbuild/RPMS/x86_64/rubygem-passenger-3.0.14-1.fc17.x86_64.rpm 
rubygem-passenger.x86_64: W: spelling-error %description -l en_US Phusion -> Fusion
rubygem-passenger.x86_64: E: no-binary
rubygem-passenger.x86_64: W: devel-file-in-non-devel-package /usr/bin/passenger-config
rubygem-passenger.x86_64: W: no-manual-page-for-binary passenger
rubygem-passenger.x86_64: W: no-manual-page-for-binary passenger-install-apache2-module
rubygem-passenger.x86_64: W: no-manual-page-for-binary passenger-install-nginx-module
1 packages and 0 specfiles checked; 1 errors, 5 warnings.

$ rpmlint /home/blentz/rpmbuild/RPMS/x86_64/mod_passenger-3.0.14-1.fc17.x86_64.rpm 
mod_passenger.x86_64: W: spelling-error %description -l en_US Phusion -> Fusion
mod_passenger.x86_64: W: spelling-error %description -l en_US pluggable -> plug gable, plug-gable, plugged
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

$ rpmlint /home/blentz/rpmbuild/SRPMS/rubygem-passenger-3.0.14-1.fc17.src.rpmrubygem-passenger.src: W: spelling-error %description -l en_US Phusion -> Fusion
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Thanks Brett! Here's an error I'm getting while mocking on F17:

 make: /usr/share/selinux/devel/Makefile: No such file or directory

When I manully install selinux-policy-devel into the mock chroot, it works. I'm not sure why the dist tag conditional isn't matching Fedora 17 there.

I've done some light testing with Gitorious on F17, and your mod_passenger package is working well.

Ken - That's really odd. Do you have fedora-release installed in your chroot?

The only way I can see that failing is if /etc/rpm/macros.dist doesn't exist in your chroot.

Yeah, same error here: http://koji.fedoraproject.org/koji/taskinfo?taskID=4386689

I then tried some different invocations of that conditional and I still couldn't get it. (On the other hand, you can drop the Fedora 15 stuff since that's EOL now :)

Fixed! Thanks Ken!

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-2.fc17.src.rpm

Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=4386836

(In reply to comment #143)
> SPEC:
> http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
> SRPM:
> http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-
> 2.fc17.src.rpm

Build fails at least on F-18:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4388592

Also it seems that Fedora specific compilation flags are not correctly passed.
build log says:
/usr/include/features.h:330:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp]

Some questions first:

* doc/images/icons/README says the image files under doc/images/icons/ was taken from http://jimmac.musichall.cz/ikony.php3 but currently I could not find any license notice on this site. Would you clarify the license of these icon files?

* Not limited to boost, there are some other softwares bundled with this software under ext. Especially libev and nginx seems available on Fedora. Would you explain why these files (under ext/ directory) cannot be removed? Also see:

https://fedoraproject.org/wiki/Packaging/Guidelines#Bundling_of_multiple_projects
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

BTW current license analysis:

Overall: MIT

GPLv2
Rakefile
build/rpm.rb

MIT or GPL+
doc/Architectural\ overview.html
doc/Security\ of\ user\ switching\ support.html
doc/Users\ guide\ Apache.html
doc/Users\ guide\ Nginx.html
doc/Users\ guide\ Standalone.html

CC-BY-SA
doc/users_guide_snippets/appendix_a_about.txt

BSD
ext/google/
ext/libev/
ext/nginx/

Boost 1.0
ext/boost/
ext/common/Utils/utf8.h
ext/common/Utils/utf8/

MIT
ext/boost/detail/limits.hpp

BSD with advertising
ext/common/BCrypt.cpp
ext/common/BCrypt.h
ext/common/Blowfish.c
ext/common/Blowfish.h

zlib
ext/common/Utils/Base64.cpp
ext/common/Utils/Base64.h
ext/common/Utils/MD5.cpp
ext/common/Utils/MD5.h
test/support/valgrind.h

??? (perhaps not license text)
lib/phusion_passenger/templates/standalone/config.erb

MIT-style?? -> MIT
test/stub/rails_apps/2.3/mycook/public/javascripts/dragdrop.js
test/stub/rails_apps/2.3/mycook/public/javascripts/effects.js
test/stub/rails_apps/2.3/mycook/public/javascripts/prototype.js

???
doc/images/icons/README:
mentions http://jimmac.musichall.cz/ikony.php3

LibEV is not used. In the %prep step, is this line:

# Don't use bundled libev
%{__rm} -rf ext/libev

The ext/nginx directory contains no bundled libraries. The software in that directory is the nginx passenger module and a couple of headers require for compiling the module, not the full nginx package.

Also of note, the nginx module is not currently compiled or shipped with this package due to the duplication and bundling issues you've observed. So, while present in the srpm, we don't do anything with it.

So... if it's necessary to remove the unused code from the tarball (and srpm), we can do that. Please let me know if that's what you're looking for, or if it's acceptable to carry this code in the srpm, but not shipped in the built rpms.

Some investigation on the icons reveals that this particular set is in a wide variety of places in the same form as Passenger, with the same level of attribution (and lack of explicit license). It appears that the source of this icon set originates in Asciidoc 6.0.0, which is licensed under GPLv2+. (See: https://code.google.com/p/asciidoc/source/browse/CHANGELOG.txt )

The text in lib/phusion_passenger/templates/standalone/config.erb is not license text, but a developer's comment about how the code is used.

I'm now working on getting the srpm to compile on f18. It does currently compile on f17. Please let me know if this is a blocker for the package review.

libev is vendored for ease of compilation. You can force linking to a system libev by setting USE_VENDORED_LIBEV=false. Phusion Passenger 3.0 requires libev 3.9. Phusion Passenger 4.0 requires libev 4.0.

libev is used by the logging agent.

doc/images/icons are indeed taken from Asciidoc.

Documentation is CC-BY-SA. See appendix A in the manual. The stuff in doc/users_guide_snippets are compiled into the final manual.

lib/phusion_passenger/templates/standalone/config.erb - it's just a config file. Is it even meaningful to license them? If so consider it the same license as Phusion Passenger.

ext/nginx does not contain Nginx. It contains the Phusion Passenger module for Nginx.

It's acceptable to remove unused code if you make sure nothing breaks.

bundled libraries do not need to be removed prior to srpm creation as long as there are no licensing or patent concerns.  rm -rf'ing the bundled code in %prep is sufficient.  http://fedoraproject.org/wiki/Packaging:Treatment_Of_Bundled_Libraries#Packages_with_Bundled_Libraries

Might want to ask spot what the license tag for this package should be.  icons are not code so I don't know how that affects things.

(In reply to comment #147)
> LibEV is not used. In the %prep step, is this line:
> 
> # Don't use bundled libev
> %{__rm} -rf ext/libev

Okay.

> 
> The ext/nginx directory contains no bundled libraries. The software in that
> directory is the nginx passenger module and a couple of headers require for
> compiling the module, not the full nginx package.

Okay.

> 
> Also of note, the nginx module is not currently compiled or shipped with
> this package due to the duplication and bundling issues you've observed. So,
> while present in the srpm, we don't do anything with it.
> 
> So... if it's necessary to remove the unused code from the tarball (and
> srpm), we can do that. Please let me know if that's what you're looking for,
> or if it's acceptable to carry this code in the srpm, but not shipped in the
> built rpms.

As Toshio said, removing these files are not needed.

> Some investigation on the icons reveals that this particular set is in a
> wide variety of places in the same form as Passenger, with the same level of
> attribution (and lack of explicit license). It appears that the source of
> this icon set originates in Asciidoc 6.0.0, which is licensed under GPLv2+.
> (See: https://code.google.com/p/asciidoc/source/browse/CHANGELOG.txt )

Okay.

> The text in lib/phusion_passenger/templates/standalone/config.erb is not
> license text, but a developer's comment about how the code is used.

Well, what I worried here was about the words "You are strongly discouraged from editing this file", however I don't think this is some license restriction (for now).

> I'm now working on getting the srpm to compile on f18. It does currently
> compile on f17. Please let me know if this is a blocker for the package
> review.

The package has to build successfully first on rawhide, next on F-18 (currently). So first please fix build issue on rawhide (currently rawhide equals F-18).

(In reply to comment #148)
> libev is vendored for ease of compilation. You can force linking to a system
> libev by setting USE_VENDORED_LIBEV=false. Phusion Passenger 3.0 requires
> libev 3.9. Phusion Passenger 4.0 requires libev 4.0.
> 
> libev is used by the logging agent.
> 
> doc/images/icons are indeed taken from Asciidoc.
> 
> Documentation is CC-BY-SA. See appendix A in the manual. The stuff in
> doc/users_guide_snippets are compiled into the final manual.

Thank you for explanation.

> lib/phusion_passenger/templates/standalone/config.erb - it's just a config
> file. Is it even meaningful to license them? If so consider it the same
> license as Phusion Passenger.

As explained above I was a bit worried about the words "You are strongly discouraged from editing this file", although now I don't think this as a blocker.

> 
> ext/nginx does not contain Nginx. It contains the Phusion Passenger module
> for Nginx.

Okay.

> 
> It's acceptable to remove unused code if you make sure nothing breaks.

> Well, what I worried here was about the words "You are strongly discouraged from editing this file", however I don't think this is some license restriction (for now).

That is not a license restriction. It's a warning to users, for usability reasons.

BTW about F-19 and F-18 build failure:

see this:
https://svn.boost.org/trac/boost/ticket/6940

> see this: https://svn.boost.org/trac/boost/ticket/6940

This helped a ton! Thanks!


I now have a successful F18/F19 build:

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-3.fc19.src.rpm
RPMS: http://wakko666.fedorapeople.org/rubygem-passenger/


Hongli - I've got a patch for passenger that fixes this issue until you're ready to update your vendored Boost to 1.50+. I'll be sending you a pull request on Github with the patch.

I can't build on F17:

ext/common/ApplicationPool/Pool.h:721:21: error: 'TIME_UTC' was not declared in this scope

Orion - Thanks for the report. I've fixed it by making the TIME_UTC patch conditional on only being needed for F18+.

New SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-4.fc19.src.rpm

Thanks, that compiles for me now.  Going to test it out now...

Some notes:
(the below notes are just from your spec file and the results
 of rpm -qlp or rpmlint (binary rpm), and I am not yet checked
 other issues. Also I have not installed the rebuilt rpms yet)

For 3.0.14-4.fc19:

* isa specific requirement
  - Please make dependency between arch-dependent subpackages
    isa specific:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

* Dependency between subpackages
  - Usually dependency between subpackages and main package is
    full EVR (epoch-version-release) specific (i.e. not just
    Requires: rubygem(%{gem_name}) = %{version}) (and also see
    the above "isa specific requirement"):
    https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

  - Would you explain why -native subpackage does not require
    -native-libs but the opposite? (i.e. -native-libs require
    -native subpackage currently). Usually -libs package is requireD
    by other packages.

* Unneeded Provides
  - Provides: rubygem-passenger-native-libs = %{version} is unneeded,
    as this is automatically provided (i.e. foo-%version-%release
    provides not only foo = %version-%release but also foo = %version,
    and of course also foo). Also see the message
    " E: useless-provides rubygem-passenger-native-libs" from rpmlint.

* Directory ownership issue
  https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership
  https://fedoraproject.org/wiki/Packaging:UnownedDirectories
  - Please make it sure that all directories created during the
    installation of these packages are correctly owned by some
    rpms.
    - At least please check which package should own the following
      directories (themselves):
      %{gem_instdir}/ext/
      %{gem_instdir}/agents/

* Redundant repeated summary
  - Why do you repeatedly write "Phusion Passenger™ is..." messages
    on each subpackages' Summary? When a subpackage requires main
    package, it is sufficient to write such summary only on main
    package.

* Binary files location
  - .so files must be under %_libdir and must not under %_datadir.
  - Also check the message "E: arch-dependent-file-in-usr-share"
    from rpmlint. rpmlint says some files under {gem_instdir}/agents/
    are also arch-dependent binaries:
    https://fedoraproject.org/wiki/Packaging/Guidelines#Filesystem_Layout
    http://www.pathname.com/fhs/pub/fhs-2.3.html#USRSHAREARCHITECTUREINDEPENDENTDATA
    https://fedoraproject.org/wiki/Packaging/Ruby#Macros

* Build-time generated unneeded files
  - Please remove unneeded files which are generated on build time
    - At least please remove .o files
    - I don't think Makefile is needed, either.
    - mkmf.log is perhaps also unneeded.

* Possibly unneeded files
  - Rakefile or INSTALL files are usually needed for people who tries
    to build a software by him/herself and not needed for people who
    installs a package by rpm.

* scriptlets
  - Well, still I am not familiar with selinux, but is the following
    scriptlet correct?
--------------------------------------------------------
   283  %preun native
   285  semodule -r rubygem_%{gem_name} 2>/dev/null || :
--------------------------------------------------------
    (note: not rubygem-%{gem_name} but rubygem_%{gem_name}:
     underscore is used)

* rpmlint issues
  - Additionally, at least please take care of the below rpmlint
    messages:
--------------------------------------------------------
rubygem-passenger-native.i686: W: log-files-without-logrotate /var/log/passenger-analytics
--------------------------------------------------------

I just did a test install of "mod_passenger", which brought in rubygem-passenger.  But this fails to initialize in apache due to missing PassengerWatchdog from rubygem-passenger-native, so there seems to be a missing dep there.

[Thu Aug 16 10:24:48 2012] [error] *** Passenger could not be initialized because of this error: Unable to start the Phusion Passenger watchdog because its executable (/usr/share/gems/gems/passenger-3.0.14/agents/PassengerWatchdog) does not exist. This probably means that your Phusion Passenger installation is broken or incomplete, or that your 'PassengerRoot' directive is set to the wrong value. Please reinstall Phusion Passenger or fix your 'PassengerRoot' directive, whichever is applicable.

FWIW - the stealthmonkey mod_passenger had a dep on rubygem-passenger-native-libs, which depended on rubygem-passenger-native, which depended on rubygem-passenger.  I haven't analyzed it myself to see what makes the most sense.

Without rubygem-passenger-native-libs installed I get:

*** Phusion Passenger: no passenger_native_support.so found for the current Ruby interpreter. Compiling one...
*** Phusion Passenger: no passenger_native_support.so found for the current Ruby interpreter. Compiling one...
# mkdir -p /usr/share/gems/gems/passenger-3.0.14/ext/ruby/native
# mkdir -p /usr/share/gems/gems/passenger-3.0.14/ext/ruby/native
Encountered permission error, trying a different directory...
-------------------------------
# mkdir -p /root/.passenger/native_support/3.0.14/native
Encountered permission error, trying a different directory...
-------------------------------
# mkdir -p /root/.passenger/native_support/3.0.14/native
*** Phusion Passenger: no passenger_native_support.so found for the current Ruby interpreter. Compiling one...
# mkdir -p /usr/share/gems/gems/passenger-3.0.14/ext/ruby/native
Encountered permission error, trying a different directory...
-------------------------------
# mkdir -p /root/.passenger/native_support/3.0.14/native
*** Phusion Passenger: no passenger_native_support.so found for the current Ruby interpreter. Compiling one...
/usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:133:in `initialize': Permission denied - /root/.passenger/native_support/3.0.14/native/.permission_test (Errno::EACCES)
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:133:in `open'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:133:in `block in compile'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:130:in `each'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:130:in `each_with_index'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:130:in `compile'
# mkdir -p /usr/share/gems/gems/passenger-3.0.14/ext/ruby/native        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:108:in `compile_and_load'

        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:36:in `start'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:163:in `<top (required)>'
        from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
        from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/utils.rb:37:in `<top (required)>'
        from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
        from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
        from /usr/share/gems/gems/passenger-3.0.14/helper-scripts/passenger-spawn-server:105:in `rescue in <main>'
        from /usr/share/gems/gems/passenger-3.0.14/helper-scripts/passenger-spawn-server:30:in `<main>'
Encountered permission error, trying a different directory...
-------------------------------
# mkdir -p /root/.passenger/native_support/3.0.14/native
/usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:133:in `initialize': Permission denied - /root/.passenger/native_support/3.0.14/native/.permission_test (Errno::EACCES)
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:133:in `open'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:133:in `block in compile'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:130:in `each'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:130:in `each_with_index'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:130:in `compile'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:108:in `compile_and_load'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:36:in `start'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/native_support.rb:163:in `<top (required)>'
        from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
        from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
        from /usr/share/gems/gems/passenger-3.0.14/lib/phusion_passenger/utils.rb:37:in `<top (required)>'
        from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
        from /usr/share/rubygems/rubygems/custom_require.rb:36:in `require'
        from /usr/share/gems/gems/passenger-3.0.14/helper-scripts/passenger-spawn-server:105:in `rescue in <main>'
        from /usr/share/gems/gems/passenger-3.0.14/helper-scripts/passenger-spawn-server:30:in `<main>'

Orion - Good catch. I've got an updated spec and srpm that I'll be posting in a few minutes.

* isa specific requirement
- Fixed. isa deps added.

* Dependency between subpackages
- Fixed. Added release to deps. We're not using epoch, so that was left out intentionally.

* Unneeded Provides
- Fixed. Requires removed.

* Directory ownership issue
- Fixed. Reworked %files section.

* Redundant repeated summary
- Fixed. Removed repeated summary from sub-packages.

* Binary files location
- Fixed. Moved passenger_native_support.so to %_libdir/%name

* Build-time generated unneeded files
- Fixed. No longer shipping *.o files, Makefile, or mkmf.log.

* Possibly unneeded files
- Fixed. Moved Rakefile, INSTALL, and PACKAGING files to devel sub-package. They're only needed by developers/packagers.

* scriptlets
- Fixed. Scriptlet was technically correct, but it was due to misnaming the selinux module. Module name and scriptlet have been corrected.

* rpmlint issues
- Fixed. Logrotate config now included in native package.

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-5.fc19.src.rpm

%package native
Requires: policycoreutils, initscripts

is not right.  I'm not sure initscripts is needed at all, and policycoreutils is needed for %post so it needs to be:

Requires(post): policycoreutils

Also, moving passenger_native_support.so to %{_libdir}/%{name} broke loading the library:

*** Phusion Passenger: no passenger_native_support.so found for the current Ruby interpreter. Compiling one...

I'm not sure where else ruby will look for libraries.  Doesn't like %{_libdir} either, so it's not using the standard loader.

FYI - I'm having SELinux issues running a puppet master server under passenger.  This is related (especially since this package ships a SELinux module), but somewhat tangential since it involves running another application.  I've filed bug 848939 against selinux-policy to work through those issues.

Looks like %{_libdir}/gems/exts may be the place.  I would check the ruby guidelines.

Orion - many thanks for the additional testing you're doing.

I've fixed the deps for %post.

I've also moved the native libs to %ruby_vendorarchdir, as the ruby packaging guidelines instruct. I've also included some sed magic to ensure that passenger is searching the correct path for its native libs. This appears to work in my local f17 vm.

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-6.fc19.src.rpm

From what I've got back from Miroslav in bug 848939, you don't want to be shipping the selinux module at all.  It just interferes with what is in the base policy.

I'll see what I can do about completing the review.

FWIW -  Looks like 3.0.15 has been released.

Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== C/C++ ====
[x]: MUST Package does not contain any libtool archives (.la)
[x]: MUST Package does not contain kernel modules.
[x]: MUST Package contains no static executables.
[x]: MUST Rpath absent or only used for internal libs.


==== Generic ====
[!]: EXTRA Rpmlint is run on all installed packages.
  - See below
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[!]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
     Note: Using prebuilt rpms
[!]: MUST %build honors applicable compiler flags or justifies otherwise.
 -  Looks like RPM_OPT_FLAGS no passed to boost?
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[!]: MUST Package contains no bundled libraries.]
 - Exception granted for boost
 - Need to evaluate ext/{apache2, boost, common, google, nginx, oxt, ruby}

[x]: MUST Changelog in prescribed format.
[x]: MUST Sources contain only permissible code or content.
[x]: MUST %config files are marked noreplace or the reason is justified.
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files -n mod_passenger section. This is
     OK if packaging for EPEL5. Otherwise not needed
[x]: MUST Macros in Summary, %description expandable at SRPM build time.
[.]: MUST Package contains desktop file if it is a GUI application.
[.]: MUST Development files must be in a -devel package
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Fully versioned dependency in subpackages, if present.
[x]: MUST Package complies to the Packaging Guidelines
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf is only needed if supporting EPEL5
[x]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
[!]: MUST License field in the package spec file matches the actual license.
     Note: Checking original sources for licenses Licenses found:
     "zlib/libpng", "BSD (2 clause)", "BSL (v1.0)", "BSD (3 clause)", "BSD (4
     clause)", "GPL (v2)", "MIT/X11 (BSD like)" For detailed output of
     licensecheck see file: /export/home/orion/redhat/rubygem-passenger-3.0.14
     /rubygem-passenger/licensecheck.txt

 - Need to explore further.

[x]: MUST License file installed when any subpackage combination is installed.
[x]: MUST Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: MUST Package is named using only allowed ascii characters.
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST No %config files under /usr.
[x]: MUST Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[.]: MUST If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Package is not relocatable.
[x]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[.]: MUST Package contains systemd file(s) if in need.
[x]: MUST File names are valid UTF-8.
[x]: SHOULD Reviewer should test that the package builds in mock.
[!]: SHOULD Buildroot is not present
     Note: Buildroot is not needed unless packager plans to package for EPEL5
[!]: SHOULD Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean is needed only if supporting EPEL5
[.]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: SHOULD Package functions as described.
[!]: SHOULD Latest version is packaged.
  3.0.15 is out, not a blocker
[x]: SHOULD Package does not include license text files separate from
     upstream.
[!]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
 - No comments on patches
[!]: SHOULD Scriptlets must be sane, if used.
 - Really need to drop - selinux policy not needed
[x]: SHOULD SourceX tarball generation or download is documented.
[.]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Source10 (apache-passenger.conf.in) Patch101
     (passenger_boost_xtime.patch) Patch100
     (passenger_apache_fix_autofoo.patch) Source0 (passenger-3.0.14.tar.gz)
     Source1 (passenger.logrotate)
  I don't agree with this, messes with cut/paste.
[x]: SHOULD SourceX is a working URL.
[.]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[!]: SHOULD %check is present and all tests pass.
  - tests don't pass.  Can this be fixed?
[x]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.


==== Language ====
[x]: MUST Gem package must not define a non-gem subpackage
[x]: MUST Gem package must exclude cached Gem.
[!]: MUST Platform dependent files must all go under %{gem_extdir}, platform
     independent under %{gem_dir}.
  Looks like native should use %{gem_extdir} ?
[x]: MUST Gem package is named rubygem-%{gem_name}
[x]: MUST Package contains BuildRequires: rubygems-devel.
[x]: MUST Gem package must define %{gem_name} macro.
[!]: MUST Pure Ruby package must be built as noarch
  Can everything but the native packages be noarch?
[!]: MUST Package contains Requires: ruby(abi).
[!]: SHOULD Specfile should utilize macros from rubygem-devel package.
     Note: The specfile doesn't use these macros: %exclude %{gem_cache},
     %{gem_libdir}
[!]: SHOULD Test suite should not be run by rake.
  ?  
[x]: SHOULD Test suite of the library should be run.

Issues:
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files -n mod_passenger section. This is
     OK if packaging for EPEL5. Otherwise not needed
See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
[!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf is only needed if supporting EPEL5
See: None
[!]: MUST Pure Ruby package must be built as noarch
See: https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Support
[!]: MUST Package contains Requires: ruby(abi).
See: http://fedoraproject.org/wiki/Packaging:Ruby#Ruby_ABI



rpmlint:
mod_passenger.i686: W: spelling-error %description -l en_US pluggable -> plug gable, plug-gable, plugged
 - bah

rubygem-passenger-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/passenger-3.0.14/ext/common/Utils/utf8/core.h
rubygem-passenger-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/passenger-3.0.14/ext/common/Utils/utf8/checked.h

rubygem-passenger-devel.i686: W: install-file-in-docs /usr/share/gems/gems/passenger-3.0.14/INSTALL
- Generally don't put these in unless they are useful for configuring.

rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/UnseekableSocket/sync%3d-i.ri %3d
- Lots of these.  Strange characters?

rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractRequestHandler/processed_requests-i.ri

rubygem-passenger-doc.i686: W: file-not-utf8 /usr/share/gems/doc/passenger-3.0.14/ri/cache.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractRequestHandler/soft_termination_linger_time-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractRequestHandler/iterations-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/InitializationError/child_exception-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppProcess/app_root-i.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/DebugLogging/log_file%3d-c.ri %3d
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AdminTools/MemoryStats/platform_provides_private_dirty_rss_information%3f-i.ri %3f
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractServer/next_cleaning_time-i.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AnalyticsLogger/Log/null%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/compiler_supports_no_tls_direct_seg_refs_option%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/compiler_supports_visibility_flag%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/local_socket_address%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/ConditionVariable/timed_wait%21-i.ri %21
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/ruby_supports_fork%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/supports_sfence_instruction%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/compiler_visibility_flag_generates_warnings%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/install_framework_extensions%21-c.ri %21
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractServerCollection/check_idle_servers%21-i.ri %21
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/_spawn_options-c.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/compiler_supports_wno_missing_field_initializers_flag%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Standalone/Main/run%21-c.ri %21
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/curl_supports_ssl%3f-c.ri %3f
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppProcess/pid-i.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/PseudoIO/respond_to%3f-i.ri %3f
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppProcess/owner_pipe-i.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/process_is_alive%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/IO/close_on_exec%21-i.ri %21
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Standalone/RuntimeInstaller/install%21-i.ri %21
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AdminTools/MemoryStats/should_show_private_dirty_rss%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractServerCollection/empty%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/DebugLogging/stderr_evaluator%3d-c.ri %3d
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/ClassicRails/ApplicationSpawner/app_root-i.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/UnseekableSocket/source_of_exception%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/FileSystemWatcher/FileInfo/changed%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/PseudoIO/done%21-i.ri %21
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/env_defined%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AdminTools/process_is_alive%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/in_rvm%3f-c.ri %3f
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppInitError/stderr-i.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Rails3Extensions/init%21-c.ri %21
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/ClassicRailsExtensions/AnalyticsLogging/install%21-c.ri %21
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/MessageChannel/closed%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/apr_config_needed_for_building_apache_modules%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/FileSystemWatcher/DirInfo/changed%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractServerCollection/has_key%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AdminTools/MemoryStats/root_privileges_required_for_private_dirty_rss%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/compiler_supports_wno_attributes_flag%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/GC/copy_on_write_friendly%3f-c.ri %3f
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppInitError/app_type-i.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/has_math_library%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/supports_lfence_instruction%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/DebugLogging/log_level%3d-c.ri %3d
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/MessageChannel/io-i.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/NativeSupportLoader/supported%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/passenger_tmpdir%3d-c.ri %3d
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AdminTools/ServerInstance/Process/has_metrics%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/ConsoleTextTemplate/%5b%5d%3d-i.ri %5b
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/ConsoleTextTemplate/%5b%5d%3d-i.ri %5d
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/ConsoleTextTemplate/%5b%5d%3d-i.ri %3d
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractServer/max_idle_time-i.ri
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AnalyticsLogger/Connection/connected%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/opens_files%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/PlatformInfo/has_alloca_h%3f-c.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/HostsFileParser/resolves_to_localhost%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AnalyticsLogger/Log/closed%3f-i.ri %3f
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Rails3Extensions/AnalyticsLogging/install%21-c.ri %21
rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Standalone/Main/run%21-i.ri %21
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractRequestHandler/connect_password-i.ri
rubygem-passenger.i686: W: spelling-error %description -l en_US Phusion -> Fusion
rubygem-passenger.i686: E: no-binary
rubygem-passenger.i686: W: devel-file-in-non-devel-package /usr/bin/passenger-config
 - What is this for?  Also:

# passenger-config
/usr/share/rubygems/rubygems/dependency.rb:247:in `to_specs': Could not find rake (>= 0.8.1) amongst [bigdecimal-1.1.0, file-tail-1.0.5, io-console-0.3, json-1.6.5, passenger-3.0.14, rack-1.4.0, rdoc-3.12, spruz-0.2.5] (Gem::LoadError)
        from /usr/share/rubygems/rubygems/specification.rb:777:in `block in activate_dependencies'
        from /usr/share/rubygems/rubygems/specification.rb:766:in `each'
        from /usr/share/rubygems/rubygems/specification.rb:766:in `activate_dependencies'
        from /usr/share/rubygems/rubygems/specification.rb:750:in `activate'
        from /usr/share/rubygems/rubygems.rb:212:in `rescue in try_activate'
        from /usr/share/rubygems/rubygems.rb:209:in `try_activate'
        from /usr/share/rubygems/rubygems/custom_require.rb:59:in `rescue in require'
        from /usr/share/rubygems/rubygems/custom_require.rb:35:in `require'
        from /bin/passenger-config:27:in `<main>'

rubygem-passenger.i686: W: no-manual-page-for-binary passenger
rubygem-passenger-native.i686: W: spelling-error Summary(en_US) Phusion -> Fusion 
 - Fine

rubygem-passenger-native.i686: E: arch-dependent-file-in-usr-share /usr/share/gems/gems/passenger-3.0.14/agents/PassengerWatchdog
rubygem-passenger-native.i686: E: arch-dependent-file-in-usr-share /usr/share/gems/gems/passenger-3.0.14/agents/PassengerLoggingAgent
rubygem-passenger-native.i686: E: arch-dependent-file-in-usr-share /usr/share/gems/gems/passenger-3.0.14/agents/apache2/PassengerHelperAgent
 - Hmm  Not sure about this, and need to coordinate with selinux-policy.

rubygem-passenger-native.i686: W: no-documentation
rubygem-passenger-native.i686: E: non-standard-executable-perm /usr/share/gems/gems/passenger-3.0.14/agents/PassengerWatchdog 0775L
rubygem-passenger-native.i686: E: non-standard-executable-perm /usr/share/gems/gems/passenger-3.0.14/agents/PassengerLoggingAgent 0775L
rubygem-passenger-native.i686: E: non-standard-executable-perm /usr/share/gems/gems/passenger-3.0.14/agents/apache2/PassengerHelperAgent 0775L
rubygem-passenger-native.i686: E: incoherent-logrotate-file /etc/logrotate.d/passenger
rubygem-passenger-native.i686: W: non-conffile-in-etc /etc/logrotate.d/passenger
rubygem-passenger-native-libs.i686: W: spelling-error Summary(en_US) Phusion -> Fusion
rubygem-passenger-native-libs.i686: W: no-documentation
rubygem-passenger-native-libs.i686: E: non-standard-executable-perm /usr/lib/ruby/vendor_ruby/passenger_native_support.so 0775L
7 packages and 0 specfiles checked; 9 errors, 96 warnings.

[!]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.

--- This has been resolved in the comments above.

[!]: MUST %build honors applicable compiler flags or justifies otherwise.
 -  Looks like RPM_OPT_FLAGS no passed to boost?

--- This is addressed in Patch100

[!]: MUST Package contains no bundled libraries.]
 - Exception granted for boost
 - Need to evaluate ext/{apache2, boost, common, google, nginx, oxt, ruby}

--- Addressed in Comment 147

[!]: MUST License field in the package spec file matches the actual license.
     Note: Checking original sources for licenses Licenses found:
     "zlib/libpng", "BSD (2 clause)", "BSL (v1.0)", "BSD (3 clause)", "BSD (4
     clause)", "GPL (v2)", "MIT/X11 (BSD like)" For detailed output of
     licensecheck see file: /export/home/orion/redhat/rubygem-passenger-3.0.14
     /rubygem-passenger/licensecheck.txt

 - Need to explore further.

--- I believe the automated check is wrong here. However, the list is primarily
all BSD and BSD-like licenses which, afaik, are compatible with one another.
The License tag is marked as 'BSD' which, I believe, is accurate.

Also, previous comments have dealt with this issue.

[!]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
 - No comments on patches

--- This is a SHOULD, not a MUST. I'd like to postpone fixing this until post-review, if you're agreeable to that.

[!]: SHOULD Scriptlets must be sane, if used.
 - Really need to drop - selinux policy not needed

--- Fixed in rev. 7, srpm and spec posted below.

[!]: SHOULD %check is present and all tests pass.
  - tests don't pass.  Can this be fixed?

--- As noted in the comments in the specfile, these test failures are expected
and need to be fixed upstream.

[!]: MUST Platform dependent files must all go under %{gem_extdir}, platform
     independent under %{gem_dir}.
  Looks like native should use %{gem_extdir} ?

--- I believe vendorarchdir is being used correctly in this case.

See: http://fedoraproject.org/wiki/Packaging:Ruby#Build_Architecture_and_File_Placement

For packages with binary content, e.g., database drivers or any other Ruby bindings to C libraries, the package MUST be architecture specific.

The binary files in a Ruby package with binary content MUST be placed into %{ruby_vendorarchdir} (or its proper subdirectory). The Ruby files in such a package SHOULD be placed into %{ruby_vendorlibdir}. The specfile MUST use these macros.

[!]: MUST Each %files section contains %defattr if rpm < 4.4

--- defattrs removed in rev 7. srpm and spec posted below.

[!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.

--- removed in rev 7. srpm and spec posted below.

[!]: MUST Pure Ruby package must be built as noarch

--- Invalid. This is not a pure ruby package.

[!]: MUST Package contains Requires: ruby(abi).

--- added in rev 7. srpm and spec posted below.


rubygem-passenger-devel.i686: W: install-file-in-docs /usr/share/gems/gems/passenger-3.0.14/INSTALL
- Generally don't put these in unless they are useful for configuring.

--- It's in the -devel package because it has some useful info for developers.

rubygem-passenger-doc.i686: W: unexpanded-macro /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/Utils/UnseekableSocket/sync%3d-i.ri %3d
- Lots of these.  Strange characters?

--- Rpmlint has a hard time with the RI doc template format. These can be ignored.

rubygem-passenger.i686: W: devel-file-in-non-devel-package /usr/bin/passenger-config
 - What is this for?

--- Rpmlint is wrong. passenger-config isn't a devel file. It's a
configuration helper script. I've added a requires for Rake in rev 7.

For an example use see: http://www.modrails.com/documentation/Users%20guide%20Apache.html#_the_apache_error_log_says_that_the_spawn_manager_script_does_not_exist_or_that_it_does_not_have_permission_to_execute_it


rubygem-passenger-native.i686: E: non-standard-executable-perm /usr/share/gems/gems/passenger-3.0.14/agents/apache2/PassengerHelperAgent 0775L

--- fixed in rev 7. srpm and spec posted below.

rubygem-passenger-native.i686: E: incoherent-logrotate-file /etc/logrotate.d/passenger
rubygem-passenger-native.i686: W: non-conffile-in-etc /etc/logrotate.d/passenger

--- Rpmlint is wrong. The logrotate file is correct and logrotate files don't end in conf.





SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-7.fc19.src.rpm

(In reply to comment #169)
> [!]: MUST Package is licensed with an open-source compatible license and
> meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.
> 
> --- This has been resolved in the comments above.

I'll take a closer look, but I'm coming in late to a very long and complicated review of a package that has had license changes and many version changes.  It's going to take me a while and I don't have a lot of time.  Toshio - if you have anything to weigh in here, that would be helpful.

> [!]: MUST %build honors applicable compiler flags or justifies otherwise.
>  -  Looks like RPM_OPT_FLAGS no passed to boost?
> 
> --- This is addressed in Patch100

Okay, looks like it isn't boost.  But:

grep -E 'g(cc|\+\+) ' build.log | grep -vF FORTIFY_SOURCE

finds a few lines like:

g++ -I../../ext -I../support -DTESTING_APPLICATION_POOL -D_REENTRANT -I/usr/local/include -DHASH_NAMESPACE="__gnu_cxx" -DHASH_NAMESPACE="__gnu_cxx" -DHASH_FUN_H="<hash_fun.h>" -DHAS_ALLOCA_H -DHAS_SFENCE -DHAS_LFENCE -Wall -Wextra -Wno-unused-parameter -Wno-parentheses -Wpointer-arith -Wwrite-strings -Wno-long-long -Wno-missing-field-initializers -g -DPASSENGER_DEBUG -DBOOST_DISABLE_ASSERTS -c oxt_test_main.cpp

But a closer looks shows that these all appear to be test code, so that should be fine.

> [!]: MUST Package contains no bundled libraries.]
>  - Exception granted for boost
>  - Need to evaluate ext/{apache2, boost, common, google, nginx, oxt, ruby}
> 
> --- Addressed in Comment 147

Addressed nginx and libev.  I suspect the others are similar, but haven't had time to take a closer look.

> 
> [!]: MUST License field in the package spec file matches the actual license.
>      Note: Checking original sources for licenses Licenses found:
>      "zlib/libpng", "BSD (2 clause)", "BSL (v1.0)", "BSD (3 clause)", "BSD (4
>      clause)", "GPL (v2)", "MIT/X11 (BSD like)" For detailed output of
>      licensecheck see file:
> /export/home/orion/redhat/rubygem-passenger-3.0.14
>      /rubygem-passenger/licensecheck.txt
> 
>  - Need to explore further.
> 
> --- I believe the automated check is wrong here. However, the list is
> primarily
> all BSD and BSD-like licenses which, afaik, are compatible with one another.
> The License tag is marked as 'BSD' which, I believe, is accurate.
> 
> Also, previous comments have dealt with this issue.
> 
> [!]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
>      justified.
>  - No comments on patches
> 
> --- This is a SHOULD, not a MUST. I'd like to postpone fixing this until
> post-review, if you're agreeable to that.

Sure.

> [!]: SHOULD Scriptlets must be sane, if used.
>  - Really need to drop - selinux policy not needed
> 
> --- Fixed in rev. 7, srpm and spec posted below.

Great.  Need to also remove:

Requires(post): policycoreutils

> [!]: SHOULD %check is present and all tests pass.
>   - tests don't pass.  Can this be fixed?
> 
> --- As noted in the comments in the specfile, these test failures are
> expected
> and need to be fixed upstream.
> 
> [!]: MUST Platform dependent files must all go under %{gem_extdir}, platform
>      independent under %{gem_dir}.
>   Looks like native should use %{gem_extdir} ?
> 
> --- I believe vendorarchdir is being used correctly in this case.
> 
> See:
> http://fedoraproject.org/wiki/Packaging:
> Ruby#Build_Architecture_and_File_Placement
> 
> For packages with binary content, e.g., database drivers or any other Ruby
> bindings to C libraries, the package MUST be architecture specific.
> 
> The binary files in a Ruby package with binary content MUST be placed into
> %{ruby_vendorarchdir} (or its proper subdirectory). The Ruby files in such a
> package SHOULD be placed into %{ruby_vendorlibdir}. The specfile MUST use
> these macros.

But this is a gem and that section of the guidelines says to use %{gem_extdir}

> [!]: MUST Pure Ruby package must be built as noarch
> 
> --- Invalid. This is not a pure ruby package.

Yeah, looks like this is all set.

> rubygem-passenger-native.i686: E: incoherent-logrotate-file
> /etc/logrotate.d/passenger
> rubygem-passenger-native.i686: W: non-conffile-in-etc
> /etc/logrotate.d/passenger
> 
> --- Rpmlint is wrong. The logrotate file is correct and logrotate files
> don't end in conf.

It's complaining that it isn't named "/etc/logrotate.d/rubygem-passenger", which is fine.  It also is complaining that it isn't marked %conf in the %files section.  I'm fine either way.

I'm still concerned about the arch binaries in /usr/share.  More tomorrow.

I've also asked spot to weigh in on the licensing.

> Addressed nginx and libev.  I suspect the others are similar, but haven't had 
> time to take a closer look.

Ok. I'll let you confirm.

> Great.  Need to also remove:
> Requires(post): policycoreutils

Fixed in rev 8, below.

> But this is a gem and that section of the guidelines says to use %{gem_extdir}

Sure. I'm reading it differently, but I don't see much point in wasting a bunch of time arguing. 

Fixed in rev 8, below.

> It's complaining that it isn't named "/etc/logrotate.d/rubygem-passenger", 
> which is fine.  It also is complaining that it isn't marked %conf in the 
> %files section.  I'm fine either way.

Ok. Let's skip it.



SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-8.fc19.src.rpm

License: BSD is the correct license here.

(In reply to comment #170)
> (In reply to comment #169)
> > [!]: MUST Package is licensed with an open-source compatible license and
> > meets
> >      other legal requirements as defined in the legal section of Packaging
> >      Guidelines.
> > 
> > --- This has been resolved in the comments above.
> 
> I'll take a closer look, but I'm coming in late to a very long and
> complicated review of a package that has had license changes and many
> version changes.  It's going to take me a while and I don't have a lot of
> time.  Toshio - if you have anything to weigh in here, that would be helpful.
> 
The two pieces that I looked at in this package were the bundling of boost and the assertion that Passenger was trademarked and the owner wanted to restrict what we could do with the code and still use the trademark.

* The bundling of boost received an exception with from FPC provided that wakk0666 was maintainer (due to his being able to do work on the bundled boost code) https://fedorahosted.org/fpc/ticket/160

* After waiting for the appearance of a trademark license with no response, spot finally declared that we would treat this package as not having trademark usage restrictions until and unless someone from phusion notified us differently. https://bugzilla.redhat.com/show_bug.cgi?id=470696#c137

So both of those are resolved for now.

(In reply to comment #171)

> > But this is a gem and that section of the guidelines says to use %{gem_extdir}

> Sure. I'm reading it differently[...]

If you can think of a way to clarify that.... Orion's interpretation is correct.  Gems have their own location for architecture specific files.  The Ruby#Build_Architecture_and_File_Placement section is only for non-gem ruby packages.

Brett,

* Drop the "Relocate the bin files to the proper location" section
  - The files are already in correct place and should stay there. You should add
    appropriate "%{gem_instdir}/bin" into the %files section.

* Drop the "fix up paths for locating native support libs."
  - Instead, you should place the libraries into correct directories, i.e. do
    
    %{__cp} -a ext/ruby/ruby*linux/passenger_native_support.so \
      %{buildroot}%{gem_extdir}/lib/

    In this case, the library will be correctly picked up by the
    PhusionPassenger::NativeSupportLoader#load_from_load_path instead of
    PhusionPassenger::NativeSupportLoader#load_from_source_dir

* You should own %{gem_extdir}
  - You own just the file, which is wrong.

* Test suite
  - It fails fairly early.
  - You even did not get to the point where the RSpec test suite is executed.
  - I have tried to execute the test suite adding the following lines:

    # This will make the test failure non-critical, but it should be examined
    # anyway.
    sed -i 's|sh "cd test && \./cxx/CxxTestMain"|& rescue true|' \
      build/cxx_tests.rb
    # Fedora has RSpec 2 while the test suite seems to require RSpec 1.
    sed -i \
      "s|return locate_ruby_tool('spec')|return locate_ruby_tool('rspec')|" \
      lib/phusion_passenger/platform_info/ruby.rb

    Hopefully, you can continue from here.
  - Some tests seems to be unstable, e.g. the LoggingTest. It sometimes passes
    and the other time fails.

Vit - Thanks! I've fixed all of those issues.

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-9.fc19.src.rpm

Brett,

When I said "hopefully you can continue from here", I expected that you would take a look why the test suite does not work and what needs to be fixed etc. Unfortunately, you even did not add the Require: rubygem(rspec), what would be the first step. Could you please explore the test failures?

(In reply to comment #175)
> SPEC:
> http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
> SRPM:
> http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-
> 9.fc19.src.rpm

I get Forbidden and Not Found respectively for these.  Found -10 srpm and using that.

# /usr/share/gems/gems/passenger-3.0.14/bin/passenger-config
/usr/share/rubygems/rubygems/dependency.rb:247:in `to_specs': Could not find fastthread (>= 1.0.1) amongst [bigdecimal-1.1.0, file-tail-1.0.5, io-console-0.3, json-1.6.5, passenger-3.0.14, rack-1.4.0, rake-0.9.2.2, rdoc-3.12, spruz-0.2.5] (Gem::LoadError)
        from /usr/share/rubygems/rubygems/specification.rb:777:in `block in activate_dependencies'
        from /usr/share/rubygems/rubygems/specification.rb:766:in `each'
        from /usr/share/rubygems/rubygems/specification.rb:766:in `activate_dependencies'
        from /usr/share/rubygems/rubygems/specification.rb:750:in `activate'
        from /usr/share/rubygems/rubygems.rb:1232:in `gem'
        from /usr/share/gems/gems/passenger-3.0.14/bin/passenger-config:22:in `<main>'

Looks like there are still some missing deps here.

Also, it looks like the agents perhaps aren't getting installed because of:

-               AGENTS_DIR         = NATIVELY_PACKAGED_AGENTS_DIR

in rubygem-passenger-3.0.12-force-native.patch.

Also, it changes:

-                               agentsDir           = "/usr/lib/phusion-passenger/agents";

+                               agentsDir           = "%%GEM_INSTALL_DIR%%/agents";

Which isn't correct.  Probably should be %{_libdir}/phusion-passenger/agents.

Orion - Looks like you caught me mid-update. I had uploaded a new version (and cleaned up the old files), then noticed some other issues. So, I was holding off on updating the bug to point at the new srpm and spec until those issues were corrected.

I'll fold your comments into the work I'm doing and update the bug when revision 10 is ready.

(In reply to comment #177)
> # /usr/share/gems/gems/passenger-3.0.14/bin/passenger-config
> /usr/share/rubygems/rubygems/dependency.rb:247:in `to_specs': Could not find
> fastthread (>= 1.0.1) amongst [bigdecimal-1.1.0, file-tail-1.0.5,
> io-console-0.3, json-1.6.5, passenger-3.0.14, rack-1.4.0, rake-0.9.2.2,
> rdoc-3.12, spruz-0.2.5] (Gem::LoadError)
>         from /usr/share/rubygems/rubygems/specification.rb:777:in `block in
> activate_dependencies'
>         from /usr/share/rubygems/rubygems/specification.rb:766:in `each'
>         from /usr/share/rubygems/rubygems/specification.rb:766:in
> `activate_dependencies'
>         from /usr/share/rubygems/rubygems/specification.rb:750:in `activate'
>         from /usr/share/rubygems/rubygems.rb:1232:in `gem'
>         from
> /usr/share/gems/gems/passenger-3.0.14/bin/passenger-config:22:in `<main>'
> 
> Looks like there are still some missing deps here.

fastthread was deprecated for F17+, since it is not needed for Ruby 1.9.3 any more. This dependency should be removed IMO.

It is safe to require fastthread. In 2009 I submitted a patch to fastthread to make it a no-op no newer Ruby versions. This way we can support older Ruby versions as well as newer Ruby versions, from a single unmodified source.

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-10.fc19.src.rpm

Okay, revision 10 is ready for a look. 

I've added the dependency for ruby(fastthread) for the time being. Once the review is done, I'll revisit this and see if I can come up with a more suitable solution. I agree that we don't need both the package manager *and* the application doing (potentially conflicting) dependency checking and enforcement.

I've moved the agents into %gem_extdir/agents, because they're arch-specific binaries.

I've added a few patches to get further along in the test process, but due to differences between rspec 1 and rspec 2, the tests still don't complete and will require significant effort getting much further. However, the packaging guidelines state that tests merely SHOULD be run and SHOULD run successfully, so this isn't something that needs to block approval of the package.

I will ask that, for the sake of meeting the F18 release schedule, that all reviewers focus on items that are MUST requirements. Anything that's a SHOULD, I am happy to address after the package review is complete.

3.0.17 has just been released: http://blog.phusion.nl/2012/08/28/phusion-passenger-3-0-17-released/

(In reply to comment #181)
> 
> I will ask that, for the sake of meeting the F18 release schedule, that all
> reviewers focus on items that are MUST requirements. Anything that's a
> SHOULD, I am happy to address after the package review is complete.

Just a note about interpretation of guidelines... SHOULD guidelines are basically left to the reviewer's discretion.  The reviewer can decide that failing that guideline is something that will prevent them from approving the package in good conscience.  The reviewer is also free to be more lenient and pass the package even though it fails that step.

For unittests, my personal feeling when I review is that unittesting is an aid to this Should guidelines: "SHOULD: The reviewer should test that the package functions as described. A package should not segfault instead of running, for example.".  If I'm familiar with running the software being reviewed and am reasonably confident that it can function successfully in a reasonably fault-tolerant fashion despite unittest failures, I work on disabling just the failing tests.  If I'm unfamiliar with the software... unittests may be the only indication of how good a reflection on Fedora's integration of the package the software is so it weighs more heavily on whether the package is ready for approval.

A different note about the release schedule -- it's getting late in the F18 cycle to still be reviewing packages for a F18 Feature.  If there are features depending on this package, it might be time to consider slipping until F19 for that feature.  However, the rubygem-passenger package itself could enter Fedora 18 even if F18 has been released already (via updates)... it would only be the dependent features that would need to slip to the next release.

Toshio - We're in agreement. :)

Passenger is critical to the Openshift Origin feature. 

I would like to avoid being forced to slip a whole feature simply because Passenger's unit tests fail due to not having been ported to rspec2 yet, despite the software itself being fully functional.

My comment was exactly as stated; asking for a particular interpretation of the guidelines to help speed the review process along. I believe that we're close enough that there are few (if any) egregious problems with the package.

If anyone reviewing the package disagrees, that's fine. I understand the importance of ensuring that packages meet a certain standard before being accepted. I'm committed to getting this review finished, regardless of which release it lands.

I don't understand the fastthread situation, especially on F17 which is where I'm testing at the moment.  There is nothing that provides "rubygem(fastthread)" on F17, but passenger-config still fails looking for it with ruby-1.9.3.194-15.fc17 (comment 177).  So what is the proper fix here?  At the very least is seems we need:

%if 0%{?rhel} <= 6 && 0%{?fedora} <= 16
Requires: rubygem(fastthread) >= 1.0.1
%endif

Maybe fixed in 3.0.17 :)

rpmlint issues that still should be dealt with:

rubygem-passenger-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/passenger-3.0.14/ext/common/Utils/utf8/core.h
rubygem-passenger-debuginfo.i686: W: spurious-executable-perm /usr/src/debug/passenger-3.0.14/ext/common/Utils/utf8/checked.h

rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractRequestHandler/processed_requests-i.ri
rubygem-passenger-doc.i686: W: file-not-utf8 /usr/share/gems/doc/passenger-3.0.14/ri/cache.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractRequestHandler/soft_termination_linger_time-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractRequestHandler/iterations-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/InitializationError/child_exception-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppProcess/app_root-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractServer/next_cleaning_time-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/_spawn_options-c.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppProcess/pid-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppProcess/owner_pipe-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/ClassicRails/ApplicationSpawner/app_root-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppInitError/stderr-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AppInitError/app_type-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/MessageChannel/io-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractServer/max_idle_time-i.ri
rubygem-passenger-doc.i686: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/passenger-3.0.14/ri/PhusionPassenger/AbstractRequestHandler/connect_password-i.ri

rubygem-passenger-native.i686: E: non-standard-executable-perm /usr/lib/gems/exts/passenger-3.0.14/agents/agents/apache2/PassengerHelperAgent 0775L
rubygem-passenger-native-libs.i686: E: non-standard-executable-perm /usr/lib/gems/exts/passenger-3.0.14/lib/passenger_native_support.so 0775L
rubygem-passenger-native-libs.i686: E: non-standard-executable-perm /usr/lib/gems/exts/passenger-3.0.14/agents/agents/apache2/PassengerHelperAgent 0775L


Also, I think you should make the -doc sub-package noarch.

The new ruby packaging draft suggests:

%exclude %{gem_cache}

I don't know the status of the draft though.  Can anyone else help here?


I'm fine with the licensing now.  

Just need to test that the new package works tomorrow and we'll be set I hope.

As for rubygem(fastthread) it can compile and be installed in F17+.  The difference is that it doesn't create a compiled library like it does for F16-.  I've tested it and it does make passenger run.
I didn't put it in as a review request because I wasn't aware that anything needed it, until I tested it with passenger.
Want me to put it in for F17+ or should we wait to see if 3.0.17 fixes that.  (I'm expecting that it doesn't fix it.)

Troy - Please submit the package review for fastthread, then.

Orion -

I've fixed all of the cases of strange perms.
I've fixed the incorrect EOL encoding issues.
The doc sub-package is now noarch.
I've added  %exclude %{gem_cache} to the files section.

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-11.fc19.src.rpm


Now, there's one outstanding issue that I need additional guidance on:

I've got a patch that fixes the fastthread issue, but it uncovers another dependency on rubygem(daemon_controller) >= 1.0.0.

The last version of daemon_controller built in Fedora is 0.2.6. Until that's fixed, the passenger-config command won't work correctly. However, daemon_controller is only needed for running a standalone passenger instance (afaik). The mod_passenger bits work just fine without it.

As I see it, there's a few options. 

1. I can remove the passenger-config command from the package until daemon_controller is updated. mod_passenger will continue to work, but running passenger standalone will be unsupported in Fedora until daemon_controller gets updated.

2. I can ship with a 'Requires: rubygem(daemon_controller) >= 1.0.0' and work with the owner of that package to get it updated in rawhide.

3. We can wait until daemon_controller is updated before finishing the review.

Brett, I don't think I'm going to be able to build a fastthread rpm that will pass review.
 - it doesn't build the binary library, so there are no binaries.
 - fastthread.rb ends up being an empty file.  So, there is no other code to run.

I went back and checked my other rubygem-fastthread rpm that I built, which fixed the ruby 1.9 passenger problem I had before, and sure enough, it was an empty fastthread.rb file.  But it did fix the passenger problem.
That was running an older 3.0.12 version of passenger.

I recommend option 1.  That way we can get a working mod_passenger into F17.  I suspect you'll have to pull everything in /usr/share/gems/gems/passenger-3.0.14/bin/.

There's a little problem in -11:

[Wed Aug 29 10:07:32 2012] [error] *** Passenger could not be initialized because of this error: Unable to start the Phusion Passenger watchdog because its executable (/usr/lib64/gems/exts/passenger-3.0.14/agents/PassengerWatchdog) does not exist. This probably means that your Phusion Passenger installation is broken or incomplete, or that your 'PassengerRoot' directive is set to the wrong value. Please reinstall Phusion Passenger or fix your 'PassengerRoot' directive, whichever is applicable.

Looks like it got installed a little off:

/usr/lib64/gems/exts/passenger-3.0.14/agents/agents/PassengerWatchdog
/usr/lib64/gems/exts/passenger-3.0.14/agents/agents/PassengerWatchdog
/usr/lib64/gems/exts/passenger-3.0.14/agents/agents/apache2
/usr/lib64/gems/exts/passenger-3.0.14/agents/agents/apache2/PassengerHelperAgent

Double "agents"

Orion - I caught that problem around the same time you did. :)

Fixed in -12:

SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-12.fc19.src.rpm
SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec

Meant to ask this earlier - any reason you are not packaging 3.0.17?

You now have native-libs owning %{gem_extdir}/agents as well as native.

Should be:

%files native
%dir %{gem_extdir}
%{gem_extdir}/agents
..

%files native-libs
%{gem_extdir}/lib

Looks like the ruby abi requires a version according to the guidelines:

Requires: ruby(abi) = 1.9.1

I mentioned it above, but I'll reiterate here. I'm sticking with 3.0.14 until this package review is complete to minimize the number of things changing during the review. I'll bump up to 3.0.17 sometime after the review is done.

Directory ownership issues and ABI declaration fixed:

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-13.fc19.src.rpm

Well, sorry for delay.

* License tag
  - Please fix License tag.
    - At least the tag "BSD" only is inappropriate.
      The "LICENSE" file is clearly MIT, not BSD.
      Also see comment 68.
    - And some documents are under CC-BY-SA, not
      under BSD (see comment 148)
    - Please recheck my license analysis on
      comment 146 and write proper license tag per
      https://fedoraproject.org/wiki/Packaging/LicensingGuidelines
      You need to check which files are included
      in which subpackages carefully.

* Removing 0-size files
  - Please check if removing 0-size files is really safe
    (because we already have some examples on rubygem
    related packages where removing 0-size files like this
    really caused some problems). If unsure, I recommend
    _not_ to remove these files.

* Directory ownership issue
  - Please make it sure that all directories created by
    installing a package is all owned properly.
    https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership
    - For example:
------------------------------------------------------------------------
# LANG=C rpm -qa | grep passenger | sort
rubygem-passenger-3.0.14-13.fc17.i686
rubygem-passenger-native-libs-3.0.14-13.fc17.i686
# rpm -ql rubygem-passenger-native-libs
/usr/lib/gems/exts/passenger-3.0.14/lib
/usr/lib/gems/exts/passenger-3.0.14/lib/native
/usr/lib/gems/exts/passenger-3.0.14/lib/native/passenger_native_support.so
# LANG=C ls -ald /usr/lib/gems/exts/passenger-3.0.14
drwxr-xr-x. 3 root root 4096  Sep  4 16:18 /usr/lib/gems/exts/passenger-3.0.14
# LANG=C rpm -qf /usr/lib/gems/exts/passenger-3.0.14
file /usr/lib/gems/exts/passenger-3.0.14 is not owned by any package
------------------------------------------------------------------------
      This must be fixed. Note that this directory is owned by
      -native package, however -native package depends on
      -native-libs, and -native-libs can be installed without
      -native subpackge.

I believe that Orion has already done functionality
check very carefully, so I will only mention packaging
issue.

Revisiting the license tag, the correct license tag for the main binary package is:

Boost 1.0 and BSD and BSD with advertising and MIT and zlib

The -doc package is: CC-BY-SA and MIT and (MIT or GPL+)

The -devel package is: Boost 1.0 and BSD and BSD with advertising and GPL+ and MIT and zlib

The mod-passenger, native, and native-libs packages are: Boost 1.0 and BSD and BSD with advertising and MIT and zlib

* License tag

- Fixed using Spot's clarifications.

I've included comments in the spec to document what license covers what portions of the package, along with a direct link to comment 148.

* Removing 0-size files

- This has so far proven to be safe. I'll keep an eye out for issues, though.

* Directory ownership issue

- native-libs now owns %gem_extdir. I considered having both -native and -native-libs own the directory, but with the dependency between native and native-libs that seemed unnecessary.

I didn't see any other directory ownership issues.

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-14.fc19.src.rpm

Checked -14 srpm. I think if Orion says this srpm "works" (wrt
functionality check) this can be approved.

One left packaging issue
* tmpfiles.d files
  - I've completely missed this one... I happened to execute
    "rpm -Va" (after reboot) and noticed below:
------------------------------------------------------------
missing     /var/run/rubygem-passenger
------------------------------------------------------------
   This is because files under /var/run are removed after reboot.
   Please follow
   https://fedoraproject.org/wiki/Packaging/Guidelines#Tmpfiles.d
   https://fedoraproject.org/wiki/Packaging:Tmpfiles.d
   and add needed files or so.

Another day, another revision...  ;-)

Tmpfiles.d support added in -15.

SPEC: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger.spec
SRPM: http://wakko666.fedorapeople.org/rubygem-passenger/rubygem-passenger-3.0.14-15.fc19.src.rpm

Well, seeing that we've reached more than 200 comments, I think it is time to approve this package!   Thanks everyone.  Still needs to run in permissive mode, but hopefully will get that shaken out soon in the main selinux-policy package.

One little note though -  Spot, the valid license short name seems to be "Boost", not "Boost 1.0" according to <http://fedoraproject.org/wiki/Licensing#Good_Licenses>.  Can you verify?

wrt packaging issue, currently I see no issue on -15.

(In reply to comment #204)
> wrt packaging issue, currently I see no issue on -15.

By the way, if you want to wrap lines to install tmpfiles.d files
with "%if 0%{?fedora} > 15 .... %endif", it is better that you
also wrap the "%{_prefix}/lib/tmpfiles.d/%{name}.conf" entry in
%file with "%if 0%{?fedora} > 15 .... %endif".

It looks like Orion set the fedora-review flag to +, but after today's Bugzilla outage, it's now looking for Mamoru's to set the fedora-review flag.

If this package is approved, can both of you guys make sure your respective fedora-review flags are set?

Approved by both Orion and me.

I confirm that "Boost" is correct, not "Boost 1.0". (Sorry for all the confusion here, feel free to correct this issue when you commit the spec to git.)

New Package SCM Request
=======================
Package Name: rubygem-passenger
Short Description: Phusion Passenger™ is an application server for Ruby on Rails
Owners: wakko666 kanarip
Branches: f17 f18 el5 el6
InitialCC:

Git done (by process-git-requests).

rubygem-passenger-3.0.17-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/rubygem-passenger-3.0.17-2.fc18

rubygem-passenger-3.0.17-2.fc18 has been pushed to the Fedora 18 testing repository.

rubygem-passenger-3.0.17-2.fc18 has been pushed to the Fedora 18 stable repository.