diff mbox

compile: rm -rf -> ftw()+rmdir()+unlink() [Re: [patch] compile: Fix MinGW build]

Message ID 20141217210144.GA26674@host2.jankratochvil.net
State New
Headers show

Commit Message

Jan Kratochvil Dec. 17, 2014, 9:01 p.m. UTC
On Tue, 16 Dec 2014 10:04:02 +0100, Kai Tietz wrote:
> Why not using here instead an implementation using FTW-API?

Done.


> At least mingw-w64 added this API recently to runtime for gcc's sake, so
> implementation of an 'rm -rf' should be pretty easy.

It has built on Fedora 21 x86_64 mingw64 for both 32-bit and 64-bit targets.
I am not sure about various other Unices but if the patch gets approved...


On Wed, 17 Dec 2014 18:29:51 +0100, Steve Ellcey wrote:
> /scratch/sellcey/repos/nightly2/src/binutils-gdb/gdb/compile/compile.c:175:10: error: ignoring return value of 'system', declared with attribute warn_unused_result [-Werror=unused-result]
> cc1: all warnings being treated as errors
> make[1]: *** [compile.o] Error 1

It should get fixed by this patch.


I have briefly tested (on Linux; on MinGW I have only tested the build) it
really does delete the directory and its files.

OK for check-in?


Thanks,
Jan
gdb/ChangeLog
2014-12-17  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* compile/compile.c: Include ftw.h.
	(do_rmdir_fn): New function.
	(do_rmdir): Call nftw with it.

Comments

Steve Ellcey Dec. 17, 2014, 10:07 p.m. UTC | #1
On Wed, 2014-12-17 at 22:01 +0100, Jan Kratochvil wrote:

> On Wed, 17 Dec 2014 18:29:51 +0100, Steve Ellcey wrote:
> > /scratch/sellcey/repos/nightly2/src/binutils-gdb/gdb/compile/compile.c:175:10: error: ignoring return value of 'system', declared with attribute warn_unused_result [-Werror=unused-result]
> > cc1: all warnings being treated as errors
> > make[1]: *** [compile.o] Error 1
> 
> It should get fixed by this patch.

Yes, that patch fixes my build problem.

Steve Ellcey
sellcey@imgtec.com
Pedro Alves Dec. 17, 2014, 10:41 p.m. UTC | #2
On 12/17/2014 09:01 PM, Jan Kratochvil wrote:
> On Tue, 16 Dec 2014 10:04:02 +0100, Kai Tietz wrote:
>> Why not using here instead an implementation using FTW-API?
> 
> Done.
> 
> 
>> At least mingw-w64 added this API recently to runtime for gcc's sake, so
>> implementation of an 'rm -rf' should be pretty easy.
> 
> It has built on Fedora 21 x86_64 mingw64 for both 32-bit and 64-bit targets.
> I am not sure about various other Unices but if the patch gets approved...

Well, if the patch gets approved, what's the plan then?  :-)

See https://www.gnu.org/software/gnulib/manual/html_node/ftw.html:

 "This function is missing on some platforms: Mac OS X 10.3, FreeBSD 5.2.1, NetBSD 3.0, Minix 3.1.8, mingw, MSVC 9, BeOS. "

Note that's problems _not_ fixed by Gnulib.  In reality, there's no real ftw
module in gnulib.  But there _is_ an fts module.

So it seems to me that we should use the fts API instead of ftw.  And then
we'll either need to import the gnulib module, or start out with an
autoconf check.

> 
> 
> On Wed, 17 Dec 2014 18:29:51 +0100, Steve Ellcey wrote:
>> /scratch/sellcey/repos/nightly2/src/binutils-gdb/gdb/compile/compile.c:175:10: error: ignoring return value of 'system', declared with attribute warn_unused_result [-Werror=unused-result]
>> cc1: all warnings being treated as errors
>> make[1]: *** [compile.o] Error 1
> 
> It should get fixed by this patch.
> 
> 
> I have briefly tested (on Linux; on MinGW I have only tested the build) it
> really does delete the directory and its files.
> 
> OK for check-in?


Thanks,
Pedro Alves
Yao Qi Dec. 18, 2014, 5:37 a.m. UTC | #3
Jan Kratochvil <jan.kratochvil@redhat.com> writes:

> On Wed, 17 Dec 2014 18:29:51 +0100, Steve Ellcey wrote:
>> /scratch/sellcey/repos/nightly2/src/binutils-gdb/gdb/compile/compile.c:175:10:
>> error: ignoring return value of 'system', declared with attribute
>> warn_unused_result [-Werror=unused-result]
>> cc1: all warnings being treated as errors
>> make[1]: *** [compile.o] Error 1
>
> It should get fixed by this patch.

