Fwd: Re: ruby 1.8.7.x for rawhide
Jim Meyering
jim at meyering.net
Wed Jun 23 07:12:31 UTC 2010
Mamoru Tasaka wrote:
> Jim Meyering wrote, at 06/23/2010 06:03 AM +9:00:
>> Jim Meyering wrote:
>>
>>> 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_PLATFORM
>>>>>>
>>>>> Using 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 1
>>>>>
>>>>> But 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 1
>>>>
>>>> This 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".
>>
>> FYI, I've confirmed that this works for me:
>>
>> commit 8cc2823aec419d282dfe3a1b62ed46e7453adbed
>> Author: Jim Meyering<meyering at redhat.com>
>> Date: Tue Jun 22 22:15:59 2010 +0200
>>
>> remove mkconfig.rb-modifying patch(#23) altogether
>>
>> - Remove patch 23 altogether. Replace it with mkconfig.rb-modifying
>> code in %install.
>> - ruby-multilib.patch: Remove file.
>>
>> diff --git a/RHEL-6/ruby.spec b/RHEL-6/ruby.spec
>> index 00e7c50..ad0c7db 100644
>> --- a/RHEL-6/ruby.spec
>> +++ b/RHEL-6/ruby.spec
>> @@ -25,7 +25,7 @@
>>
>> Name: ruby
>> Version: %{rubyver}%{?dotpatchlevel}
>> -Release: 2%{?dist}
>> +Release: 3%{?dist}
>> License: Ruby or GPLv2
>> URL: http://www.ruby-lang.org/
>> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
>> @@ -53,8 +53,6 @@ Source4: irb.1
>> # http://lists.fedoraproject.org/pipermail/ruby-sig/2010-May/000096.html
>> Source100: ruby-1.8-rev%{tk_using_svn_number}_trunk-ext_tk.tar.gz
>>
>> -# Patch23 is Fedora specific
>> -Patch23: ruby-multilib.patch
>> # Patch27 is not in the upstream VCS.
>> # This patch, while not technically required in the context of Rails,
>> # does seem like a useful addition: improved safety/compatibility:
>> @@ -193,7 +191,6 @@ pushd %{name}-%{arcver}
>> find tk -type d -name \.svn | sort -r | xargs rm -rf
>> )
>>
>> -%patch23 -p1
>> %patch27 -p0
>> %patch29 -p1
>> %patch30 -p1
>> @@ -360,6 +357,24 @@ popd
>> # done
>> cd ..
>>
>> +# Change /lib/ruby/ to /lib64/ruby/ on a 64-bit system
>> +if test %{_lib} != lib; then
>> + d=$RPM_BUILD_DIR/%{name}-%{version}/%{name}-%{arcver}
>> + t=$d/mkconfig.rb
>> + # ruby-1.9.2 has this line in tool/mkconfig.rb:
>> + # prefix = "/lib/ruby/#{version}/#{arch}"
>> + # while ruby-1.8.7 has this in mkconfig.rb:
>> + # prefix = '/lib/ruby/' + RUBY_VERSION.sub(/\.\d+$/, '') + '/' + RUBY_PLATFORM
>> + # The following covers both:
>> + test -f $t || t=$d/tool/mkconfig.rb
>> + if grep '^prefix = ./lib/ruby/' $t; then
>> + sed -i 's,^\(prefix = .\)/lib/ruby/,\1%{_lib}/ruby/,' $t || exit 1
>> + else
>> + echo "unexpected content in $t"
>> + exit 1
>> + fi
>> +fi
>> +
>> # installing binaries ...
>> make -C $RPM_BUILD_DIR/%{name}-%{version}/%{name}-%{arcver} DESTDIR=$RPM_BUILD_ROOT install
>>
>> @@ -546,6 +561,11 @@ rm -rf $RPM_BUILD_ROOT
>> %doc tmp-ruby-docs/ruby-libs/*
>>
>> %changelog
>> +* Tue Jun 22 2010 Jim Meyering<meyering at redhat.com> - 1.8.7.249-3
>> +- Remove patch 23 altogether. Replace it with mkconfig.rb-modifying
>> + code in %install.
>> +- ruby-multilib.patch: Remove file.
>> +
>> * Tue Jun 22 2010 Jim Meyering<meyering at redhat.com> - 1.8.7.249-2
>> - Integrate addition of patch101 by Mohammed Morsi. This fixes
>> the segv-inducing marshaling bug affecting rails-3.0.0.
>>
>> The above isn't quite accurate.
>> You need this additional change (to the %changelog text!)
>> in order to avoid a cryptic "Package already exists: %package debuginfo"
>> error from rpmbuild:
>>
>> - code in %install.
>> + code in "install", above.
>>
>> I guess that's rpmbuild's way of saying you must not refer
>> to "%install" in a %changelog entry.
>
> Please avoid to use such comprecated sed craft. While I use sed craft
> so many times, this type of craft makes it difficult for other persons
> to understand what these lines are for.
That is why I added nearly as many lines of comment as of code.
Considering the size of the existing %install section and its
code/comment ratio, I'm surprised that you think my addition would
be difficult to understand.
If understanding is the only issue, let me know if adding more
comments would help.
> If comprecated sed craft is needed, creating a patch is much appreciated.
I presume you would prefer a patch, so I am discarding the above,
leaving the patch I used before. I cannot take David Lutterkort's
suggestion to use the likes of %{_lib} without adding shell code
(or fragile envvar settings) to %install.
FYI, here is my ruby-multilib.patch (patch23):
diff -ruN ruby-1.8.3.orig/mkconfig.rb ruby-1.8.3/mkconfig.rb
--- ruby-1.8.7/mkconfig.rb 2008-06-06 12:39:57.000000000 +0200
+++ ruby-1.8.7/mkconfig.rb 2010-06-21 11:17:13.839498249 +0200
@@ -39,6 +39,7 @@ vars = {}
has_version = false
continued_name = nil
continued_line = nil
+lib_64 = ''
File.foreach "config.status" do |line|
next if /^#/ =~ line
name = nil
@@ -96,13 +97,21 @@ File.foreach "config.status" do |line|
v_others << v
end
has_version = true if name == "MAJOR"
+
+ # If the target architecture is one of the following,
+ # ppc64 s390x sparc64 x86_64
+ # then use "lib64", not "lib" in prefix.
+ if name == "target_cpu" and (/64"$/ =~ val or val == '"s390x"')
+ lib_64 = '64'
+ end
end
# break if /^CEOF/
end
drive = File::PATH_SEPARATOR == ';'
-prefix = '/lib/ruby/' + RUBY_VERSION.sub(/\.\d+$/, '') + '/' + RUBY_PLATFORM
+prefix = "/lib#{lib_64}/ruby/" \
+ + RUBY_VERSION.sub(/\.\d+$/, '') + '/' + RUBY_PLATFORM
print " TOPDIR = File.dirname(__FILE__).chomp!(#{prefix.dump})\n"
print " DESTDIR = ", (drive ? "TOPDIR && TOPDIR[/\\A[a-z]:/i] || " : ""), "'' unless defined? DESTDIR\n"
print " CONFIG = {}\n"
More information about the ruby-sig
mailing list