diff mbox

ARC: Add GNU glob to ARC defconfigs

Message ID 1423053291-31754-1-git-send-email-Anton.Kolesov@synopsys.com
State Accepted
Commit 89b63496e88c31c2714e42656212078388718b78
Headers show

Commit Message

Anton Kolesov Feb. 4, 2015, 12:34 p.m. UTC
GNU glob is required by make.

Signed-off-by: Anton Kolesov <Anton.Kolesov@synopsys.com>
Cc: Vineet Gupta <Vineet.Gupta1@synopsys.com>
---
 extra/Configs/defconfigs/arc/arcv2_defconfig | 1 +
 extra/Configs/defconfigs/arc/defconfig       | 1 +
 2 files changed, 2 insertions(+)

Comments

Rich Felker Feb. 4, 2015, 2:26 p.m. UTC | #1
On Wed, Feb 04, 2015 at 03:34:51PM +0300, Anton Kolesov wrote:
> GNU glob is required by make.

Could you clarify this claim? GNU make works fine on musl and we don't
have GNU glob. If there's some incompatibility with the uclibc glob
implementation it could probably be fixed by a tiny improvement to
uclibc rather than just replacing yet another part of uclibc with
glibc code by default...

Rich
Anton Kolesov Feb. 4, 2015, 3:58 p.m. UTC | #2
> -----Original Message-----

> From: uClibc [mailto:uclibc-bounces@uclibc.org] On Behalf Of Rich Felker

> Sent: 04 February 2015 17:26

> To: uclibc@uclibc.org

> Subject: Re: [PATCH] ARC: Add GNU glob to ARC defconfigs

> 

> On Wed, Feb 04, 2015 at 03:34:51PM +0300, Anton Kolesov wrote:

> > GNU glob is required by make.

> 

> Could you clarify this claim? GNU make works fine on musl and we don't

> have GNU glob. If there's some incompatibility with the uclibc glob

> implementation it could probably be fixed by a tiny improvement to

> uclibc rather than just replacing yet another part of uclibc with

> glibc code by default...


I had a compilation error when building make with uClibc without GNU glob:
-------->8--------
dir.c: In function 'dir_setup_glob':
dir.c:1225:5: error: 'glob_t' has no member named 'gl_opendir'
   gl->gl_opendir = open_dirstream;
     ^
dir.c:1226:5: error: 'glob_t' has no member named 'gl_readdir'
   gl->gl_readdir = read_dirstream;
     ^
dir.c:1227:5: error: 'glob_t' has no member named 'gl_closedir'
   gl->gl_closedir = ansi_free;
     ^
dir.c:1228:5: error: 'glob_t' has no member named 'gl_stat'
   gl->gl_stat = local_stat;
     ^
-------->8--------

Enabling GNU glob resolved my troubles. 

Now I've looked into more details of error cause and what happens is that those fields (gl_opendir, gl_readdir, gl_closedir, gl_stat) in glob_t in uclibc/include/glob.h are surrounded by "#ifdef __UCLIBC_HAS_GNU_GLOB__". "Make" might be built without GNU glob on target system because it contains its own copy in source tree and uses it when it detects that target system doesn’t have GNU glob. However the way it checks for GNU glob is not compatible with uClibc, because make/configure checks for " _GNU_GLOB_INTERFACE_VERSION == 1" as an indicator that GNU glob is present, but uclibc/gnu-versions.h defines _GNU_GLOB_INTERFACE_VERSION as 1 regardless of whether GNU glob is present in configuration. Thus make/configure decides that GNU glob is present when it is not, which causes further compilation error.

I also has been told that arcv2_defconfig file is not yet present in upstream uClibc repository, so only "defconfig" is applicable.

Anton

> 

> Rich

> _______________________________________________

> uClibc mailing list

> uClibc@uclibc.org

> http://lists.busybox.net/mailman/listinfo/uclibc
Rich Felker Feb. 5, 2015, 7:20 p.m. UTC | #3
On Wed, Feb 04, 2015 at 03:58:41PM +0000, Anton Kolesov wrote:
> Now I've looked into more details of error cause and what happens is
> that those fields (gl_opendir, gl_readdir, gl_closedir, gl_stat) in
> glob_t in uclibc/include/glob.h are surrounded by "#ifdef
> __UCLIBC_HAS_GNU_GLOB__". "Make" might be built without GNU glob on
> target system because it contains its own copy in source tree and
> uses it when it detects that target system doesn’t have GNU glob.
> However the way it checks for GNU glob is not compatible with
> uClibc, because make/configure checks for "
> _GNU_GLOB_INTERFACE_VERSION == 1" as an indicator that GNU glob is
> present, but uclibc/gnu-versions.h defines
> _GNU_GLOB_INTERFACE_VERSION as 1 regardless of whether GNU glob is
> present in configuration. Thus make/configure decides that GNU glob
> is present when it is not, which causes further compilation error.