This build failure was reported in 
https://sourceware.org/bugzilla/show_bug.cgi?id=17718
Eli Zaretskii Dec. 18, 2014, 5:23 p.m. UTC | #4
> Date: Wed, 17 Dec 2014 22:01:44 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, brobecker@adacore.com, yao@codesourcery.com,
>         gdb-patches@sourceware.org
> 
> > Why not using here instead an implementation using FTW-API?
> 
> Done.
> 
> 
> > At least mingw-w64 added this API recently to runtime for gcc's sake, so
> > implementation of an 'rm -rf' should be pretty easy.
> 
> It has built on Fedora 21 x86_64 mingw64 for both 32-bit and 64-bit targets.
> I am not sure about various other Unices but if the patch gets approved...

There's no ftw in MinGW32.
Jan Kratochvil Dec. 18, 2014, 5:31 p.m. UTC | #5
On Thu, 18 Dec 2014 18:23:11 +0100, Eli Zaretskii wrote:
> > It has built on Fedora 21 x86_64 mingw64 for both 32-bit and 64-bit targets.
> > I am not sure about various other Unices but if the patch gets approved...
> 
> There's no ftw in MinGW32.

32-bit .exe can be build either by MinGW3 (considered by Fedora as obsolete
now)
	http://www.mingw.org/
or by MinGW64 in 32-bit mode
	http://mingw-w64.sourceforge.net/
at least according to:
	https://fedoraproject.org/wiki/Features/Mingw-w64_cross_compiler

From the Fedora point of view MinGW64 32-bit mode seems to be a superset of
MinGW32 so why to care about MinGW32 anymore?  Or what do I miss?


Jan
Eli Zaretskii Dec. 18, 2014, 5:40 p.m. UTC | #6
> Date: Thu, 18 Dec 2014 18:31:03 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: ktietz@redhat.com, sellcey@imgtec.com, brobecker@adacore.com,
>         yao@codesourcery.com, gdb-patches@sourceware.org
> 
> >From the Fedora point of view MinGW64 32-bit mode seems to be a superset of
> MinGW32 so why to care about MinGW32 anymore?  Or what do I miss?

That _I_ use MinGW32?

Seriously, though: from my POV, MinGW64 is not reliable yet to switch
to it completely, their development still breaks the libraries too
often, and there are significant backward-incompatible changes now and
then.  The fact that there's no official releases of MinGW64 doesn't
help.  And no, MinGW64 is not a superset of MinGW32, it's a different
project with an incompatible ABI and subtly incompatible header files.
(Please don't start an argument about that, I'm not really
interested.)

Even if there were no problems with MinGW64, I don't think we should
stop supporting MinGW32 just like that, it is still a live project,
and I, for one, is quite happy with it.  I hope GDB will not drop its
support any time soon.
Jan Kratochvil Dec. 18, 2014, 6:14 p.m. UTC | #7
On Wed, 17 Dec 2014 23:41:53 +0100, Pedro Alves wrote:
> See https://www.gnu.org/software/gnulib/manual/html_node/ftw.html:
> 
>  "This function is missing on some platforms: Mac OS X 10.3, FreeBSD 5.2.1,
>  NetBSD 3.0, Minix 3.1.8, mingw, MSVC 9, BeOS. "

This list does not say much and it is even obsolete, at least "mingw" supports
ftw(); although Eli disagrees in:
	https://sourceware.org/ml/gdb-patches/2014-12/msg00540.html

On MacOS X GDB does not work anyway, FreeBSD latest stable release is 10.1 so
5.2.1 seems irrelevant, NetBSD 6.1.5 vs. 3.0 likewise, for other OS I have no
idea if they are supported by GDB and nobody knows - configure.{host,tgt}
triplets naming are too cryptic to quickly check it by someone not familiar
with that platform and there is no official list of supported platforms
I asked for recently at least in:

Message-ID: <20140817211647.GA17152@host2.jankratochvil.net>
On Sun, 17 Aug 2014 23:16:47 +0200, Jan Kratochvil wrote:
# This is one of the general problems of GDB that one cannot do any change
# affecting platforms which one cannot (or at least not easily enough) test on.
#
# Platform not automatically being tested by Jenkins for any patch submitted to
# Gerrit should be officially unsupported.


> So it seems to me that we should use the fts API instead of ftw.  And then
> we'll either need to import the gnulib module, or start out with an
> autoconf check.

As global maintainer Eli asked for ftw()->fts() (despite I do not see there
valid technical argument in that mail myself) going to implement it; otherwise
I would not see from the platform list at
	https://www.gnu.org/software/gnulib/manual/html_node/ftw.html
if ftw() is really supported on all GDB hosts or not.


