Re: Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

Re: Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]

On 06 May 2015 13:37, Mark Wielaard wrote:
> On Wed, 2015-05-06 at 07:22 -0400, Frank Ch. Eigler wrote:
> > > I would like to get rid of the portable branch. [...]  
> > 
> > What is the argument against getting rid of the portable branch by
> > merging it into master?
> 
> It contains hacks that we don't want to have on master because they
> either just conflict with current standards/implementations (like using
> %a in scanf instead of %m), inefficient (busy looping to detect process
> state changes, because of buggy kernels), have conditional code paths
> (which are a pain to maintain/keep working and obscure the code) or have
> workarounds that are subtly wrong (like in this case where switching
> back to path based file manipulation after already having opened and
> checked a file descriptor might open up possible security issues because
> of Time-of-check Time-of-use race conditions).
> 
> If there are specific hacks you would like to see brought over from the
> portable branch to master, please do propose and we can discuss them.
> But I really think none of them are needed or should be used these days.

there's still the issue of --disable-werror
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 799 bytes --]

On Wed, May 13, 2015 at 12:17:58PM -0400, Frank Ch. Eigler wrote:
> > [...]  because you guys (reasonably) cannot test every gcc/C library
> > version/flags/arch combination.  [...]
> 
> Exactly - and even we encountered the downsides of this as
> fedora/rawhide moved between different patches of gcc 5.* that
> had different warning policies.

As far as I know when gcc 5.* started to report new issues we fixed
them in Fedora and backported them when they made sense.

I really am a fan of fixing compiler warnings, they very often do point
out something that needs attention. I also really would like to know
what the advantage of a new configure option is over users being able
to set their own CFLAGS with -Wno-error included if they feel that is
necessary.

Cheers,

Mark

Re: Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 2194 bytes --]

Hi Mike,

On Wed, May 13, 2015 at 12:09:50PM -0400, Mike Frysinger wrote:
> On 13 May 2015 16:18, Mark Wielaard wrote:
> > On Thu, 2015-05-07 at 22:08 -0400, Mike Frysinger wrote:
> > > > If there are specific hacks you would like to see brought over from the
> > > > portable branch to master, please do propose and we can discuss them.
> > > > But I really think none of them are needed or should be used these days.
> > > 
> > > there's still the issue of --disable-werror
> > 
> > I believe we discussed before, but could you remind me why it is
> > necessary for newer GCC versions?
> > 
> > If there are any warnings that are turned into errors we should really
> > just fix them. Do you have any specific examples?
> > 
> > What does the configure option add over just using CFLAGS=-Wno-error?
> 
> because you guys (reasonably) cannot test every gcc/C library
> version/flags/arch combination.  focusing on newer versions makes sense,
> but not all distros are always running the latest.  i also agree that
> having it default to on is ok and some distros which have tight control
> over everything (like fedora) will set it to on.  but i think that should
> be left to the distro to control.  in practice, we already are either by
> `sed` or patching in the werror configure flag.

Sure, but if you are using none supported versions of gcc and glibc you
are on your own already and will have to patch yourself. For the rest we
would really be error free. I do hope to setup a buildbot soon with slaves
for most supported arches to catch any issues early.

What is the advantage of --disable-werror over using CFLAGS=-Wno-error?

> keep in mind that not all warnings are even correct -- gcc has false
> positives from time to time.  trying to track how to squelch those across
> multiple gcc versions is a waste of time.

I don't think so (for a reasonable number of gcc versions). We pick the
enabled warning flags carefully and make sure no warnings slip through
(possible disabling them per file if necessary). If we enable specific
warnings in elfutils they have often caught important bugs that really
should be fixed.

Cheers,

Mark

Re: Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 291 bytes --]

Hi -

> [...]  because you guys (reasonably) cannot test every gcc/C library
> version/flags/arch combination.  [...]

Exactly - and even we encountered the downsides of this as
fedora/rawhide moved between different patches of gcc 5.* that
had different warning policies.

