From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 44474 invoked by alias); 25 Nov 2016 18:49:25 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 44450 invoked by uid 89); 25 Nov 2016 18:49:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=claim X-HELO: mx1.redhat.com Subject: Re: [PATCH] extras: New test/build infrastructure To: Zack Weinberg References: <9426ee75-3e45-6cde-b659-567398007a32@redhat.com> <1d0c74a4-965d-be13-6945-5af479eecbdc@panix.com> <662a011c-f3f8-1c43-9b98-542d499b3dca@redhat.com> Cc: GNU C Library From: Florian Weimer Message-ID: <54d642e3-2965-c6b7-bb15-d2c6131c7f8e@redhat.com> Date: Fri, 25 Nov 2016 18:49:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2016-11/txt/msg00941.txt.bz2 On 11/25/2016 07:44 PM, Zack Weinberg wrote: > On Fri, Nov 25, 2016 at 12:45 PM, Florian Weimer wrote: >> On 11/25/2016 05:14 PM, Zack Weinberg wrote: >>> >>> Can I ask why the new directory is called "extras"? That makes it sound >>> like a home for extra features that we want to provide but not in the >>> core C library. Something more obviously internal-use and >>> build/test-related would be better, I think. >> >> I plan to use bits of it for fixing localedef bugs and contributing Fedora >> changes upstream. Those bits would then end up in installed binaries. >> >> The immediate need is for testing only and generic test support code >> (container setup, a fake DNS server implementation, and so on). > > So maybe "support/" or "build-test-support/"? support/ works for me. Any objects to that? >>>> +libextras-static-only-routines := $(libextras-routines) >>>> +# Only build one variant of the library. >>>> +libextras-inhibit-o := .os >>>> +ifeq ($(build-shared),yes) >>>> +libextras-inhibit-o += .o >>>> +endif >>> >>> This doesn't look right if the goal is to build only the .a version of >>> the library. >> >> Could you clarify what worries you? If there's just one variant, it has to >> be PIC, unless it's a static-only build. > > I may well not understand what this combination of > -static-only-routines and -inhibit-o does, but what it *looks* like it > does is disable generation of .os (PIC) object files unconditionally, > and if shared libraries are enabled, it also disables .o (static) > object files. I would expect this to wind up either not working at > all, or producing only a *shared* library, or possibly only a > profiling library! For shared builds, we still build .oS, that is lib_*nonshared.a. Sure, it's rather strange, but it looks like this is the way it is expected to work. > Also I don't know why this code would need to be PIC. We already have one use of write_message from a test DSO, in dlfcn/bug-atexit3-lib.cc. This was impossible with the existing test skeleton. >>> This library is _not_ part of the implementation and should not be using >>> __ names. And I'm not sure it ought to be using features.h either. >> >> is needed for __BEGIN_DECLS. Including would be >> even more extreme, I think. > > Honestly I think sys/cdefs.h has a better claim to be a public > interface than features.h, since it exists on *BSD (and does in fact > define compatible __BEGIN_DECLS/__END_DECLS there too: see > https://github.com/freebsd/freebsd/blob/master/sys/sys/cdefs.h) Fine, it is. Thanks, Florian