Jan
Pedro Alves Dec. 18, 2014, 6:23 p.m. UTC | #8
On 12/18/2014 06:14 PM, Jan Kratochvil wrote:
> On Wed, 17 Dec 2014 23:41:53 +0100, Pedro Alves wrote:
>> See https://www.gnu.org/software/gnulib/manual/html_node/ftw.html:
>>
>>  "This function is missing on some platforms: Mac OS X 10.3, FreeBSD 5.2.1,
>>  NetBSD 3.0, Minix 3.1.8, mingw, MSVC 9, BeOS. "
> 
> This list does not say much and it is even obsolete, at least "mingw" supports
> ftw(); although Eli disagrees in:
> 	https://sourceware.org/ml/gdb-patches/2014-12/msg00540.html

What you miss is that mingw and mingw-w64 are different projects,
lead by different people.

> 
> On MacOS X GDB does not work anyway, 

Huh?

> Message-ID: <20140817211647.GA17152@host2.jankratochvil.net>
> On Sun, 17 Aug 2014 23:16:47 +0200, Jan Kratochvil wrote:
> # This is one of the general problems of GDB that one cannot do any change
> # affecting platforms which one cannot (or at least not easily enough) test on.

Sergio has been working hard on a setting up a buildbot.  But you know that ...

Thanks,
Pedro Alves
Jan Kratochvil Dec. 18, 2014, 6:41 p.m. UTC | #9
On Thu, 18 Dec 2014 19:23:55 +0100, Pedro Alves wrote:
> What you miss is that mingw and mingw-w64 are different projects,
> lead by different people.

https://www.gnu.org/software/gnulib/manual/html_node/Target-Platforms.html#Target-Platforms
does not list "mingw-w64" and it does not list which implementation of "mingw"
they mean.


> > On MacOS X GDB does not work anyway, 
> 
> Huh?

This is from my memories of reading #gdb@freenode that FSF GDB was reporting
fatal errors on trivial programs for anyone asking about OSX there.  I do not
have OSX to test it myself (and I do not remember if I tested GDB there myself
when I had OSX available) so maybe GDB works on OSX now if you say so, OK.


> > Message-ID: <20140817211647.GA17152@host2.jankratochvil.net>
> > On Sun, 17 Aug 2014 23:16:47 +0200, Jan Kratochvil wrote:
> > # This is one of the general problems of GDB that one cannot do any change
> > # affecting platforms which one cannot (or at least not easily enough) test on.
> 
> Sergio has been working hard on a setting up a buildbot.  But you know that ...

(1) That does not work yet (for example it is not public yet).
(2) It is currently unclear whether all the supported OSes (while nobody knows
    the currently valid list of them) will get buildslaves connected to the
    buildbot project.  Some of those OSes are proprietary.

But sure that all may happen one day.  But for now it is irrelevant.


Jan
Pedro Alves Dec. 18, 2014, 7:07 p.m. UTC | #10
Ranting does not help, that's all I'll say.

From the trying to be useful department: you'll find code using
the fts api in GNU coreutils / rm.

Thanks,
Pedro Alves
Kai Tietz Dec. 18, 2014, 8:14 p.m. UTC | #11
----- Ursprüngliche Mail -----
> > Date: Thu, 18 Dec 2014 18:31:03 +0100
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > Cc: ktietz@redhat.com, sellcey@imgtec.com, brobecker@adacore.com,
> >         yao@codesourcery.com, gdb-patches@sourceware.org
> > 
> > >From the Fedora point of view MinGW64 32-bit mode seems to be a superset
> > >of
> > MinGW32 so why to care about MinGW32 anymore?  Or what do I miss?
> 
> That _I_ use MinGW32?

That is actually your problem, isn't it?  The mingw-w64 target support ftw, so why not simply allow it for targets providing it, and other targets can be covered by gnulib?
Anyway gnulib is nothing I am concerned in general.
 
> Seriously, though: from my POV, MinGW64 is not reliable yet to switch
> to it completely, their development still breaks the libraries too
> often, and there are significant backward-incompatible changes now and
> then.  The fact that there's no official releases of MinGW64 doesn't
> help.

Seriously answering this ...

Mingw-w64 has official release.  All bigger distributors have done already a lot of different releases for it.  Just for the interest people, mingw-w64 already has 3 different release-branches.  Current release-version we provide is 3.4.  So, this is for sure no valid point.  We have official releases, and we had actually already more then one ...

What libraries "mingw-w64" breaks often?!?   Could you please go in detail?  I am curious to hear that, as all distributors I know (Fedora, Debian, OpenSuse, ArchLinux, ...) haven't reported this.  Or is that just one thing you have a "gut" feeling about?

>  And no, MinGW64 is not a superset of MinGW32, it's a different
> project with an incompatible ABI and subtly incompatible header files.
> (Please don't start an argument about that, I'm not really
> interested.)

