From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id D4A6438930E9 for ; Wed, 29 Apr 2020 17:57:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D4A6438930E9 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-235-QxM5FTFIOZ2q5teJBI00wQ-1; Wed, 29 Apr 2020 13:57:25 -0400 X-MC-Unique: QxM5FTFIOZ2q5teJBI00wQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 08345800C78 for ; Wed, 29 Apr 2020 17:57:25 +0000 (UTC) Received: from greed.delorie.com (ovpn-112-52.phx2.redhat.com [10.3.112.52]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CE1391A923; Wed, 29 Apr 2020 17:57:21 +0000 (UTC) Received: from greed.delorie.com.redhat.com (localhost [127.0.0.1]) by greed.delorie.com (8.14.7/8.14.7) with ESMTP id 03THvKuA013833; Wed, 29 Apr 2020 13:57:20 -0400 From: DJ Delorie To: "Carlos O'Donell" Cc: libc-alpha@sourceware.org Subject: Re: [PATCH v2] test-container: Support $(complocaledir) and mkdirp. In-Reply-To: <643f2f94-6797-874e-04aa-79e4599b4e3c@redhat.com> (carlos@redhat.com) Date: Wed, 29 Apr 2020 13:57:20 -0400 Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Apr 2020 17:57:29 -0000 "Carlos O'Donell" writes: > My goal is this: > > * Have the variables mirror the configure variables. > - If in configure we call it 'complocaledir' then internally we always > use 'complocaledir'. > > We have strayed from this somewhat with 'support_complocaledir_prefix', > and if anything I'd suggest we would just rename all of these to match > the configure names e.g. complocaledir instead of support_complocaledir_p= refix. I think the only part that's important is that the Makefile variables match the $variables in the script. Internally, since they're global scope, adding some decoration like support_{variable}_value (or _prefix or _path, whatever) makes sense to avoid namespace collision. > (c) When a container test needs a specific locale, we look for it in the > special place, and copy it into place as required (without locale-arc= hive > support). > - Container tests just copy the locales required into the testroot.root s= o > no additional compilation is required. As much as we're complaining about the speed of compiling these, I think doing it any way other than how the user would install them should be a non-starter, unless we CLEARLY outline how to test the user-path and cache-path code seperately. Although some hackery that *does* the user-path into the testroot, then caches the results (rsync?) would meet both needs. > This patch doesn't make this worse because I'm only compiling test locale= s > not the real locales for testing. Yup :-) >>> +=09=09m =3D strtol (the_words[1], NULL, 0); >>=20 >> Error handling? > > It's in xmkdirp, where we do this: strtol("Carlos", ...) returns 0. You'll silently create an unusable directory. You need to check errno here, with the usual clear/call/test code. > =09test -d $(objpfx)testroot.pristine/bin || \ > =09 mkdir $(objpfx)testroot.pristine/bin > +=09# We need the compiled locale dir for localedef tests. > +=09test -d $(objpfx)testroot.pristine/$(complocaledir) || \ > +=09 mkdir -p $(objpfx)testroot.pristine/$(complocaledir) > =09cp $(objpfx)support/shell-container $(objpfx)testroot.pristine/bin/sh Ok. > * mkdir $buildroot/testroot.pristine/ > * install into it > + * default glibc install > + * create /bin for /bin/sh > + * create $(complocaledir) so localedef tests work with default path= s. > + * install /bin/sh, /bin/echo, and /bin/true. > * rsync to $buildroot/testroot.root/ Ok. > =09 exec FILE > -=09 FILE must start with $B/, $S/, $I/, $L/, or / > -=09 (expands to build dir, source dir, install dir, library dir > -=09 (in container), or container's root) > +=09 mkdirp MODE DIR > + > + variables: > +=09 $B/ build dir, equivalent to $(common-objpfx) > +=09 $S/ source dir, equivalent to $(srcdir) > +=09 $I/ install dir, equivalent to $(prefix) > +=09 $L/ library dir (in container), equivalent to $(libdir) > +=09 $complocaledir/ compiled locale dir, equivalent to $(complocaledir) > +=09 / container's root > + > +=09 If FILE begins with any of these variables then they will be > +=09 substituted for the described value. > + > +=09 The goal is to expose as many of the runtime's configured paths > +=09 via variables so they can be used to setup the container environment > +=09 before execution reaches the test. > + > details: Ok. > =09 - 'exec': change test binary location (may end in /) > +=09 - 'mkdirp': A minimal "mkdir -p FILE" command. > + Ok. > =20 > +=09 /* Expand variables. */ > =09 for (i =3D 1; i < nt; ++i) Ok. > =09=09=09=09=09 the_words[i] + 2, NULL); > +=09=09else if (memcmp (the_words[i], "$complocaledir/", 15) =3D=3D 0) > +=09=09 the_words[i] =3D concat (new_root_path, > +=09=09=09=09=09 support_complocaledir_prefix, > +=09=09=09=09=09 the_words[i] + 14, NULL); > =09=09/* "exec" and "cwd" use inside-root paths. */ Ok. > =09 } > =20 > +=09 /* Run the following commands in the_words[0] with NT number of > +=09 arguments (including the command). */ > + > =09 if (nt =3D=3D 2 && strcmp (the_words[0], "so") =3D=3D 0) Ok. > =09 } > +=09 else if (nt =3D=3D 3 && strcmp (the_words[0], "mkdirp") =3D=3D 0) > +=09 { > +=09=09long int m; > +=09=09m =3D strtol (the_words[1], NULL, 0); > +=09=09xmkdirp (the_words[2], m); > +=09 } > =09 else if (nt > 0 && the_words[0][0] !=3D '#') Needs errno check on strtol().