Mohammed Morsi wrote:
On 06/22/2010 01:26 PM, Jim Meyering wrote:
David Lutterkort wrote:
On Tue, 2010-06-22 at 12:49 +0200, Jim Meyering wrote:
Mohammed Morsi wrote:
I added this patch to my specfile as well, bumped the release, and updated the changelog to include your feedback.
Hi again,
rpmlint complains about this conditionally-applied patch:
%ifarch ppc64 s390x sparc64 x86_64 %patch23 -p1 %endif
That patch (ruby-1.8.7-multilib.patch) is small enough that I'll include it here, for reference:
--- ruby-1.8.7-p249/mkconfig.rb.orig 2010-06-15 11:30:44.000000000 -0400 +++ ruby-1.8.7-p249/mkconfig.rb 2010-06-15 11:31:01.000000000 -0400 @@ -102,7 +102,7 @@
drive = File::PATH_SEPARATOR == ';'
-prefix = '/lib/ruby/' + RUBY_VERSION.sub(/.\d+$/, '') + '/' + RUBY_PLATFORM +prefix = '/lib64/ruby/' + RUBY_VERSION.sub(/.\d+$/, '') + '/' + RUBY_PLATFORM
It is easy to perform the same task using an unconditional patch, so I wrote this replacement:
Couldn't we just get the right value from the environment ? I haven't looked if it's already in one of the standard env vars that rpmbuild sets up, but if bad comes to worst, couldn't we just do 'export LIB_PREFIX=%{_libdir}' in the spec file and then
prefix = ENV["LIB_PREFIX"].gsub(%r{^/usr}, "") + "/ruby" + RUBY_VERSION.sub(/\.\d+$/, '') + '/' + RUBY_PLATFORMUsing an rpm variable like that is more maintainable than enumerating 64-bit architectures or even using a regexp like /64$/ like I did. However, while it would work when building via rpm tools, if someone ever builds manually, they would have to know to set that envvar -- too easily missed or forgotten.
How would anyone go about building this manually with this change included? AFAIK since this change is only in the patch that is pulled in via the spec / rpmbuild, the only way that it will appear is if someone is building ruby using the rpm tools.
Hi Mo,
I've seen it documented to run e.g., "make prep" for a project and then cd into the directory that creates and "develop" (i.e., run make, etc.). This is not a hypothetical case.
However, we can do something similar that might be more robust: run something like this from the spec file:
sed -i 's,/lib/ruby/,%{_whatever}/,' .../mkconfig.rb || exit 1But that is fragile in its own way. If mkconfig.rb changes, the sed regexp may fail to match or it may match in some new place, introducing an unwanted change.
Agreed, this global sed will most likely cause headaches in the future.
If you're so concerned, and don't like the insurance below, it's easy to make the sed regexp more specific. The line it's modifying is long and unlikely to change frequently.
We could mitigate that risk by adding tests that would fail if our preconditions stop being met:
# Ensure that the "sed" command below does the right thing. grep 'prefix = ./lib/ruby/. + RUBY_VERSION.sub' || exit 1 test $(grep -c '/lib/ruby/') = 1 || exit 1 # Use /lib64/ruby/ on a 64-bit system. sed -i 's,/lib/ruby/,%{_whatever}/,' mkconfig.rb || exit 1This will also lead to more maintenance as if either of those conditions become true and cause rpmbuild to exit, we will need to figure out an alternative solution to this problem.
That's the whole idea of this approach. In the unlikely event that something important changes, you absolutely do want to be forced to get involved. You'd get an rpmbuild failure that has to be investigated, rather than a silent change in behavior that someone would have to debug farther away from the source.
It is for the same reason that patches are applied using the most strict setting, allowing no "fuzz".