True, MinGW.org and mingw-w64 are two different ventures with differences in feature-sets.  Actually, mingw-w64 supports most (if not all) what MinGW.org supports plus a bit more (Unicode-entry support, 64-bit and 32-bit, partial ARM 32 support, supporting also other compilers then just gcc (but of course gcc is our main-target, full C99-math, working complex-math support, etc).

Just one point here I got curious about. What you mean by ABI?  The ABI of mingw-targets is the same for all targets using gcc.  So what ABI-differences you are talking about?!?

> Even if there were no problems with MinGW64, I don't think we should
> stop supporting MinGW32 just like that, it is still a live project,
> and I, for one, is quite happy with it.  I hope GDB will not drop its
> support any time soon.

No problem about this, but why blocking things not related to MinGW.org?

Kai
Eli Zaretskii Dec. 18, 2014, 8:26 p.m. UTC | #12
> Date: Thu, 18 Dec 2014 19:14:32 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Kai Tietz <ktietz@redhat.com>, Steve Ellcey <sellcey@imgtec.com>,
>         Eli Zaretskii <eliz@gnu.org>, brobecker@adacore.com,
>         yao@codesourcery.com, gdb-patches@sourceware.org
> 
> As global maintainer Eli asked for ftw()->fts()

Actually, I didn't.  (But I don't object to that, either.)  I just
said that MinGW32 doesn't have ftw.
Eli Zaretskii Dec. 18, 2014, 8:47 p.m. UTC | #13
> Date: Thu, 18 Dec 2014 15:14:48 -0500 (EST)
> From: Kai Tietz <ktietz@redhat.com>
> Cc: Jan Kratochvil <jan.kratochvil@redhat.com>, sellcey@imgtec.com,
>         brobecker@adacore.com, yao@codesourcery.com,
>         gdb-patches@sourceware.org
> 
> > > >From the Fedora point of view MinGW64 32-bit mode seems to be a superset
> > > >of
> > > MinGW32 so why to care about MinGW32 anymore?  Or what do I miss?
> > 
> > That _I_ use MinGW32?
> 
> That is actually your problem, isn't it?

I don't see it as a problem, necessarily.

> The mingw-w64 target support ftw, so why not simply allow it for targets providing it, and other targets can be covered by gnulib?

Sure, why not?  I wasn't objecting to that, I just provided
information, since Jan seemed to think ftw is available everywhere.

> What libraries "mingw-w64" breaks often?!?  Could you please go in detail?  I am curious to hear that, as all distributors I know (Fedora, Debian, OpenSuse, ArchLinux, ...) haven't reported this.  Or is that just one thing you have a "gut" feeling about?

The latest that I saw is this:

  http://lists.gnu.org/archive/html/bug-gnulib/2014-12/msg00186.html

And I remember a few more lately.

But look, I don't want to argue, I specifically said that.  Jan asked
why not forget about MinGW32, and I gave _my_ reasons.  You don't have
to agree, and we don't have to convince each other.  My only request
is that GDB doesn't drop MinGW32 support.

> Just one point here I got curious about. What you mean by ABI?  The ABI of mingw-targets is the same for all targets using gcc.  So what ABI-differences you are talking about?!?

Exception handling across DLLs is one difference I know of.

> > Even if there were no problems with MinGW64, I don't think we should
> > stop supporting MinGW32 just like that, it is still a live project,
> > and I, for one, is quite happy with it.  I hope GDB will not drop its
> > support any time soon.
> 
> No problem about this, but why blocking things not related to MinGW.org?

I didn't, it's a misunderstanding.  Sorry if I caused it.
Kai Tietz Dec. 18, 2014, 8:54 p.m. UTC | #14
2014-12-18 21:47 GMT+01:00 Eli Zaretskii <eliz@gnu.org>:
>> Date: Thu, 18 Dec 2014 15:14:48 -0500 (EST)
>> From: Kai Tietz <ktietz@redhat.com>
>> Cc: Jan Kratochvil <jan.kratochvil@redhat.com>, sellcey@imgtec.com,
>>         brobecker@adacore.com, yao@codesourcery.com,
>>         gdb-patches@sourceware.org
>>
>> > > >From the Fedora point of view MinGW64 32-bit mode seems to be a superset
>> > > >of
>> > > MinGW32 so why to care about MinGW32 anymore?  Or what do I miss?
>> >
>> > That _I_ use MinGW32?
>>
>> That is actually your problem, isn't it?
>
> I don't see it as a problem, necessarily.
>
>> The mingw-w64 target support ftw, so why not simply allow it for targets providing it, and other targets can be covered by gnulib?
>
> Sure, why not?  I wasn't objecting to that, I just provided
> information, since Jan seemed to think ftw is available everywhere.
>
>> What libraries "mingw-w64" breaks often?!?  Could you please go in detail?  I am curious to hear that, as all distributors I know (Fedora, Debian, OpenSuse, ArchLinux, ...) haven't reported this.  Or is that just one thing you have a "gut" feeling about?
>
> The latest that I saw is this:
>
>   http://lists.gnu.org/archive/html/bug-gnulib/2014-12/msg00186.html
>
> And I remember a few more lately.