I think the definition of _GNU_GLOB_INTERFACE_VERSION just needs to be
moved inside the #ifdef __UCLIBC_HAS_GNU_GLOB__. uclibc should not be
reporting that it has GNU glob when it was built without it.

Rich
Bernhard Reutner-Fischer Feb. 5, 2015, 11:05 p.m. UTC | #4
On February 5, 2015 8:20:57 PM GMT+01:00, Rich Felker <dalias@libc.org> wrote:
>On Wed, Feb 04, 2015 at 03:58:41PM +0000, Anton Kolesov wrote:
>> Now I've looked into more details of error cause and what happens is
>> that those fields (gl_opendir, gl_readdir, gl_closedir, gl_stat) in
>> glob_t in uclibc/include/glob.h are surrounded by "#ifdef
>> __UCLIBC_HAS_GNU_GLOB__". "Make" might be built without GNU glob on
>> target system because it contains its own copy in source tree and
>> uses it when it detects that target system doesn’t have GNU glob.
>> However the way it checks for GNU glob is not compatible with
>> uClibc, because make/configure checks for "
>> _GNU_GLOB_INTERFACE_VERSION == 1" as an indicator that GNU glob is
>> present, but uclibc/gnu-versions.h defines
>> _GNU_GLOB_INTERFACE_VERSION as 1 regardless of whether GNU glob is
>> present in configuration. Thus make/configure decides that GNU glob
>> is present when it is not, which causes further compilation error.
>
>I think the definition of _GNU_GLOB_INTERFACE_VERSION just needs to be
>moved inside the #ifdef __UCLIBC_HAS_GNU_GLOB__. uclibc should not be
>reporting that it has GNU glob when it was built without it.

Right.
Vineet Gupta Feb. 7, 2015, 5:31 a.m. UTC | #5
On Friday 06 February 2015 04:35 AM, Bernhard Reutner-Fischer wrote:
> On February 5, 2015 8:20:57 PM GMT+01:00, Rich Felker <dalias@libc.org> wrote:
>> On Wed, Feb 04, 2015 at 03:58:41PM +0000, Anton Kolesov wrote:
>>> Now I've looked into more details of error cause and what happens is
>>> that those fields (gl_opendir, gl_readdir, gl_closedir, gl_stat) in
>>> glob_t in uclibc/include/glob.h are surrounded by "#ifdef
>>> __UCLIBC_HAS_GNU_GLOB__". "Make" might be built without GNU glob on
>>> target system because it contains its own copy in source tree and
>>> uses it when it detects that target system doesn’t have GNU glob.
>>> However the way it checks for GNU glob is not compatible with
>>> uClibc, because make/configure checks for "
>>> _GNU_GLOB_INTERFACE_VERSION == 1" as an indicator that GNU glob is
>>> present, but uclibc/gnu-versions.h defines
>>> _GNU_GLOB_INTERFACE_VERSION as 1 regardless of whether GNU glob is
>>> present in configuration. Thus make/configure decides that GNU glob
>>> is present when it is not, which causes further compilation error.
>> I think the definition of _GNU_GLOB_INTERFACE_VERSION just needs to be
>> moved inside the #ifdef __UCLIBC_HAS_GNU_GLOB__. uclibc should not be
>> reporting that it has GNU glob when it was built without it.
> Right.

Anton, are you going to spin a patch to that effect ?

-Vineet
Anton Kolesov Feb. 9, 2015, 10:23 a.m. UTC | #6
> Anton, are you going to spin a patch to that effect ?

Yes.
Anton

> 
> -Vineet
Anton Kolesov Feb. 9, 2015, 2:37 p.m. UTC | #7
> > Anton, are you going to spin a patch to that effect ?
> 
> Yes.
> Anton

It didn't worked quite well - glob implementation shipped with make sources makes some assumptions WRT of presence of __mempcpy, __alloca and __stat, which are not true for uClibc. I was able to avoid the problem by changing make/glob/glob.c: by adding there several #ifdef __UCLIBC__ (similar to the one found in libpthread/nptl_db/thread_dbP.h at line 32). But that means that patching of uclibc/include/gnu-versions.h is not sufficient on its own.

