Fwd: Re: ruby 1.8.7.x for rawhide

[Mamoru Tasaka]

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.
If comprecated sed craft is needed, creating a patch is much appreciated.

Regards,
Mamoru