[Bug 676308] Review Request: rubygem-net-scp - A pure Ruby implementation of the SCP client protocol

bugzilla at redhat.com bugzilla at redhat.com
Thu Mar 17 12:20:49 UTC 2011


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=676308

--- Comment #6 from Vít Ondruch <vondruch at redhat.com> 2011-03-17 08:20:48 EDT ---
(In reply to comment #4)
> For just for the last comment (will check the whole thing later,
> please wait for a moment...)
> 
> (In reply to comment #3)
> > (In reply to comment #1)
> > > ! Directly installing gem archive into %buildroot
> > >   - I always recommend to install gem archive once under %_builddir
> > >     to make it sure that no additional files are generated or files
> > >     are not modified after installation is done.
> > >     * For example, some testsuites generate additional executables
> > >       or log files or modify files, which frequently confuses %files 
> > >       entry or leads to rpmbuild failure.
> > > 
> > >     And because of the same reason, it is recommended that %buildroot
> > >     is touched only in %install section (i.e. executing %check under
> > >     %buildroot is discouraged).
> > 
> > I agree with you if the gem has binary extension, but this is not the case.
> 
> First of all, I have already seen some cases in which even 
> noarch gem based srpm
> - produced binary rpms containing unneeded files (such as test.log or some
>   additional temp files generated during %check section)
>   * This usually happens when %files entry just contain %geminstdir/
> - or case to fail to build due to such addtional unneeded files
>   (if you write %files entry rather verbosely as we recommend, this
>    can happen)
> - or even generates arch-dependent files during %check (although the package
>   itself can be noarch)
> 
> test.log or so may be generated even with arch-independent gem-based rpms
> (and you can find that this is not specific to rubygem based pkgs).
> We have to make sure that %buildroot is not polluted during %check section.
> So
> - preparing everything before %install
> - and generally %install is actually only for %install
> - %check section uses %_builddir, not %buildroot
> is the easiest way for ensuring this. 
> 

I agree that we have to avoid pollution. But at the end, if the test pollutes
the %buildroot of %_builddir, you still have to either do cleanup or exclude
these files from installing/copying. So building into %buildroot cannot save
you and it is invalid argument in this discussion.

> > Unfortunately your statement is double-edged sword. You want to ensure that the
> > test suite you are executing is testing the gem, you are going to install to
> > our users. However, during install phase, there are typically done some changes
> > in folder structure, some adjustments in path etc.
> - So if some "large" structure change or so is needed on %install,
>   it must be moved to %build, actually.
> 
> > If this is true, then
> > executing of test suite in build folders is worthless, because it does not
> > ensure anything. There might be bugs introduces by the install step.
> - So see above. If such "large" change is needed on %install, it
>   must be done in %build. Basically %install is for install (as the string
>   says).
> 
> 
> > On the other hand, you can done the restructuring also during build phase, but
> > then, what is the purpose of the install section? Just copy the already
> > prepared folder structure? There is no point.
> - But this is not specific to rubygem based pkgs and we usually do so
>   (i.e. %install is only for installation, i.e. install or cp commands). 
>   Generally what is needed before installation should be done before
>   %install. For autotools based pkgs,
>   we
>   - basically just unpack tarball on %prep (tar xf foo.tar.bz2)
>   - do everything needed before installation at %build (configure -> make
>     -> some other stuffs)
>   - then in %install, basically just do "make install".
>   - in %check, we usually actually do check under %_builddir
> 
>   So actually we want to follow this structure also when using rubygem, like
>   - in %prep, just unpack gem
>   - in %build, modify or create everything needed
>   - then in %install, just install the needed files
> 
>   The problem is that we have no idea how to do this (gem install does
>   what we usually do on %prep and %build). However anyway not installing
>   files under %buildroot directly is preferred because of the reason I said
>   above.
> 
> * Note that as you know, arch-dependent gem cannot be installed under
>   %buildroot directly due to generating debuginfo rpm issue.

It is always good to follow parallels. So I agree that we should probably
follow the %prep, %build and %install as you described above, although in this
particular case it will be more work. But we should be strict about it,
therefore I would like to see this in Ruby packaging guidelines, instead of
this paragraph: "The %prep and %build sections of the specfile should be
empty."

But I can't agree that the test suite should be always executed in %_builddir,
because as I already pointed above, it might happen that you will not catch all
possible bugs and you have to care about pollution anyway.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


More information about the package-review mailing list