Hi,
In my recent request for the shorewall package to factory with the SR#131525 Andreas Jaeger has declined the request with the following comment, therefore I am bringing the discusssion to the packaging list
"Why are you running configure as part of the %install section? This is not normal, it should be part of the %build section. If this is really correct, then document why it is done this way - or let's discuss on opensuse-packaging how to do this better. Thanks, Andreas "
First of let me clarify the process, as the configure command is not actually a configure thing, but to set the install variables to the shorewall install program, and this is approach is the same way upstream rpm specs which are for each shorewall package separately and by fedora packages at koji http://koji.fedoraproject.org/koji/buildinfo?buildID=341839 and for RHEL releases by http://www.invoca.ch/pub/packages/shorewall/RPMS/ils-5/SRPMS/.
Therefore as the maintainer, I have followed the same approach as upstream and other package maintainers and to maintain one spec file for the whole shorewall system also used the combined approach.
I am behind my logic of doing the packaging the way it is, but I am open to suggestions as well.
Thanks
Togan
Hi Togan,
On Sun, 26 Aug 2012 17:18:55 +0200, Togan Muftuoglu toganm@opensuse.org wrote:
First of let me clarify the process, as the configure command is not actually a configure thing, but to set the install variables to the shorewall install program, and this is approach is the same way upstream rpm specs
Andreas didn't check the script itself. In most cases, configure is the name of a script to configure sources for building. As shorewalls configure is meant for something completely different, you should add a comment in the script to make ist clear that this script sets up things during the installation and also add a comment to the .changes file to make it clear for any reviewer that you know what you're doing and why you do it that way. That should clear the way for a checkin to factory.
hth Philipp
Hi Philipp,
On 08/26/2012 06:43 PM, Philipp Thomas wrote:
Hi Togan,
On Sun, 26 Aug 2012 17:18:55 +0200, Togan Muftuoglu toganm@opensuse.org wrote:
First of let me clarify the process, as the configure command is not actually a configure thing, but to set the install variables to the shorewall install program, and this is approach is the same way upstream rpm specs
Andreas didn't check the script itself. In most cases, configure is the name of a script to configure sources for building. As shorewalls configure is meant for something completely different, you should add a comment in the script to make ist clear that this script sets up things during the installation and also add a comment to the .changes file to make it clear for any reviewer that you know what you're doing and why you do it that way. That should clear the way for a checkin to factory.
Ok sounds fine. In that case I would reopen the SR and do the changes in the next update as Shorewall is very active in terms of development and the next release will also enable me to remove some of the suse specific patches as they are now upstreamed
Togan
On Sunday 26 August 2012 19.01:16 Togan Muftuoglu wrote:
Hi Philipp,
On 08/26/2012 06:43 PM, Philipp Thomas wrote:
Hi Togan,
On Sun, 26 Aug 2012 17:18:55 +0200, Togan Muftuoglu toganm@opensuse.org wrote:
First of let me clarify the process, as the configure command is not actually a configure thing, but to set the install variables to the shorewall install program, and this is approach is the same way upstream rpm specs
Andreas didn't check the script itself. In most cases, configure is the name of a script to configure sources for building. As shorewalls configure is meant for something completely different, you should add a comment in the script to make ist clear that this script sets up things during the installation and also add a comment to the .changes file to make it clear for any reviewer that you know what you're doing and why you do it that way. That should clear the way for a checkin to factory.
Ok sounds fine. In that case I would reopen the SR and do the changes in the next update as Shorewall is very active in terms of development and the next release will also enable me to remove some of the suse specific patches as they are now upstreamed
Togan
And keep the good work Togan, your shorewall packages are really appreciate.
On 08/26/2012 08:43 PM, Bruno Friedmann wrote:
Ok sounds fine. In that case I would reopen the SR and do the changes in the next update as Shorewall is very active in terms of development and the next release will also enable me to remove some of the suse specific patches as they are now upstreamed
Togan
And keep the good work Togan, your shorewall packages are really appreciate.
Thanks, Bruno I have reopened the SR#131525 but in the mean time packages in the security:netfilter repo, as you know, are available and with the 4.5.7 for those using fedora the packages are also there ;) (hint even fedora doesn't have them as yet) and if I can I'll try to fix rhel packaging as well
Togan
On Sunday, August 26, 2012 19:01:16 Togan Muftuoglu wrote:
Hi Philipp,
On 08/26/2012 06:43 PM, Philipp Thomas wrote:
Hi Togan,
On Sun, 26 Aug 2012 17:18:55 +0200, Togan Muftuoglu
toganm@opensuse.org wrote:
First of let me clarify the process, as the configure command is not actually a configure thing, but to set the install variables to the shorewall install program, and this is approach is the same way upstream rpm specs
Andreas didn't check the script itself. In most cases, configure is the name of a script to configure sources for building. As shorewalls configure is meant for something completely different, you should add a comment in the script to make ist clear that this script sets up things during the installation and also add a comment to the .changes file to make it clear for any reviewer that you know what you're doing and why you do it that way. That should clear the way for a checkin to factory.
Ok sounds fine. In that case I would reopen the SR and do the changes in the next update as Shorewall is very active in terms of development and the next release will also enable me to remove some of the suse specific patches as they are now upstreamed
I've accepted the request now and look forward to your next update with a few comments in it to not confuse me again ;)
Thanks! Andreas
On 08/27/2012 11:30 AM, Andreas Jaeger wrote:
I've accepted the request now and look forward to your next update with a few comments in it to not confuse me again ;)
Thanks Andreas, and I have already added the comment to the spec with a link to this thread for future reference. So with the next update it will be there ;)
Togan
-
Andreas Jaeger -
Bruno Friedmann -
Philipp Thomas -
Togan Muftuoglu