> 
> >
> > -Vineet
> _______________________________________________
> uClibc mailing list
> uClibc@uclibc.org
> http://lists.busybox.net/mailman/listinfo/uclibc
Rich Felker Feb. 13, 2015, 2:04 a.m. UTC | #8
On Mon, Feb 09, 2015 at 02:37:11PM +0000, Anton Kolesov wrote:
> > > Anton, are you going to spin a patch to that effect ?
> > 
> > Yes.
> > Anton
> 
> It didn't worked quite well - glob implementation shipped with make
> sources makes some assumptions WRT of presence of __mempcpy,
> __alloca and __stat, which are not true for uClibc. I was able to
> avoid the problem by changing make/glob/glob.c: by adding there
> several #ifdef __UCLIBC__ (similar to the one found in
> libpthread/nptl_db/thread_dbP.h at line 32). But that means that
> patching of uclibc/include/gnu-versions.h is not sufficient on its
> own.

This does not sound plausible. GNU make works on dozens if not
hundreds of platforms which do not have the above glibc nonsense
exported. Or is it using this stuff based on uclibc's defining of the
__GLIBC__ macro?

Rich
Anton Kolesov Feb. 13, 2015, 11:26 a.m. UTC | #9
> On Mon, Feb 09, 2015 at 02:37:11PM +0000, Anton Kolesov wrote:
> > > > Anton, are you going to spin a patch to that effect ?
> > >
> > > Yes.
> > > Anton
> >
> > It didn't worked quite well - glob implementation shipped with make
> > sources makes some assumptions WRT of presence of __mempcpy,
> > __alloca and __stat, which are not true for uClibc. I was able to
> > avoid the problem by changing make/glob/glob.c: by adding there
> > several #ifdef __UCLIBC__ (similar to the one found in
> > libpthread/nptl_db/thread_dbP.h at line 32). But that means that
> > patching of uclibc/include/gnu-versions.h is not sufficient on its
> > own.
> 
> This does not sound plausible. GNU make works on dozens if not
> hundreds of platforms which do not have the above glibc nonsense
> exported. Or is it using this stuff based on uclibc's defining of the
> __GLIBC__ macro?
> 

