Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Petr Spacek]

On 25.4.2016 10:12, Lukas Slebodnik wrote:
> On (25/04/16 09:59), Jan Cholasta wrote:
>> On 25.4.2016 09:34, Petr Spacek wrote:
>>> On 25.4.2016 09:29, Lukas Slebodnik wrote:
 On (25/04/16 07:23), Jan Cholasta wrote:
> Hi,
>
> On 22.4.2016 13:29, Petr Spacek wrote:
>> Hello,
>>
>> Makefile: add sed to BuildRequires
>>
>> It was requried since forever but we did not explicitly mention it.
>
> IIRC sed is part of the minimum build environemnt and as such should not 
> be
> explicitly required in the spec file. I personally don't care, but this is
> the likely reason why it wan't there from the beginning.
>
 +1

 It is part of group "@buildsys-build".
 and fedora packaging guidelines does not recommend to list
 packages from this group in BuildRequires.
>>>
>>> I consider this piece of Fedora guidelines brain-dead as "explicit is better
>>> than implicit". Anyway, feel free to NACK it so the status of the patch is
>>> clear and this thread can die. I do not insist on it.
>>
>> I can't find it in the guidelines anymore, so LGTM.
>>
> It seems that it was changed since I read it last time.
> 
> There is vague description of which packages should be there.
> http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires_2
>It is important that your package list all necessary build dependencies
>using the BuildRequires?:
>tag. You may assume that enough of an environment exists for RPM to 
> function
>and execute basic shell scripts, but you should not assume any other 
> packages
>are present as RPM dependencies and anything brought into the buildroot
>by the build system may change over time.
> 
> But utility fedora-review still complains if you list packages from group
> "@buildsys-build"

So, should I drop the patch from my queue or not?

I still think that it is brain-dead not to list dependencies explicitly but
you tell me if I should remove the patch from my waiting-for-review-queue or 
not.

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Martin Basti]

On 25.04.2016 09:39, Lukas Slebodnik wrote:

On (22/04/16 13:29), Petr Spacek wrote:

Hello,

Makefile: add sed to BuildRequires

It was requried since forever but we did not explicitly mention it.

Makefile: replace perl with sed

Perl was missing in BuildRequires anyway and it is used only on one place,
all other places are using sed.

--
Petr^2 Spacek

>From 3c7a3c87d62358407d856119e384c70040acec1e Mon Sep 17 00:00:00 2001

From: Petr Spacek 
Date: Fri, 22 Apr 2016 10:40:11 +0200
Subject: [PATCH] Makefile: replace perl with sed

Perl was missing in BuildRequires anyway and it is used only on one place,
all other places are using sed.
---
Makefile | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)


ACK

>From 2deaef91ab71c0e78b2142c2102cfe65f0e4ed96 Mon Sep 17 00:00:00 2001

From: Petr Spacek 
Date: Fri, 22 Apr 2016 10:40:37 +0200
Subject: [PATCH] Makefile: add sed to BuildRequires

It was requried since forever but we did not explicitly mention it.
---
freeipa.spec.in | 1 +
1 file changed, 1 insertion(+)


NACK

As it was mentioned elsewhere in thread this patch is not needed.

LS


pushed to master:
* 8689e6be515aaf5b3d6c891e9e450b99cd8af8d4 Makefile: replace perl with sed

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Lukas Slebodnik]

On (25/04/16 09:59), Jan Cholasta wrote:
>On 25.4.2016 09:34, Petr Spacek wrote:
>> On 25.4.2016 09:29, Lukas Slebodnik wrote:
>> > On (25/04/16 07:23), Jan Cholasta wrote:
>> > > Hi,
>> > > 
>> > > On 22.4.2016 13:29, Petr Spacek wrote:
>> > > > Hello,
>> > > > 
>> > > > Makefile: add sed to BuildRequires
>> > > > 
>> > > > It was requried since forever but we did not explicitly mention it.
>> > > 
>> > > IIRC sed is part of the minimum build environemnt and as such should not 
>> > > be
>> > > explicitly required in the spec file. I personally don't care, but this 
>> > > is
>> > > the likely reason why it wan't there from the beginning.
>> > > 
>> > +1
>> > 
>> > It is part of group "@buildsys-build".
>> > and fedora packaging guidelines does not recommend to list
>> > packages from this group in BuildRequires.
>> 
>> I consider this piece of Fedora guidelines brain-dead as "explicit is better
>> than implicit". Anyway, feel free to NACK it so the status of the patch is
>> clear and this thread can die. I do not insist on it.
>
>I can't find it in the guidelines anymore, so LGTM.
>
It seems that it was changed since I read it last time.

