Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772243
--- Comment #5 from Doug Ledford dledford@redhat.com 2012-01-09 15:15:24 EST --- (In reply to comment #1)
Just some comments:
- License seems to be GPLv2 or BSD -- not GPLv2+ or BSD
- You can use the name macro in Source0
- Comment on the patches in the spec file, if possible
All done.
- FSF address is wrong in rds-sample.c
While checking on this, I noted that the main source files redirect a person to a copy of the GPLv2 in the file COPYING, yet that file isn't present.
- Please harmonize the use of RPM_BUILD_ROOT and buildroot macro
Done.
- If you don't go for EPEL 5, you can delete the buildroot definition, the
clean section and the rm in the install section
The package will be shared with RHEL, including RHEL5, so these should stay for now.
- defattr is no longer necessary
See above.
- Remove README from the documentation as it holds no valuable information
Done.
- The optflags are not used when compiling
This is fixed, but it's kind of ugly. The upstream build files simply do not honor passed in CFLAGS via the environment, only if you explicitly set CFLAGS to the make comment are they honored (and this is in contradiction with the fact that configure says you can do CFLAGS=blah and it will put it in the makefile, it doesn't actually do so however). And the makefile doesn't append its own stuff to the passed in CFLAGS, it's either the built in CFLAGS or the passed in, no merging. So, I had to include the one item that the makefile needs in addition to the rpm opt stuff in order for it to compile.
- You can patch the permissions for the executables in rds-tools-make.patch
instead of changing them in the install section
Done.
- What is that rds.conf file about?
The rds protocol is autoloaded if anyone attempts to open an rds socket. However, there is no method in place to determine if either the tcp transport, rdma transport, or both should be loaded in order to satisfy the needs of the rds protocol. So, whenever we load the rds protocol, also load both transport modules. That way, whatever one is appropriate can be used to service the request versus failing to open an rds socket even though the rds protocol was successfully loaded.