1. Error for mempcpy depends on (__GLIBC__ - 0 == 2 && __GLIBC_MINOR__ >= 1) (http://git.savannah.gnu.org/cgit/make.git/tree/glob/glob.c#n179). Those conditions are met for uClibc (http://git.uclibc.org/uClibc/tree/include/features.h#n395), as a result make's glob  undefines mempcpy and sets it to __mempcpy. Unfortunately __mempcpy doesn't get resolved by linker to anything.

2. For alloca (http://git.savannah.gnu.org/cgit/make.git/tree/glob/glob.c#n211) we need "#define __alloca alloca" to be done, but that doesn't happen because condition (!defined __GNU_LIBRARY__) is false.

3. For stat it is the same: condition is "#ifndef __GNU_LIBRARY__" (http://git.savannah.gnu.org/cgit/make.git/tree/glob/glob.c#n234) which is false for uClibc, and __stat is left undefined, but is used by glob.c.

I'm not really a libc developer so I would refrain from deciding what is right and what is wrong with those defines, I just need "make" to run uClibc testsuite and Vineet greenlighted for me enabling gnu glob in default config for ARC.

> Rich
Bernhard Reutner-Fischer Feb. 13, 2015, 8:02 p.m. UTC | #10
On February 13, 2015 12:26:07 PM GMT+01:00, Anton Kolesov <Anton.Kolesov@synopsys.com> wrote:
>> On Mon, Feb 09, 2015 at 02:37:11PM +0000, Anton Kolesov wrote:
>> > > > Anton, are you going to spin a patch to that effect ?
>> > >
>> > > Yes.
>> > > Anton
>> >
>> > It didn't worked quite well - glob implementation shipped with make
>> > sources makes some assumptions WRT of presence of __mempcpy,
>> > __alloca and __stat, which are not true for uClibc. I was able to
>> > avoid the problem by changing make/glob/glob.c: by adding there
>> > several #ifdef __UCLIBC__ (similar to the one found in
>> > libpthread/nptl_db/thread_dbP.h at line 32). But that means that
>> > patching of uclibc/include/gnu-versions.h is not sufficient on its
>> > own.
>> 
>> This does not sound plausible. GNU make works on dozens if not
>> hundreds of platforms which do not have the above glibc nonsense
>> exported. Or is it using this stuff based on uclibc's defining of the
>> __GLIBC__ macro?
>> 
>
>1. Error for mempcpy depends on (__GLIBC__ - 0 == 2 && __GLIBC_MINOR__
>>= 1)
>(http://git.savannah.gnu.org/cgit/make.git/tree/glob/glob.c#n179).
>Those conditions are met for uClibc
>(http://git.uclibc.org/uClibc/tree/include/features.h#n395), as a
>result make's glob  undefines mempcpy and sets it to __mempcpy.
>Unfortunately __mempcpy doesn't get resolved by linker to anything.
>
>2. For alloca
>(http://git.savannah.gnu.org/cgit/make.git/tree/glob/glob.c#n211) we
>need "#define __alloca alloca" to be done, but that doesn't happen
>because condition (!defined __GNU_LIBRARY__) is false.
>
>3. For stat it is the same: condition is "#ifndef __GNU_LIBRARY__"
>(http://git.savannah.gnu.org/cgit/make.git/tree/glob/glob.c#n234) which
>is false for uClibc, and __stat is left undefined, but is used by
>glob.c.
>
>I'm not really a libc developer so I would refrain from deciding what
>is right and what is wrong with those defines, I just need "make" to
>run uClibc testsuite and Vineet greenlighted for me enabling gnu glob
>in default config for ARC.

So we will just enable the GNU glob for ARC as a quick measure and I already planned to look into removing our cheat after the release, thus we can easily disable GNU glob for the defconfig afterwards.

Thanks,
Vineet Gupta Feb. 18, 2015, 6:01 a.m. UTC | #11
On Saturday 14 February 2015 01:32 AM, Bernhard Reutner-Fischer wrote:
>> I'm not really a libc developer so I would refrain from deciding what
>> >is right and what is wrong with those defines, I just need "make" to
>> >run uClibc testsuite and Vineet greenlighted for me enabling gnu glob
>> >in default config for ARC.
> So we will just enable the GNU glob for ARC as a quick measure and I already planned to look into removing our cheat after the release, thus we can easily disable GNU glob for the defconfig afterwards.
> 
> Thanks,

Hi Bernhard,

Can u please do that as a standalone commit or perhaps squash it with 3/8 in my
other series.

Thx,
-Vineet
Bernhard Reutner-Fischer Feb. 18, 2015, 8:05 a.m. UTC | #12
On February 18, 2015 7:01:23 AM GMT+01:00, Vineet Gupta <Vineet.Gupta1@synopsys.com> wrote:
>On Saturday 14 February 2015 01:32 AM, Bernhard Reutner-Fischer wrote:
>>> I'm not really a libc developer so I would refrain from deciding
>what
>>> >is right and what is wrong with those defines, I just need "make"
>to
>>> >run uClibc testsuite and Vineet greenlighted for me enabling gnu
>glob
>>> >in default config for ARC.
>> So we will just enable the GNU glob for ARC as a quick measure and I
>already planned to look into removing our cheat after the release, thus
>we can easily disable GNU glob for the defconfig afterwards.
>> 
>> Thanks,
>
>Hi Bernhard,
>
>Can u please do that as a standalone commit or perhaps squash it with
>3/8 in my
>other series.

I committed it as separate patch yesterday evening.
See?

Thanks,
diff mbox

Patch

diff --git a/extra/Configs/defconfigs/arc/arcv2_defconfig b/extra/Configs/defconfigs/arc/arcv2_defconfig
index fb418e3..547685f 100644
--- a/extra/Configs/defconfigs/arc/arcv2_defconfig
+++ b/extra/Configs/defconfigs/arc/arcv2_defconfig
@@ -23,6 +23,7 @@  UCLIBC_HAS_LOCALE=y
 UCLIBC_HAS_GLIBC_CUSTOM_STREAMS=y
 UCLIBC_HAS_NFTW=y
 UCLIBC_HAS_FTW=y
+UCLIBC_HAS_GNU_GLOB=y
 RUNTIME_PREFIX="%RUNTIME_PREFIX%"
 DEVEL_PREFIX="%DEVEL_PREFIX%"
 CROSS_COMPILER_PREFIX="arc-linux-uclibc-"
diff --git a/extra/Configs/defconfigs/arc/defconfig b/extra/Configs/defconfigs/arc/defconfig
index 939585f..2dde88b 100644
--- a/extra/Configs/defconfigs/arc/defconfig
+++ b/extra/Configs/defconfigs/arc/defconfig
@@ -22,6 +22,7 @@  UCLIBC_HAS_LOCALE=y
 UCLIBC_HAS_GLIBC_CUSTOM_STREAMS=y
 UCLIBC_HAS_NFTW=y
 UCLIBC_HAS_FTW=y
+UCLIBC_HAS_GNU_GLOB=y
 RUNTIME_PREFIX="%RUNTIME_PREFIX%"
 DEVEL_PREFIX="%DEVEL_PREFIX%"
 CROSS_COMPILER_PREFIX="arc-linux-uclibc-"