This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] test-container: Add $i18ndir, $complocaledir, and mkdirp.
- From: DJ Delorie <dj at redhat dot com>
- To: "Carlos O'Donell" <carlos at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Mon, 10 Dec 2018 22:00:34 -0500
- Subject: Re: [PATCH] test-container: Add $i18ndir, $complocaledir, and mkdirp.
"Carlos O'Donell" <carlos@redhat.com> writes:
> - -DLIBDIR_PATH=\"$(libdir)\"
> + -DLIBDIR_PATH=\"$(libdir)\" \
> + -DI18NDIR_PATH=\"$(i18ndir)\" \
> + -DCOMPLOCALEDIR_PATH=\"$(complocaledir)\"
Expand the list of variables, ok.
> +/* Corresponds to the install's i18ndir/ directory. */
Should be i18n/ directory.
> +/* Corresponds to the install's compiled locale directory. */
> +extern const char support_complocaledir_prefix[];
Ok.
> +#ifdef I18NDIR_PATH
> +/* Corresponds to the install's i18ndir/ directory. */
Should be i18n/ directory.
> +const char support_i18ndir_prefix[] = I18NDIR_PATH;
> +#else
> +# error please -DI18NDIR_PATH=something in the Makefile
> +#endif
Ok.
> +#ifdef COMPLOCALEDIR_PATH
> +/* Corresponds to the install's compiled locale directory. */
> +const char support_complocaledir_prefix[] = COMPLOCALEDIR_PATH;
> +#else
> +# error please -DCOMPLOCALEDIR_PATH=something in the Makefile
> +#endif
Ok.
> - su
> + su
Heh, either make all the whitespace consistent, or leave existing
whitespace alone ;-)
> + so FILE
Ok.
> - FILE must start with $B/, $S/, $I/, $L/, or /
> - (expands to build dir, source dir, install dir, library dir
> - (in container), or container's root)
> + chmod MODE FILE
> + mkdirp MODE DIR
Should we mention that MODE should be like 0775? I.e. not a+x?
> +
> + FILE must start with $B/, $S/, $I/, $L/, $i18ndir/,
> + $complocaledir/ or / (expands to build dir, source dir,
> + install dir, library dir (in container), I18NPATH
Should be "i18n dir"
I wonder if the syntax should be $i18n/ instead of $i18ndir/, and
$complocale/ instead of $complocaledir/ ?
> + (in container), compiled locale dir (in container),
> + or container's root)
> +
Ok.
> details:
> - '#': A comment.
> - 'su': Enables running test as root in the container.
> + - 'so': A shortcut for "cp $PWD/FILE $L/FILE"
Hmm... the "cp" command below doesn't support $PWD... I wonder if this
comment would ever confuse someone into trying $PWD... (OK otherwise,
though, just wondering)
> - 'mv': A minimal move files command.
> - 'cp': A minimal copy files command.
> - 'rm': A minimal remove files command.
> + - 'chmod': A minimal chmod command.
> + - 'mkdirp': A minimal "mkdir -p FILE" command.
> +
Inconsistent whitespace, if we care. I don't, just mentioning it ;-)
> + /* Expand variables. */
Ok.
> + else if (memcmp (the_words[i], "$i18ndir/", 9) == 0)
> + the_words[i] = concat (new_root_path,
> + support_i18ndir_prefix,
> + the_words[i] + 8, NULL);
> + else if (memcmp (the_words[i], "$complocaledir/", 15) == 0)
> + the_words[i] = concat (new_root_path,
> + support_complocaledir_prefix,
> + the_words[i] + 14, NULL);
Ok.
> + /* Run the command in the_words[0] with NT number of arguments
> + (including the command). */
Ok.
> + else if (nt == 3 && strcmp (the_words[0], "mkdirp") == 0)
> + {
> + long int m;
> + m = strtol (the_words[1], NULL, 0);
> + xmkdirp (the_words[2], m);
> + }
Ok.
At some point we should check that strtol() did something useful, and
wasn't passed something like "a+x".