Hmm, this isn't something we got reported at all.  As you are using
pthread-library based toolchain (this is a build-option, and not a
mandatory mingw-w64 thing at all) I don't see that this is a mingw-w64
venture issue at all.  Btw we (on mingw-w64) strongly recomment to use
winpthread instead, as it solves some quirks existing with other
pthread-implementations available for Win32 ... anyway later is
off-topic.

> But look, I don't want to argue, I specifically said that.  Jan asked
> why not forget about MinGW32, and I gave _my_ reasons.  You don't have
> to agree, and we don't have to convince each other.  My only request
> is that GDB doesn't drop MinGW32 support.

Sure, no problem about that.  Nevertheless I have a problem if you try
to tell people things not true.

>> Just one point here I got curious about. What you mean by ABI?  The ABI of mingw-targets is the same for all targets using gcc.  So what ABI-differences you are talking about?!?
>
> Exception handling across DLLs is one difference I know of.

This is again a build-option of gcc, and has nothing directly to do
with mingw-w64.  For example, you can find for 32-bit dw2 based
exception-handling toolchain provided by mingw-builds projects (and
some others providing for 32-bit dw2 too).  For 64-bit there is SEH,
which replaces dw2 completely.

>> > Even if there were no problems with MinGW64, I don't think we should
>> > stop supporting MinGW32 just like that, it is still a live project,
>> > and I, for one, is quite happy with it.  I hope GDB will not drop its
>> > support any time soon.
>>
>> No problem about this, but why blocking things not related to MinGW.org?
>
> I didn't, it's a misunderstanding.  Sorry if I caused it.

Ok, np.

Kai
Sergio Durigan Junior Dec. 18, 2014, 8:56 p.m. UTC | #15
On Wednesday, December 17 2014, Jan Kratochvil wrote:

> On Wed, 17 Dec 2014 18:29:51 +0100, Steve Ellcey wrote:
>> /scratch/sellcey/repos/nightly2/src/binutils-gdb/gdb/compile/compile.c:175:10: error: ignoring return value of 'system', declared with attribute warn_unused_result [-Werror=unused-result]
>> cc1: all warnings being treated as errors
>> make[1]: *** [compile.o] Error 1
>
> It should get fixed by this patch.

https://sourceware.org/bugzilla/show_bug.cgi?id=17718 has been fixed
about this as well, FYI.