- FChE

Re: Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]

On 13 May 2015 16:18, Mark Wielaard wrote:
> On Thu, 2015-05-07 at 22:08 -0400, Mike Frysinger wrote:
> > > If there are specific hacks you would like to see brought over from the
> > > portable branch to master, please do propose and we can discuss them.
> > > But I really think none of them are needed or should be used these days.
> > 
> > there's still the issue of --disable-werror
> 
> I believe we discussed before, but could you remind me why it is
> necessary for newer GCC versions?
> 
> If there are any warnings that are turned into errors we should really
> just fix them. Do you have any specific examples?
> 
> What does the configure option add over just using CFLAGS=-Wno-error?

because you guys (reasonably) cannot test every gcc/C library version/flags/arch 
combination.  focusing on newer versions makes sense, but not all distros are 
always running the latest.  i also agree that having it default to on is ok and 
some distros which have tight control over everything (like fedora) will set it 
to on.  but i think that should be left to the distro to control.  in practice, 
we already are either by `sed` or patching in the werror configure flag.

keep in mind that not all warnings are even correct -- gcc has false positives 
from time to time.  trying to track how to squelch those across multiple gcc 
versions is a waste of time.

i don't have specific examples readily available ... we noticed in Gentoo when 
users report via irc (which has no logs).  i know some more extensive flags like 
lto and inline can often trigger unresolvable warnings about functions not being 
inlined.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Thu, 2015-05-07 at 22:08 -0400, Mike Frysinger wrote:
> > If there are specific hacks you would like to see brought over from the
> > portable branch to master, please do propose and we can discuss them.
> > But I really think none of them are needed or should be used these days.
> 
> there's still the issue of --disable-werror

I believe we discussed before, but could you remind me why it is
necessary for newer GCC versions?

If there are any warnings that are turned into errors we should really
just fix them. Do you have any specific examples?

What does the configure option add over just using CFLAGS=-Wno-error?

Thanks,

Mark

Re: Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1085 bytes --]

On Wed, 2015-05-06 at 07:22 -0400, Frank Ch. Eigler wrote:
> > I would like to get rid of the portable branch. [...]  
> 
> What is the argument against getting rid of the portable branch by
> merging it into master?

It contains hacks that we don't want to have on master because they
either just conflict with current standards/implementations (like using
%a in scanf instead of %m), inefficient (busy looping to detect process
state changes, because of buggy kernels), have conditional code paths
(which are a pain to maintain/keep working and obscure the code) or have
workarounds that are subtly wrong (like in this case where switching
back to path based file manipulation after already having opened and
checked a file descriptor might open up possible security issues because
of Time-of-check Time-of-use race conditions).

If there are specific hacks you would like to see brought over from the
portable branch to master, please do propose and we can discuss them.
But I really think none of them are needed or should be used these days.

Cheers,

Mark

Re: Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

[Frank Ch. Eigler]

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

Hi -

> I would like to get rid of the portable branch. [...]  

What is the argument against getting rid of the portable branch by
merging it into master?

> Nobody really should be using it unless they are using glibc <= 2.5,
> kernel <= 2.6.18 or gcc < 4.4. Hopefully by now those are really
> ancient memory. [...]

(e.g. RHEL5 is supported till 2020.)


- FChE

Getting rid of portable branch (Was: [PATCH 3/6] Fall back on utimes if futimes is not available)

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

On Tue, 2015-05-05 at 12:26 -0700, Roland McGrath wrote:
> Almost exactly that is already on the portable branch.

I would like to get rid of the portable branch. It is only there for
really ancient glibc versions (which don't have futimens). I do still
update the branch and test it against RHEL5 just before each release.
But my intention is to do it one last time for the next 0.162 release,
and then just get rid of it.

Nobody really should be using it unless they are using glibc <= 2.5,
kernel <= 2.6.18 or gcc < 4.4. Hopefully by now those are really ancient
memory.

Cheers,

Mark