There is vague description of which packages should be there.
http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires_2
   It is important that your package list all necessary build dependencies
   using the BuildRequires?:
   tag. You may assume that enough of an environment exists for RPM to function
   and execute basic shell scripts, but you should not assume any other packages
   are present as RPM dependencies and anything brought into the buildroot
   by the build system may change over time.

But utility fedora-review still complains if you list packages from group
"@buildsys-build"

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Jan Cholasta]

On 25.4.2016 09:34, Petr Spacek wrote:

On 25.4.2016 09:29, Lukas Slebodnik wrote:

On (25/04/16 07:23), Jan Cholasta wrote:

Hi,

On 22.4.2016 13:29, Petr Spacek wrote:

Hello,

Makefile: add sed to BuildRequires

It was requried since forever but we did not explicitly mention it.


IIRC sed is part of the minimum build environemnt and as such should not be
explicitly required in the spec file. I personally don't care, but this is
the likely reason why it wan't there from the beginning.


+1

It is part of group "@buildsys-build".
and fedora packaging guidelines does not recommend to list
packages from this group in BuildRequires.


I consider this piece of Fedora guidelines brain-dead as "explicit is better
than implicit". Anyway, feel free to NACK it so the status of the patch is
clear and this thread can die. I do not insist on it.


I can't find it in the guidelines anymore, so LGTM.

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Lukas Slebodnik]

On (22/04/16 13:29), Petr Spacek wrote:
>Hello,
>
>Makefile: add sed to BuildRequires
>
>It was requried since forever but we did not explicitly mention it.
>
>Makefile: replace perl with sed
>
>Perl was missing in BuildRequires anyway and it is used only on one place,
>all other places are using sed.
>
>-- 
>Petr^2 Spacek

>From 3c7a3c87d62358407d856119e384c70040acec1e Mon Sep 17 00:00:00 2001
>From: Petr Spacek 
>Date: Fri, 22 Apr 2016 10:40:11 +0200
>Subject: [PATCH] Makefile: replace perl with sed
>
>Perl was missing in BuildRequires anyway and it is used only on one place,
>all other places are using sed.
>---
> Makefile | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
ACK

>From 2deaef91ab71c0e78b2142c2102cfe65f0e4ed96 Mon Sep 17 00:00:00 2001
>From: Petr Spacek 
>Date: Fri, 22 Apr 2016 10:40:37 +0200
>Subject: [PATCH] Makefile: add sed to BuildRequires
>
>It was requried since forever but we did not explicitly mention it.
>---
> freeipa.spec.in | 1 +
> 1 file changed, 1 insertion(+)
>
NACK

As it was mentioned elsewhere in thread this patch is not needed.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Petr Spacek]

On 25.4.2016 09:29, Lukas Slebodnik wrote:
> On (25/04/16 07:23), Jan Cholasta wrote:
>> Hi,
>>
>> On 22.4.2016 13:29, Petr Spacek wrote:
>>> Hello,
>>>
>>> Makefile: add sed to BuildRequires
>>>
>>> It was requried since forever but we did not explicitly mention it.
>>
>> IIRC sed is part of the minimum build environemnt and as such should not be
>> explicitly required in the spec file. I personally don't care, but this is
>> the likely reason why it wan't there from the beginning.
>>
> +1
> 
> It is part of group "@buildsys-build".
> and fedora packaging guidelines does not recommend to list
> packages from this group in BuildRequires.

I consider this piece of Fedora guidelines brain-dead as "explicit is better
than implicit". Anyway, feel free to NACK it so the status of the patch is
clear and this thread can die. I do not insist on it.

-- 
Petr^2 Spacek

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Lukas Slebodnik]

On (25/04/16 07:23), Jan Cholasta wrote:
>Hi,
>
>On 22.4.2016 13:29, Petr Spacek wrote:
>> Hello,
>> 
>> Makefile: add sed to BuildRequires
>> 
>> It was requried since forever but we did not explicitly mention it.
>
>IIRC sed is part of the minimum build environemnt and as such should not be
>explicitly required in the spec file. I personally don't care, but this is
>the likely reason why it wan't there from the beginning.
>
+1

It is part of group "@buildsys-build".
and fedora packaging guidelines does not recommend to list
packages from this group in BuildRequires.

LS

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0097-0098] Makefile: replace perl with sed

[Jan Cholasta]

Hi,

On 22.4.2016 13:29, Petr Spacek wrote:

Hello,

Makefile: add sed to BuildRequires

It was requried since forever but we did not explicitly mention it.


IIRC sed is part of the minimum build environemnt and as such should not 
be explicitly required in the spec file. I personally don't care, but 
this is the likely reason why it wan't there from the beginning.




Makefile: replace perl with sed

Perl was missing in BuildRequires anyway and it is used only on one place,
all other places are using sed.


Honza

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code