> I have briefly tested (on Linux; on MinGW I have only tested the build) it
> really does delete the directory and its files.
>
> OK for check-in?
>
>
> Thanks,
> Jan
> gdb/ChangeLog
> 2014-12-17  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
> 	* compile/compile.c: Include ftw.h.
> 	(do_rmdir_fn): New function.
> 	(do_rmdir): Call nftw with it.
>
> diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
> index 414fc35..7dbc819 100644
> --- a/gdb/compile/compile.c
> +++ b/gdb/compile/compile.c
> @@ -37,6 +37,7 @@
>  #include "filestuff.h"
>  #include "target.h"
>  #include "osabi.h"
> +#include <ftw.h>
>  
>  
>  
> @@ -162,17 +163,42 @@ compile_code_command (char *arg, int from_tty)
>    do_cleanups (cleanup);
>  }
>  
> +/* Helper for do_rmdir.  */
> +
> +static int
> +do_rmdir_fn (const char *fpath, const struct stat *sb, int typeflag,
> +	     struct FTW *ftwbuf)
> +{
> +  switch (typeflag)
> +    {
> +    case FTW_DP:
> +      if (rmdir (fpath) != 0)
> +	warning (_("Cannot remove 'compile' command directory \"%s\": %s"),
> +		 fpath, safe_strerror (errno));
> +      break;
> +    case FTW_F:
> +      if (unlink (fpath) != 0)
> +	warning (_("Cannot remove 'compile' command file \"%s\": %s"),
> +		 fpath, safe_strerror (errno));
> +      break;
> +    default:
> +      warning (_("Unknown 'typeflag' %d for 'compile' command file \"%s\"."),
> +	       typeflag, fpath);
> +    }
> +
> +  /* Continue the walk.  */
> +  return 0;
> +}
> +
>  /* A cleanup function to remove a directory and all its contents.  */
>  
>  static void
>  do_rmdir (void *arg)
>  {
>    const char *dir = arg;
> -  char *zap;
> -  
> +
>    gdb_assert (strncmp (dir, TMP_PREFIX, strlen (TMP_PREFIX)) == 0);
> -  zap = concat ("rm -rf ", dir, (char *) NULL);
> -  system (zap);
> +  nftw (dir, do_rmdir_fn, 10, FTW_DEPTH | FTW_MOUNT | FTW_PHYS);
>  }
>  
>  /* Return the name of the temporary directory to use for .o files, and
Jan Kratochvil Dec. 18, 2014, 9:04 p.m. UTC | #16
On Thu, 18 Dec 2014 20:07:39 +0100, Pedro Alves wrote:
> Ranting does not help, that's all I'll say.

I am just trying to show (for many years) despite contributor's best effort
(*) the current GDB project's maintenance requires multiple reimplementations
of patches until everyone approves the patch as all the requirements are
nowhere written and the maintainers only show their requirements one after
another after each of the patch variant is submitted.  This leads to the slow
development of GDB despite a lot of time invested into it.

(*) I feel at least as good as a median newcomer to GDB.


Jan
Eli Zaretskii Dec. 18, 2014, 9:11 p.m. UTC | #17
> Date: Thu, 18 Dec 2014 22:04:45 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Kai Tietz <ktietz@redhat.com>, Steve Ellcey <sellcey@imgtec.com>,
>         Eli Zaretskii <eliz@gnu.org>, brobecker@adacore.com,
>         yao@codesourcery.com, gdb-patches@sourceware.org
> 
> I am just trying to show (for many years) despite contributor's best effort
> (*) the current GDB project's maintenance requires multiple reimplementations
> of patches until everyone approves the patch as all the requirements are
> nowhere written and the maintainers only show their requirements one after
> another after each of the patch variant is submitted.  This leads to the slow
> development of GDB despite a lot of time invested into it.

If this is a problem, how about writing down the requirements?
Jan Kratochvil Dec. 18, 2014, 9:23 p.m. UTC | #18
On Thu, 18 Dec 2014 22:11:16 +0100, Eli Zaretskii wrote:
> If this is a problem, how about writing down the requirements?

I was more general above.  In this specific case it would mean describing APIs
of all the supported OSes.  I do not think it is possible.

This can be implemented by a buildbot for all supported OSes.

A prerequisite for that is to settle down a list of really supported OSes.


Jan
Sergio Durigan Junior Dec. 18, 2014, 10:17 p.m. UTC | #19
On Thursday, December 18 2014, Jan Kratochvil wrote:

> This can be implemented by a buildbot for all supported OSes.

As Pedro already mentioned, I am working towards setting up a buildbot
for GDB.  I will write more details soon at the gdb@ list; I don't want
to hijack this thread.
Pedro Alves Dec. 18, 2014, 11:06 p.m. UTC | #20
On 12/18/2014 09:23 PM, Jan Kratochvil wrote:
> On Thu, 18 Dec 2014 22:11:16 +0100, Eli Zaretskii wrote:
>> If this is a problem, how about writing down the requirements?
> 
> I was more general above.  In this specific case it would mean describing APIs
> of all the supported OSes.  I do not think it is possible.

- gnulib seems to be good at describe portability issues.
- We use gnulib as low level host portability layer

Always check gnulib.

> This can be implemented by a buildbot for all supported OSes.

Yes, we're getting there.  We need a working master buildbot setup first.
Ideally, that'd be setup in sourceware.org itself, and then we'd
use machines in the gcc compile farm for the build slaves.  But,
as you know, we're actively working on this.

> A prerequisite for that is to settle down a list of really supported OSes.

Not necessarily.  I see it the other way around.   Once buildbot is up
and running for a while, we can require that either people setup
buildbot slaves for their favorite host OSs, or the OSs will be considered
unsupported.

Meanwhile, we should start with a list of OSes gdb supposedly
builds for, per gdb/configure.host.  I had built this table
a while ago:

 https://sourceware.org/ml/gdb/2014-10/msg00026.html

It's obviously quite bare, missing OS version info, for
example.  I had extended it since with GDBserver info,
but left it to neglect on my hard drive since.  But it's a start.
I've now put it up on the wiki, here:

  https://sourceware.org/gdb/wiki/Systems

(linked from the home page).

Maybe once this has more info, it should be migrated to
MAINTAINERS or README in the sources, but while in "collect info"
phase at least, wiki is easier.  Everyone, please do feel free
to extend|massage|whatever this page.

Thanks,
Pedro Alves
Pedro Alves Dec. 18, 2014, 11:47 p.m. UTC | #21
On 12/18/2014 08:14 PM, Kai Tietz wrote:
> ----- Ursprüngliche Mail -----
>>> Date: Thu, 18 Dec 2014 18:31:03 +0100
>>> From: Jan Kratochvil <jan.kratochvil@redhat.com>
>>> Cc: ktietz@redhat.com, sellcey@imgtec.com, brobecker@adacore.com,
>>>         yao@codesourcery.com, gdb-patches@sourceware.org
>>>
>>> >From the Fedora point of view MinGW64 32-bit mode seems to be a superset
>>>> of
>>> MinGW32 so why to care about MinGW32 anymore?  Or what do I miss?
>>
>> That _I_ use MinGW32?
> 
> That is actually your problem, isn't it?  The mingw-w64 target support ftw, so why
> not simply allow it for targets providing it, and other targets can be covered by gnulib?
> Anyway gnulib is nothing I am concerned in general.

Let me try to make this clear.

gnulib does not provide an ftw replacement.  And AFAIK, that's on
purpose.  Note that ftw has been marked obsolete in POSIX.1-2008.
Per POSIX, applications "should" be using nftw/nftw64 instead.

For our purposes, both APIs are just as good (*).  It's just
that ftw has System V roots, while fts has BSD roots.

In practice, programs that use fts instead are just, if not
more portable.

But, as I mentioned, gnulib provides an fts replacement
for systems that don't have it.  So if we use fts instead, we're
good to go everywhere.

(*) - see ftw vs fts limitations here:
   http://lists.gnu.org/archive/html/bug-gnu-utils/2003-02/msg00224.html

Thanks,
Pedro Alves
Jan Kratochvil Dec. 19, 2014, 12:11 a.m. UTC | #22
On Fri, 19 Dec 2014 00:06:10 +0100, Pedro Alves wrote:
> Always check gnulib.

As I have described I do not find the obsolete list of OSes too useful, it is
rather waiting for the buildbot.


> > A prerequisite for that is to settle down a list of really supported OSes.
> 
> Not necessarily.  I see it the other way around.   Once buildbot is up
> and running for a while, we can require that either people setup
> buildbot slaves for their favorite host OSs, or the OSs will be considered
> unsupported.

I do not expect this can ever pass GDB global maintainers.


> I've now put it up on the wiki, here:
> 
>   https://sourceware.org/gdb/wiki/Systems

Yes, that is really useful, waiting for it for so many years, thanks.


Jan
Eli Zaretskii Dec. 19, 2014, 8:23 a.m. UTC | #23
> Date: Thu, 18 Dec 2014 22:23:53 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: palves@redhat.com, ktietz@redhat.com, sellcey@imgtec.com,
>         brobecker@adacore.com, yao@codesourcery.com,
>         gdb-patches@sourceware.org
> 
> On Thu, 18 Dec 2014 22:11:16 +0100, Eli Zaretskii wrote:
> > If this is a problem, how about writing down the requirements?
> 
> I was more general above.

So was I.  If you think our requirements are not detailed enough, with
the result of complicating the patch submission process and the job of
contributors, I think we should try to put more details into the
existing documentation.

Of course, some requirements that come up during the review cannot
possibly be codified, because they are general principles or minor
preferences that are too many to write down.  I don't think we can
avoid that, but we could try.  E.g., after each such issue comes up,
we could ask ourselves whether it is worth adding to the
documentation.
Kai Tietz Dec. 19, 2014, 9:28 a.m. UTC | #24
----- Ursprüngliche Mail -----
Just for the records.  We provide on mingw-w64 both APIs.  The nftw, and the old ftw.

Kai
Pedro Alves Dec. 19, 2014, 10:48 a.m. UTC | #25
On 12/19/2014 12:11 AM, Jan Kratochvil wrote:
> On Fri, 19 Dec 2014 00:06:10 +0100, Pedro Alves wrote:
>> Always check gnulib.
> 
> As I have described I do not find the obsolete list of OSes too useful, it is
> rather waiting for the buildbot.

Not just the list, but the code.

>> I've now put it up on the wiki, here:
>>
>>   https://sourceware.org/gdb/wiki/Systems
> 
> Yes, that is really useful, waiting for it for so many years, thanks.

Note that all I did was sed configure.host, and massage the result
a bit.

Thanks,
Pedro Alves
Joel Brobecker Dec. 19, 2014, 12:59 p.m. UTC | #26
> > Huh?
> 
> This is from my memories of reading #gdb@freenode that FSF GDB was reporting
> fatal errors on trivial programs for anyone asking about OSX there.  I do not
> have OSX to test it myself (and I do not remember if I tested GDB there myself
> when I had OSX available) so maybe GDB works on OSX now if you say so, OK.

OSX is not an easy OS for debuggers, and I am not surprised that people
might have some issues with it. But, FWIW, I confirm GDB works pretty
well for us. And we also tested it on the latest OSX release, and for
the first time ever, nothing appears to have regressed.

Not sure what people's issues are on OS, and I won't hijack this thread
to discuss this.
Jan Kratochvil Dec. 19, 2014, 7:24 p.m. UTC | #27
On Fri, 19 Dec 2014 00:47:27 +0100, Pedro Alves wrote:
> For our purposes, both APIs are just as good (*).  It's just
> that ftw has System V roots, while fts has BSD roots.
> 
> In practice, programs that use fts instead are just, if not
> more portable.
> 
> But, as I mentioned, gnulib provides an fts replacement
> for systems that don't have it.  So if we use fts instead, we're
> good to go everywhere.

Linux fts man page
	http://man7.org/linux/man-pages/man3/fts.3.html
says
	BUGS
	All of the APIs described in this man page are not safe when
	compiling a program using the LFS APIs (e.g., when compiling with
	-D_FILE_OFFSET_BITS=64).
and _FILE_OFFSET_BITS=64 happens on 32-bit hosts and --enable-64-bit-bfd.

Contrary to it gnulib fts documentation
	https://www.gnu.org/software/gnulib/manual/html_node/fts_005fread.html
says
	On platforms where off_t is a 32-bit type, this function may not
	correctly report the size of files or block devices larger than 2 GB
	and may not work correctly on huge directories larger than 2 GB. Also,
	on platforms where ino_t is a 32-bit type, this function may report
	inode numbers incorrectly. The fix is to use the AC_SYS_LARGEFILE
	macro (only on Mac OS X systems).

This would suggest as if GDB should be compatible with 32-bit hosts and
--enable-64-bit-bfd GDB should use gnulib fts even on Linux; but I guess the
gnulib fts replacement would not be active on Linux.

The temporary files being deleted are never larger than 2GB nor their size is
read.  So I hope nobody is going address these issues (I would add 32-bit
hosts are not relevant anymore anyway).


Jan
Jan Kratochvil Dec. 19, 2014, 8:54 p.m. UTC | #28
On Fri, 19 Dec 2014 20:24:57 +0100, Jan Kratochvil wrote:
> This would suggest as if GDB should be compatible with 32-bit hosts and
> --enable-64-bit-bfd GDB should use gnulib fts even on Linux; but I guess the
> gnulib fts replacement would not be active on Linux.

It would be active - so that this mail should be resolved by it:

gnulib/modules/fts:
dnl Use this version of fts unconditionally, since the GNU libc and
dnl NetBSD versions have bugs and/or unnecessary limitations.
AC_LIBOBJ([fts])


Jan
Joel Brobecker Dec. 20, 2014, 12:26 p.m. UTC | #29
> The temporary files being deleted are never larger than 2GB nor their
> size is read.  So I hope nobody is going address these issues (I would
> add 32-bit hosts are not relevant anymore anyway).

Yes, they are still very relevant. A lot of programs do not want
to switch over to 64bit; and I think the x32 ABI might also have
stemmed from that.
diff mbox

Patch

diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 414fc35..7dbc819 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -37,6 +37,7 @@ 
 #include "filestuff.h"
 #include "target.h"
 #include "osabi.h"
+#include <ftw.h>
 
 
 
@@ -162,17 +163,42 @@  compile_code_command (char *arg, int from_tty)
   do_cleanups (cleanup);
 }
 
+/* Helper for do_rmdir.  */
+
+static int
+do_rmdir_fn (const char *fpath, const struct stat *sb, int typeflag,
+	     struct FTW *ftwbuf)
+{
+  switch (typeflag)
+    {
+    case FTW_DP:
+      if (rmdir (fpath) != 0)
+	warning (_("Cannot remove 'compile' command directory \"%s\": %s"),
+		 fpath, safe_strerror (errno));
+      break;
+    case FTW_F:
+      if (unlink (fpath) != 0)
+	warning (_("Cannot remove 'compile' command file \"%s\": %s"),
+		 fpath, safe_strerror (errno));
+      break;
+    default:
+      warning (_("Unknown 'typeflag' %d for 'compile' command file \"%s\"."),
+	       typeflag, fpath);
+    }
+
+  /* Continue the walk.  */
+  return 0;
+}
+
 /* A cleanup function to remove a directory and all its contents.  */
 
 static void
 do_rmdir (void *arg)
 {
   const char *dir = arg;
-  char *zap;
-  
+
   gdb_assert (strncmp (dir, TMP_PREFIX, strlen (TMP_PREFIX)) == 0);
-  zap = concat ("rm -rf ", dir, (char *) NULL);
-  system (zap);
+  nftw (dir, do_rmdir_fn, 10, FTW_DEPTH | FTW_MOUNT | FTW_PHYS);
 }
 
 /* Return the name of the temporary directory to use for .o files, and