Spec URL: http://people.redhat.com/vondruch/rubygem-net-scp.spec SRPM URL: http://people.redhat.com/vondruch/rubygem-net-scp-1.0.4-1.fc14.src.rpm Description: A pure Ruby implementation of the SCP client protocol Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2818302
If you are still interested in importing this package into Fedora, I will have a look at this one. Instead I will appreciate it if you would review one of my review requests (e.g. bug 678678) Some notes: * Explicit version-specific Requires - Please consider if you really have to write explicit version-specific Requires like rubygems ">= 1.2". * Usually when packages shipped on currently supported Fedora branches all satisfy such version requirements, we don't add such explicit version Requires: https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires * Remove obsolete stuff - rm -rf %{buildroot} at the top of %install is no longer needed (if you intend to import this only on Fedora). ! 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).
* By the way, I also recommend to add "-V" option to "gem install" because it shows some debuging information.
(In reply to comment #1) > If you are still interested in importing this package into > Fedora, I will have a look at this one. Instead I will appreciate > it if you would review one of my review requests (e.g. bug 678678) Yes, I am still interested in import. Thank you for your comments. Here is updated version: Spec URL: http://people.redhat.com/vondruch/rubygem-net-scp.spec SRPM URL: http://people.redhat.com/vondruch/rubygem-net-scp-1.0.4-2.fc15.src.rpm Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2919651 > Some notes: > > * Explicit version-specific Requires > - Please consider if you really have to write explicit > version-specific Requires like rubygems ">= 1.2". > * Usually when packages shipped on currently supported > Fedora branches all satisfy such version requirements, > we don't add such explicit version Requires: > https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires I removed the explicit versions. You are right. > * Remove obsolete stuff > - rm -rf %{buildroot} at the top of %install is no longer needed > (if you intend to import this only on Fedora). Done > ! 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. 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. 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. 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.
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. > 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.
Note that I am not going to make this issue (i.e. my previous comment) as a blocker.
(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.
(In reply to comment #6) > (In reply to comment #4) > > 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. - Well, perhaps you are just confused about %_builddir vs %buildroot (I don't know what you mean here by "the %buildroot of %_builddir") Usually * %_builddir is ~/rpmbuild/BUILD With normal %setup -q, - rpmbuild firstly moves to ~/rpmbuild/BUILD (%_builddir) - unpack tarball - then move to ~/rpmbuild/BUILD/%name-%version - And then, when %build, %install, %check begins, rpmbuild will always move to ~/rpmbuild/BUILD/%name-%version first * On the other hand, %buildroot is usually ~/ rpmbuild/BUILDROOT/%{name}-%{version}-%{release}-%{_arch}. On install, files must be installed under %buildroot. So what I am saying is that usually - %install section should generally just do copy files under ~/rpmbuild/BUILD into ~/rpmbuild/BUILDROOT - and do not touch files under ~/rpmbuild/BUILDROOT during %check and touch only files under ~/rpmbuild/BUILD And rpmbuild always do the scripts in the order of %prep -> %build -> %install -> %check (not %prep -> %build -> %check -> %install). So even if testsuite may pollute files under %_builddir on %check, files under %buildroot won't be polluted (again %install is done before %check). Also please check this: https://fedoraproject.org/wiki/Packaging/Guidelines#Scriplets_are_only_allowed_to_write_in_certain_directories (%_builddir and %buildroot are clearly distinguished and it is written that %check should not write anything under %buildroot). > 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." - Note that when containing C extension modules, the paragraph is already invalid (as written in ruby packaging guidelines). Well, we may really have to revise current ruby packaging guideline (even for arch-independent gem based srpm). > 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. - Well, I don't think we can catch all possible bugs anyway... And for %buildroot pollution issue, please check my comments above.
(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #4) > > > > 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. > > - Well, perhaps you are just confused about %_builddir vs %buildroot > (I don't know what you mean here by "the %buildroot of %_builddir") This is clear to me. > Usually > * %_builddir is ~/rpmbuild/BUILD > With normal %setup -q, > - rpmbuild firstly moves to ~/rpmbuild/BUILD (%_builddir) > - unpack tarball > - then move to ~/rpmbuild/BUILD/%name-%version > - And then, when %build, %install, %check begins, rpmbuild > will always move to ~/rpmbuild/BUILD/%name-%version first > * On the other hand, %buildroot is usually ~/ > rpmbuild/BUILDROOT/%{name}-%{version}-%{release}-%{_arch}. > On install, files must be installed under %buildroot. > > So what I am saying is that usually > - %install section should generally just do copy files under > ~/rpmbuild/BUILD into ~/rpmbuild/BUILDROOT > - and do not touch files under ~/rpmbuild/BUILDROOT during %check > and touch only files under ~/rpmbuild/BUILD > And rpmbuild always do the scripts in the order of > %prep -> %build -> %install -> %check (not %prep -> %build -> %check > -> %install). I did not realized the order, you are right. > > So even if testsuite may pollute files under %_builddir on %check, > files under %buildroot won't be polluted (again %install is done > before %check). > > Also please check this: > > > https://fedoraproject.org/wiki/Packaging/Guidelines#Scriplets_are_only_allowed_to_write_in_certain_directories > > (%_builddir and %buildroot are clearly distinguished and it is written > that %check should not write anything under %buildroot). While I agree that pollution is wrong and the guideline is reasonable, I have the feeling that the guideline comes from C/C++ world. For such applications, you are typically executing test suite using "make test" and it is hard to adjust the test suite to run from different place, so nobody bothers. In this case the test suite ensures that the build was correct, but does not ensure that much that the installation itself will be working. But we can do this with Ruby. We can try to ensure that the installation structure, i.e. the %buildroot, is correct, although it is not free of penalties. Even better, we should foster upstream in preventing pollution of the %buildroot, i.e. they should place temporary files on suitable places, such as /tmp folder. > > > 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." > > - Note that when containing C extension modules, the paragraph is already > invalid (as written in ruby packaging guidelines). > Well, we may really have to revise current ruby packaging guideline > (even for arch-independent gem based srpm). +1 > > > 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. > > - Well, I don't think we can catch all possible bugs anyway... Everybody knows that, but we can't reconcile with it. > And for %buildroot pollution issue, please check my comments above.
(Anyway for now this [i.e. installing gem under %_builddir first] is for now just my recommendation and again I won't make this as a blocker).
Alright. ------------------------------------------------------- This package (rubygem-net-scp) is APPROVED by mtasaka -------------------------------------------------------
Thank you for your review! New Package SCM Request ======================= Package Name: rubygem-net-scp Short Description: A pure Ruby implementation of the SCP client protocol Owners: vondruch Branches: f15
Git done (by process-git-requests).
rubygem-net-scp-1.0.4-2.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/rubygem-net-scp-1.0.4-2.fc15
rubygem-net-scp-1.0.4-2.fc15 has been pushed to the Fedora 15 testing repository.
rubygem-net-scp-1.0.4-2.fc15 has been pushed to the Fedora 15 stable repository.
Package Change Request ====================== Package Name: rubygem-net-scp New Branches: epel7 Owners: tdawson