* [PATCH 0/4] i18n: Add shell script translation infrastructure @ 2011-05-08 12:10 Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() Ævar Arnfjörð Bjarmason ` (4 more replies) 0 siblings, 5 replies; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-05-08 12:10 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jonathan Nieder, Ævar Arnfjörð Bjarmason As threatened this is the updated shell script translation infrastructure that I'm submitting now that 1.7.5 has been out for a bit. This adds skeleton no-op functions to git-sh-i18n.sh analogous to the gettext.c skeleton functions for C, adds *.sh scripts to the "pot" target for message extraction, and updates the git-sh-i18n--envsubst tests to use the new test_i18ncmp function. Ævar Arnfjörð Bjarmason (4): git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() git-sh-i18n.sh: add no-op gettext() and eval_gettext() wrappers git-sh-i18n.sh: add GIT_GETTEXT_POISON support Makefile: add xgettext target for *.sh files .gitignore | 3 + Documentation/git-sh-i18n--envsubst.txt | 36 +++ Documentation/git-sh-i18n.txt | 57 ++++ Makefile | 8 +- git-sh-i18n.sh | 29 ++ sh-i18n--envsubst.c | 444 +++++++++++++++++++++++++++++++ t/t0201-gettext-fallbacks.sh | 51 ++++ 7 files changed, 627 insertions(+), 1 deletions(-) create mode 100644 Documentation/git-sh-i18n--envsubst.txt create mode 100644 Documentation/git-sh-i18n.txt create mode 100644 git-sh-i18n.sh create mode 100644 sh-i18n--envsubst.c create mode 100755 t/t0201-gettext-fallbacks.sh -- 1.7.4.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() 2011-05-08 12:10 [PATCH 0/4] i18n: Add shell script translation infrastructure Ævar Arnfjörð Bjarmason @ 2011-05-08 12:10 ` Ævar Arnfjörð Bjarmason 2011-05-08 17:15 ` Junio C Hamano 2011-05-08 12:10 ` [PATCH 2/4] git-sh-i18n.sh: add no-op gettext() and eval_gettext() wrappers Ævar Arnfjörð Bjarmason ` (3 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-05-08 12:10 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jonathan Nieder, Ævar Arnfjörð Bjarmason Add a git-sh-i18n--envsubst program which is a stripped-down version of the GNU envsubst(1) program that comes with GNU gettext for use in the eval_gettext() fallback, instead of using a clever (but broken) printf + eval + printf trick. In a previous incarnation of the gettext series I implemented the eval_gettext() fallback like this: eval_gettext() { gettext_out=$(gettext "$1") gettext_eval="printf '%s' \"$gettext_out\"" printf "%s" "`eval \"$gettext_eval\"`" } This was clever, but would incorrectly handle cases where the variable being interpolated contained spaces. E.g.: cmd="git foo"; eval_gettext "command: \$cmd" Would emit "command: gitfoo", instead of the correct "command: git foo". This happened with a message in git-am.sh that used the $cmdline variable. An alternate version suggested by Junio is able to handle this case: eval_gettext() { gettext_out=$(gettext "$1") && gettext_eval="printf '%s' \"$gettext_out\"" && gettext_cmd=$(eval "$gettext_eval") && printf "%s" "$gettext_cmd" } But it strips out double-quotes when they're included in the message being passed to eval_gettext. E.g. the message in git-am.sh: "When you have resolved this problem run \"$cmdline --resolved\"." Will be emitted as: When you have resolved this problem run git am --resolved. Instead of the correct: When you have resolved this problem run "git am --resolved". To work around this, and to improve our variable expansion behavior (eval has security issues) I've imported a stripped-down version of gettext's envsubst(1) program. With it eval_gettext() is implemented like this: eval_gettext() { printf "%s" "$1" | ( export PATH $(git sh-i18n--envsubst --variables "$1"); git sh-i18n--envsubst "$1" ) } These are the modifications I made to envsubst.c as I turned it into sh-i18n--envsubst.c: * Added our git-compat-util.h header for xrealloc() and friends. * Removed inclusion of gettext-specific headers. * Removed most of main() and replaced it with my own. The modified version only does option parsing for --variables. That's all it needs. * Modified error() invocations to use our error() instead of error(3). * Replaced the gettext XNMALLOC(n, size) macro with just xmalloc(n). Since XNMALLOC() only allocated char's. * Removed the string_list_destroy function. It's redundant (also in the upstream code). * Replaced the use of stdbool.h (a C99 header) by doing the following replacements on the code: * s/bool/unsigned short int/g * s/true/1/g * s/false/0/g Reported-by: Johannes Sixt <j.sixt@viscovery.net> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- .gitignore | 2 + Documentation/git-sh-i18n--envsubst.txt | 36 +++ Makefile | 1 + sh-i18n--envsubst.c | 444 +++++++++++++++++++++++++++++++ t/t0201-gettext-fallbacks.sh | 51 ++++ 5 files changed, 534 insertions(+), 0 deletions(-) create mode 100644 Documentation/git-sh-i18n--envsubst.txt create mode 100644 sh-i18n--envsubst.c create mode 100755 t/t0201-gettext-fallbacks.sh diff --git a/.gitignore b/.gitignore index 711fce7..1ccf797 100644 --- a/.gitignore +++ b/.gitignore @@ -127,6 +127,8 @@ /git-rm /git-send-email /git-send-pack +/git-sh-i18n +/git-sh-i18n--envsubst /git-sh-setup /git-shell /git-shortlog diff --git a/Documentation/git-sh-i18n--envsubst.txt b/Documentation/git-sh-i18n--envsubst.txt new file mode 100644 index 0000000..e146a2c --- /dev/null +++ b/Documentation/git-sh-i18n--envsubst.txt @@ -0,0 +1,36 @@ +git-sh-i18n--envsubst(1) +======================== + +NAME +---- +git-sh-i18n--envsubst - Git's own envsubst(1) for i18n fallbacks + +DESCRIPTION +----------- + +This is not a command the end user would want to run. Ever. +This documentation is meant for people who are studying the +plumbing scripts and/or are writing new ones. + +git-sh-i18n--envsubst is Git's stripped-down copy of the GNU +`envsubst(1)` program that comes with the GNU gettext +package. It's used internally by linkgit:git-sh-i18n[1] to +provide the `eval_gettext` fallback on Solaris and systems +that don't have a gettext implementation. + +No promises are made about the interface, or that this +program won't disappear without warning in the next version +of Git. Don't use it. + +Author +------ +Written by Ævar Arnfjörð Bjarmason <avarab@gmail.com> + +Documentation +-------------- +Documentation by Ævar Arnfjörð Bjarmason and the git-list +<git@vger.kernel.org>. + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Makefile b/Makefile index ef57c1c..87c83cb 100644 --- a/Makefile +++ b/Makefile @@ -414,6 +414,7 @@ PROGRAM_OBJS += shell.o PROGRAM_OBJS += show-index.o PROGRAM_OBJS += upload-pack.o PROGRAM_OBJS += http-backend.o +PROGRAM_OBJS += sh-i18n--envsubst.o PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS)) diff --git a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c new file mode 100644 index 0000000..8db71b1 --- /dev/null +++ b/sh-i18n--envsubst.c @@ -0,0 +1,444 @@ +/* + * sh-i18n--envsubst.c - a stripped-down version of gettext's envsubst(1) + * + * Copyright (C) 2010 Ævar Arnfjörð Bjarmason + * + * This is a modified version of + * 67d0871a8c:gettext-runtime/src/envsubst.c from the gettext.git + * repository. It has been stripped down to only implement the + * envsubst(1) features that we need in the git-sh-i18n fallbacks. + * + * The "Close standard error" part in main() is from + * 8dac033df0:gnulib-local/lib/closeout.c. The copyright notices for + * both files are reproduced immediately below. + */ + +#include "git-compat-util.h" + +/* Substitution of environment variables in shell format strings. + Copyright (C) 2003-2007 Free Software Foundation, Inc. + Written by Bruno Haible <bruno@clisp.org>, 2003. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2, or (at your option) + any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software Foundation, + Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ + +/* closeout.c - close standard output and standard error + Copyright (C) 1998-2007 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2, or (at your option) + any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software Foundation, + Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ + +#include <errno.h> +#include <getopt.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +/* If true, substitution shall be performed on all variables. */ +static unsigned short int all_variables; + +/* Forward declaration of local functions. */ +static void print_variables (const char *string); +static void note_variables (const char *string); +static void subst_from_stdin (void); + +int +main (int argc, char *argv[]) +{ + /* Default values for command line options. */ + unsigned short int show_variables = 0; + + switch (argc) + { + case 1: + error ("we won't substitute all variables on stdin for you"); + /* + all_variables = 1; + subst_from_stdin (); + */ + case 2: + /* echo '$foo and $bar' | git sh-i18n--envsubst --variables '$foo and $bar' */ + all_variables = 0; + note_variables (argv[1]); + subst_from_stdin (); + break; + case 3: + /* git sh-i18n--envsubst --variables '$foo and $bar' */ + if (strcmp(argv[1], "--variables")) + error ("first argument must be --variables when two are given"); + show_variables = 1; + print_variables (argv[2]); + break; + default: + error ("too many arguments"); + break; + } + + /* Close standard error. This is simpler than fwriteerror_no_ebadf, because + upon failure we don't need an errno - all we can do at this point is to + set an exit status. */ + errno = 0; + if (ferror (stderr) || fflush (stderr)) + { + fclose (stderr); + exit (EXIT_FAILURE); + } + if (fclose (stderr) && errno != EBADF) + exit (EXIT_FAILURE); + + exit (EXIT_SUCCESS); +} + +/* Parse the string and invoke the callback each time a $VARIABLE or + ${VARIABLE} construct is seen, where VARIABLE is a nonempty sequence + of ASCII alphanumeric/underscore characters, starting with an ASCII + alphabetic/underscore character. + We allow only ASCII characters, to avoid dependencies w.r.t. the current + encoding: While "${\xe0}" looks like a variable access in ISO-8859-1 + encoding, it doesn't look like one in the BIG5, BIG5-HKSCS, GBK, GB18030, + SHIFT_JIS, JOHAB encodings, because \xe0\x7d is a single character in these + encodings. */ +static void +find_variables (const char *string, + void (*callback) (const char *var_ptr, size_t var_len)) +{ + for (; *string != '\0';) + if (*string++ == '$') + { + const char *variable_start; + const char *variable_end; + unsigned short int valid; + char c; + + if (*string == '{') + string++; + + variable_start = string; + c = *string; + if ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_') + { + do + c = *++string; + while ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') + || (c >= '0' && c <= '9') || c == '_'); + variable_end = string; + + if (variable_start[-1] == '{') + { + if (*string == '}') + { + string++; + valid = 1; + } + else + valid = 0; + } + else + valid = 1; + + if (valid) + callback (variable_start, variable_end - variable_start); + } + } +} + + +/* Print a variable to stdout, followed by a newline. */ +static void +print_variable (const char *var_ptr, size_t var_len) +{ + fwrite (var_ptr, var_len, 1, stdout); + putchar ('\n'); +} + +/* Print the variables contained in STRING to stdout, each one followed by a + newline. */ +static void +print_variables (const char *string) +{ + find_variables (string, &print_variable); +} + + +/* Type describing list of immutable strings, + implemented using a dynamic array. */ +typedef struct string_list_ty string_list_ty; +struct string_list_ty +{ + const char **item; + size_t nitems; + size_t nitems_max; +}; + +/* Initialize an empty list of strings. */ +static inline void +string_list_init (string_list_ty *slp) +{ + slp->item = NULL; + slp->nitems = 0; + slp->nitems_max = 0; +} + +/* Append a single string to the end of a list of strings. */ +static inline void +string_list_append (string_list_ty *slp, const char *s) +{ + /* Grow the list. */ + if (slp->nitems >= slp->nitems_max) + { + size_t nbytes; + + slp->nitems_max = slp->nitems_max * 2 + 4; + nbytes = slp->nitems_max * sizeof (slp->item[0]); + slp->item = (const char **) xrealloc (slp->item, nbytes); + } + + /* Add the string to the end of the list. */ + slp->item[slp->nitems++] = s; +} + +/* Compare two strings given by reference. */ +static int +cmp_string (const void *pstr1, const void *pstr2) +{ + const char *str1 = *(const char **)pstr1; + const char *str2 = *(const char **)pstr2; + + return strcmp (str1, str2); +} + +/* Sort a list of strings. */ +static inline void +string_list_sort (string_list_ty *slp) +{ + if (slp->nitems > 0) + qsort (slp->item, slp->nitems, sizeof (slp->item[0]), cmp_string); +} + +/* Test whether a string list contains a given string. */ +static inline int +string_list_member (const string_list_ty *slp, const char *s) +{ + size_t j; + + for (j = 0; j < slp->nitems; ++j) + if (strcmp (slp->item[j], s) == 0) + return 1; + return 0; +} + +/* Test whether a sorted string list contains a given string. */ +static int +sorted_string_list_member (const string_list_ty *slp, const char *s) +{ + size_t j1, j2; + + j1 = 0; + j2 = slp->nitems; + if (j2 > 0) + { + /* Binary search. */ + while (j2 - j1 > 1) + { + /* Here we know that if s is in the list, it is at an index j + with j1 <= j < j2. */ + size_t j = (j1 + j2) >> 1; + int result = strcmp (slp->item[j], s); + + if (result > 0) + j2 = j; + else if (result == 0) + return 1; + else + j1 = j + 1; + } + if (j2 > j1) + if (strcmp (slp->item[j1], s) == 0) + return 1; + } + return 0; +} + + +/* Set of variables on which to perform substitution. + Used only if !all_variables. */ +static string_list_ty variables_set; + +/* Adds a variable to variables_set. */ +static void +note_variable (const char *var_ptr, size_t var_len) +{ + char *string = xmalloc (var_len + 1); + memcpy (string, var_ptr, var_len); + string[var_len] = '\0'; + + string_list_append (&variables_set, string); +} + +/* Stores the variables occurring in the string in variables_set. */ +static void +note_variables (const char *string) +{ + string_list_init (&variables_set); + find_variables (string, ¬e_variable); + string_list_sort (&variables_set); +} + + +static int +do_getc () +{ + int c = getc (stdin); + + if (c == EOF) + { + if (ferror (stdin)) + error ("error while reading standard input"); + } + + return c; +} + +static inline void +do_ungetc (int c) +{ + if (c != EOF) + ungetc (c, stdin); +} + +/* Copies stdin to stdout, performing substitutions. */ +static void +subst_from_stdin () +{ + static char *buffer; + static size_t bufmax; + static size_t buflen; + int c; + + for (;;) + { + c = do_getc (); + if (c == EOF) + break; + /* Look for $VARIABLE or ${VARIABLE}. */ + if (c == '$') + { + unsigned short int opening_brace = 0; + unsigned short int closing_brace = 0; + + c = do_getc (); + if (c == '{') + { + opening_brace = 1; + c = do_getc (); + } + if ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_') + { + unsigned short int valid; + + /* Accumulate the VARIABLE in buffer. */ + buflen = 0; + do + { + if (buflen >= bufmax) + { + bufmax = 2 * bufmax + 10; + buffer = xrealloc (buffer, bufmax); + } + buffer[buflen++] = c; + + c = do_getc (); + } + while ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') + || (c >= '0' && c <= '9') || c == '_'); + + if (opening_brace) + { + if (c == '}') + { + closing_brace = 1; + valid = 1; + } + else + { + valid = 0; + do_ungetc (c); + } + } + else + { + valid = 1; + do_ungetc (c); + } + + if (valid) + { + /* Terminate the variable in the buffer. */ + if (buflen >= bufmax) + { + bufmax = 2 * bufmax + 10; + buffer = xrealloc (buffer, bufmax); + } + buffer[buflen] = '\0'; + + /* Test whether the variable shall be substituted. */ + if (!all_variables + && !sorted_string_list_member (&variables_set, buffer)) + valid = 0; + } + + if (valid) + { + /* Substitute the variable's value from the environment. */ + const char *env_value = getenv (buffer); + + if (env_value != NULL) + fputs (env_value, stdout); + } + else + { + /* Perform no substitution at all. Since the buffered input + contains no other '$' than at the start, we can just + output all the buffered contents. */ + putchar ('$'); + if (opening_brace) + putchar ('{'); + fwrite (buffer, buflen, 1, stdout); + if (closing_brace) + putchar ('}'); + } + } + else + { + do_ungetc (c); + putchar ('$'); + if (opening_brace) + putchar ('{'); + } + } + else + putchar (c); + } +} diff --git a/t/t0201-gettext-fallbacks.sh b/t/t0201-gettext-fallbacks.sh new file mode 100755 index 0000000..54d98b9 --- /dev/null +++ b/t/t0201-gettext-fallbacks.sh @@ -0,0 +1,51 @@ +#!/bin/sh +# +# Copyright (c) 2010 Ævar Arnfjörð Bjarmason +# + +test_description='Gettext Shell fallbacks' + +. ./test-lib.sh +. "$GIT_BUILD_DIR"/git-sh-i18n + +test_expect_success 'gettext: our gettext() fallback has pass-through semantics' ' + printf "test" >expect && + gettext "test" >actual && + test_i18ncmp expect actual && + printf "test more words" >expect && + gettext "test more words" >actual && + test_i18ncmp expect actual +' + +test_expect_success 'eval_gettext: our eval_gettext() fallback has pass-through semantics' ' + printf "test" >expect && + eval_gettext "test" >actual && + test_i18ncmp expect actual && + printf "test more words" >expect && + eval_gettext "test more words" >actual && + test_i18ncmp expect actual +' + +test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables' ' + printf "test YesPlease" >expect && + GIT_INTERNAL_GETTEXT_TEST_FALLBACKS=YesPlease eval_gettext "test \$GIT_INTERNAL_GETTEXT_TEST_FALLBACKS" >actual && + test_i18ncmp expect actual +' + +test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables with spaces' ' + cmdline="git am" && + export cmdline; + printf "When you have resolved this problem run git am --resolved." >expect && + eval_gettext "When you have resolved this problem run \$cmdline --resolved." >actual + test_i18ncmp expect actual +' + +test_expect_success 'eval_gettext: our eval_gettext() fallback can interpolate variables with spaces and quotes' ' + cmdline="git am" && + export cmdline; + printf "When you have resolved this problem run \"git am --resolved\"." >expect && + eval_gettext "When you have resolved this problem run \"\$cmdline --resolved\"." >actual + test_i18ncmp expect actual +' + +test_done -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() 2011-05-08 12:10 ` [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() Ævar Arnfjörð Bjarmason @ 2011-05-08 17:15 ` Junio C Hamano 2011-05-08 21:33 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2011-05-08 17:15 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Jonathan Nieder Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Add a git-sh-i18n--envsubst program which is a stripped-down version > of the GNU envsubst(1) program that comes with GNU gettext for use in > the eval_gettext() fallback Ok up to this point. > ... instead of using a clever (but broken) > printf + eval + printf trick. > > In a previous incarnation of the gettext series I implemented the > eval_gettext() fallback like this: > ... > This was clever, but ... > ... > To work around this, and to improve our variable expansion behavior > (eval has security issues) I've imported a stripped-down version of > gettext's envsubst(1) program. I do not think the lengthy history of failed experiments above is worth explaining. If you really want to say something to justify a new helper, I think it is sufficient to just explain that it is unsolvable in shell. I tried that in the first 9-line paragraph in: http://thread.gmane.org/gmane.comp.version-control.git/170703/focus=170770 In other words, "we tried X that didn't work and we tried Y that didn't either, we cannot think of any better solution, so we are doing something else" is not a good justificiation for doing that "something else". "Anything based on shell is an unpractical solution for this and that reasons, so we use this instead" explains that the earlier failures were not because we did not try hard enough. Unlike "tried X and Y but didn't work", dismissing "anything based on shell" as a whole class with clear explanation why it would not work would prevent people from pursuing that dead-end approach. It also avoids giving quibbling people an excuse to argue against importing envsubst implementation saying "you didn't try hard enough". > With it eval_gettext() is implemented > like this: > ... > Reported-by: Johannes Sixt <j.sixt@viscovery.net> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Ok. > diff --git a/Documentation/git-sh-i18n--envsubst.txt b/Documentation/git-sh-i18n--envsubst.txt > new file mode 100644 > index 0000000..e146a2c > --- /dev/null > +++ b/Documentation/git-sh-i18n--envsubst.txt > @@ -0,0 +1,36 @@ > ... > +Author > +------ > +Written by Ævar Arnfjörð Bjarmason <avarab@gmail.com> > + > +Documentation > +-------------- > +Documentation by Ævar Arnfjörð Bjarmason and the git-list > +<git@vger.kernel.org>. I do not think we do these individual credits these days in the doc. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() 2011-05-08 17:15 ` Junio C Hamano @ 2011-05-08 21:33 ` Ævar Arnfjörð Bjarmason 2011-05-09 3:17 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-05-08 21:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jonathan Nieder On Sun, May 8, 2011 at 19:15, Junio C Hamano <gitster@pobox.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Add a git-sh-i18n--envsubst program which is a stripped-down version >> of the GNU envsubst(1) program that comes with GNU gettext for use in >> the eval_gettext() fallback > > Ok up to this point. > >> ... instead of using a clever (but broken) >> printf + eval + printf trick. >> >> In a previous incarnation of the gettext series I implemented the >> eval_gettext() fallback like this: >> ... >> This was clever, but ... >> ... >> To work around this, and to improve our variable expansion behavior >> (eval has security issues) I've imported a stripped-down version of >> gettext's envsubst(1) program. > > I do not think the lengthy history of failed experiments above is worth > explaining. If you really want to say something to justify a new helper, I > think it is sufficient to just explain that it is unsolvable in shell. I > tried that in the first 9-line paragraph in: > > http://thread.gmane.org/gmane.comp.version-control.git/170703/focus=170770 > > In other words, "we tried X that didn't work and we tried Y that didn't > either, we cannot think of any better solution, so we are doing something > else" is not a good justificiation for doing that "something else". > > "Anything based on shell is an unpractical solution for this and that > reasons, so we use this instead" explains that the earlier failures were > not because we did not try hard enough. Unlike "tried X and Y but didn't > work", dismissing "anything based on shell" as a whole class with clear > explanation why it would not work would prevent people from pursuing that > dead-end approach. It also avoids giving quibbling people an excuse to > argue against importing envsubst implementation saying "you didn't try > hard enough". I included the failed experiments because I know for sure that those don't work, but I'm not sure that this is impossible in shell, and it might be possible in some non-POSIX shell (which we could optionally take advantage of on certain platforms in the future). So having the failed examples that we know don't work is valuable for anyone digging into this in the future. Or are we sure that this can't work in POSIX (or non-POSIX shells)? >> With it eval_gettext() is implemented >> like this: >> ... >> Reported-by: Johannes Sixt <j.sixt@viscovery.net> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > > Ok. > >> diff --git a/Documentation/git-sh-i18n--envsubst.txt b/Documentation/git-sh-i18n--envsubst.txt >> new file mode 100644 >> index 0000000..e146a2c >> --- /dev/null >> +++ b/Documentation/git-sh-i18n--envsubst.txt >> @@ -0,0 +1,36 @@ >> ... >> +Author >> +------ >> +Written by Ævar Arnfjörð Bjarmason <avarab@gmail.com> >> + >> +Documentation >> +-------------- >> +Documentation by Ævar Arnfjörð Bjarmason and the git-list >> +<git@vger.kernel.org>. > > I do not think we do these individual credits these days in the doc. I wrote this when that was the convention, but evidently we've changed that since then. I'll remove it in the next iteration. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() 2011-05-08 21:33 ` Ævar Arnfjörð Bjarmason @ 2011-05-09 3:17 ` Junio C Hamano 2011-05-09 7:52 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2011-05-09 3:17 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Jonathan Nieder Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Or are we sure that this can't work in POSIX (or non-POSIX shells)? You need to implement the same logic to tokenize enough to understand the expansion the shell does inside dq. Yes, shell is general programming language and you can theoretically implement it, but would the result be a practical solution? I doubt it. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() 2011-05-09 3:17 ` Junio C Hamano @ 2011-05-09 7:52 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-05-09 7:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jonathan Nieder On Mon, May 9, 2011 at 05:17, Junio C Hamano <gitster@pobox.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Or are we sure that this can't work in POSIX (or non-POSIX shells)? > > You need to implement the same logic to tokenize enough to understand the > expansion the shell does inside dq. Yes, shell is general programming > language and you can theoretically implement it, but would the result be a > practical solution? I doubt it. I'll write a better commit message and re-send this soon (after some other comments have trickled in). ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] git-sh-i18n.sh: add no-op gettext() and eval_gettext() wrappers 2011-05-08 12:10 [PATCH 0/4] i18n: Add shell script translation infrastructure Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() Ævar Arnfjörð Bjarmason @ 2011-05-08 12:10 ` Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 3/4] git-sh-i18n.sh: add GIT_GETTEXT_POISON support Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 4 siblings, 0 replies; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-05-08 12:10 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jonathan Nieder, Ævar Arnfjörð Bjarmason Add a no-op wrapper library for Git's shell scripts. To split up the gettext series I'm first submitting patches to gettextize the source tree before I add any of the Makefile and Shell library changes needed to actually use them. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- .gitignore | 1 + Documentation/git-sh-i18n.txt | 57 +++++++++++++++++++++++++++++++++++++++++ Makefile | 1 + git-sh-i18n.sh | 17 ++++++++++++ 4 files changed, 76 insertions(+), 0 deletions(-) create mode 100644 Documentation/git-sh-i18n.txt create mode 100644 git-sh-i18n.sh diff --git a/.gitignore b/.gitignore index 1ccf797..4bdb839 100644 --- a/.gitignore +++ b/.gitignore @@ -130,6 +130,7 @@ /git-sh-i18n /git-sh-i18n--envsubst /git-sh-setup +/git-sh-i18n /git-shell /git-shortlog /git-show diff --git a/Documentation/git-sh-i18n.txt b/Documentation/git-sh-i18n.txt new file mode 100644 index 0000000..b41b3af --- /dev/null +++ b/Documentation/git-sh-i18n.txt @@ -0,0 +1,57 @@ +git-sh-i18n(1) +============== + +NAME +---- +git-sh-i18n - Git's i18n setup code for shell scripts + +SYNOPSIS +-------- +'. "$(git --exec-path)/git-sh-i18n"' + +DESCRIPTION +----------- + +This is not a command the end user would want to run. Ever. +This documentation is meant for people who are studying the +Porcelain-ish scripts and/or are writing new ones. + +The 'git sh-i18n scriptlet is designed to be sourced (using +`.`) by Git's porcelain programs implemented in shell +script. It provides wrappers for the GNU `gettext` and +`eval_gettext` functions accessible through the `gettext.sh` +script, and provides pass-through fallbacks on systems +without GNU gettext. + +FUNCTIONS +--------- + +gettext:: + On GNU systems this will be the `gettext` function from + `gettext.sh`, on Solaris it`ll be the `gettext(1)` + command. + + If neither of those implementations are available a + dummy fall-through function is provided. + +eval_gettext:: + On GNU systems this will be the `eval_gettext` function + from `gettext.sh`, on Solaris we provide an + `eval_gettext` using the + linkgit:git-sh-i18n--envsubst[1] helper. + + If neither of those implementations are available a + dummy fall-through function is provided. + +Author +------ +Written by Ævar Arnfjörð Bjarmason <avarab@gmail.com> + +Documentation +-------------- +Documentation by Ævar Arnfjörð Bjarmason and the git-list +<git@vger.kernel.org>. + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Makefile b/Makefile index 87c83cb..83889f3 100644 --- a/Makefile +++ b/Makefile @@ -381,6 +381,7 @@ SCRIPT_LIB += git-rebase--am SCRIPT_LIB += git-rebase--interactive SCRIPT_LIB += git-rebase--merge SCRIPT_LIB += git-sh-setup +SCRIPT_LIB += git-sh-i18n SCRIPT_PERL += git-add--interactive.perl SCRIPT_PERL += git-difftool.perl diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh new file mode 100644 index 0000000..ea05e16 --- /dev/null +++ b/git-sh-i18n.sh @@ -0,0 +1,17 @@ +#!/bin/sh +# +# Copyright (c) 2010 Ævar Arnfjörð Bjarmason +# +# This is a skeleton no-op implementation of gettext for Git. It'll be +# replaced by something that uses gettext.sh in a future patch series. + +gettext () { + printf "%s" "$1" +} + +eval_gettext () { + printf "%s" "$1" | ( + export PATH $(git sh-i18n--envsubst --variables "$1"); + git sh-i18n--envsubst "$1" + ) +} -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] git-sh-i18n.sh: add GIT_GETTEXT_POISON support 2011-05-08 12:10 [PATCH 0/4] i18n: Add shell script translation infrastructure Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 2/4] git-sh-i18n.sh: add no-op gettext() and eval_gettext() wrappers Ævar Arnfjörð Bjarmason @ 2011-05-08 12:10 ` Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 4/4] Makefile: add xgettext target for *.sh files Ævar Arnfjörð Bjarmason 2011-05-08 17:03 ` [PATCH 0/4] i18n: Add shell script translation infrastructure Sverre Rabbelier 4 siblings, 0 replies; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-05-08 12:10 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jonathan Nieder, Ævar Arnfjörð Bjarmason Change git-sh-i18n.sh to support the GIT_GETTEXT_POISON environment variable like gettext.c does, this ensures that tests that use git-sh-i18n.sh will fail under GETTEXT_POISON=YesPlease if they rely on Git's C locale messages without declaring that they do. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- git-sh-i18n.sh | 30 +++++++++++++++++++++--------- 1 files changed, 21 insertions(+), 9 deletions(-) diff --git a/git-sh-i18n.sh b/git-sh-i18n.sh index ea05e16..32ca59d 100644 --- a/git-sh-i18n.sh +++ b/git-sh-i18n.sh @@ -5,13 +5,25 @@ # This is a skeleton no-op implementation of gettext for Git. It'll be # replaced by something that uses gettext.sh in a future patch series. -gettext () { - printf "%s" "$1" -} +if test -z "$GIT_GETTEXT_POISON" +then + gettext () { + printf "%s" "$1" + } + + eval_gettext () { + printf "%s" "$1" | ( + export PATH $(git sh-i18n--envsubst --variables "$1"); + git sh-i18n--envsubst "$1" + ) + } +else + gettext () { + printf "%s" "# GETTEXT POISON #" + } + + eval_gettext () { + printf "%s" "# GETTEXT POISON #" + } +fi -eval_gettext () { - printf "%s" "$1" | ( - export PATH $(git sh-i18n--envsubst --variables "$1"); - git sh-i18n--envsubst "$1" - ) -} -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] Makefile: add xgettext target for *.sh files 2011-05-08 12:10 [PATCH 0/4] i18n: Add shell script translation infrastructure Ævar Arnfjörð Bjarmason ` (2 preceding siblings ...) 2011-05-08 12:10 ` [PATCH 3/4] git-sh-i18n.sh: add GIT_GETTEXT_POISON support Ævar Arnfjörð Bjarmason @ 2011-05-08 12:10 ` Ævar Arnfjörð Bjarmason 2011-05-08 17:03 ` [PATCH 0/4] i18n: Add shell script translation infrastructure Sverre Rabbelier 4 siblings, 0 replies; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-05-08 12:10 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jonathan Nieder, Ævar Arnfjörð Bjarmason Change the "pot" target to also extract strings from our $(SCRIPT_SH) files with xgettext(1). Note that due to Jonathan Nieder's trick of doing "mv $@+ $@" at the end of the target the "pot" target will now warn: $ make pot XGETTEXT po/git.pot po/git.pot+: warning: Charset "CHARSET" is not a portable encoding name. Message conversion to user's charset might not work. This warnings is emitted because xgettext is writing into a non-*.pot file, it's harmless however. The content that's written out is equivalent to what it would be if we were writing directly into an existing POT file with --join-existing. As part of this change I've eliminated the && chain between xgettext calls, this is incompatible with $(QUIET_XGETTEXT), if the && is left in it'll emit: /bin/sh: @echo: not found Since it's redundant (the Makefile will stop if there's an error) I've removed it altogether. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 83889f3..10252d5 100644 --- a/Makefile +++ b/Makefile @@ -2065,10 +2065,14 @@ XGETTEXT_FLAGS = \ --from-code=UTF-8 XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \ --keyword=_ --keyword=N_ --keyword="Q_:1,2" +XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell LOCALIZED_C := $(C_OBJ:o=c) +LOCALIZED_SH := $(SCRIPT_SH) po/git.pot: $(LOCALIZED_C) - $(QUIET_XGETTEXT)$(XGETTEXT) -o$@+ $(XGETTEXT_FLAGS_C) $(LOCALIZED_C) && \ + $(QUIET_XGETTEXT)$(XGETTEXT) -o$@+ $(XGETTEXT_FLAGS_C) $(LOCALIZED_C) + $(QUIET_XGETTEXT)$(XGETTEXT) -o$@+ --join-existing $(XGETTEXT_FLAGS_SH) \ + $(LOCALIZED_SH) mv $@+ $@ pot: po/git.pot -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] i18n: Add shell script translation infrastructure 2011-05-08 12:10 [PATCH 0/4] i18n: Add shell script translation infrastructure Ævar Arnfjörð Bjarmason ` (3 preceding siblings ...) 2011-05-08 12:10 ` [PATCH 4/4] Makefile: add xgettext target for *.sh files Ævar Arnfjörð Bjarmason @ 2011-05-08 17:03 ` Sverre Rabbelier 2011-05-08 21:38 ` Ævar Arnfjörð Bjarmason 4 siblings, 1 reply; 13+ messages in thread From: Sverre Rabbelier @ 2011-05-08 17:03 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Junio C Hamano, Jonathan Nieder Heya, On Sun, May 8, 2011 at 14:10, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > This adds skeleton no-op functions to git-sh-i18n.sh analogous to the > gettext.c skeleton functions for C, adds *.sh scripts to the "pot" > target for message extraction, and updates the git-sh-i18n--envsubst > tests to use the new test_i18ncmp function. I seem to remember there were some concerns about performance with a previous version of this series. Have you done any before/after timings on, say, the git test suite? Or am I remembering incorrectly? -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] i18n: Add shell script translation infrastructure 2011-05-08 17:03 ` [PATCH 0/4] i18n: Add shell script translation infrastructure Sverre Rabbelier @ 2011-05-08 21:38 ` Ævar Arnfjörð Bjarmason 2011-05-08 21:45 ` Sverre Rabbelier 0 siblings, 1 reply; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-05-08 21:38 UTC (permalink / raw) To: Sverre Rabbelier; +Cc: git, Junio C Hamano, Jonathan Nieder On Sun, May 8, 2011 at 19:03, Sverre Rabbelier <srabbelier@gmail.com> wrote: > Heya, > > On Sun, May 8, 2011 at 14:10, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >> This adds skeleton no-op functions to git-sh-i18n.sh analogous to the >> gettext.c skeleton functions for C, adds *.sh scripts to the "pot" >> target for message extraction, and updates the git-sh-i18n--envsubst >> tests to use the new test_i18ncmp function. > > I seem to remember there were some concerns about performance with a > previous version of this series. Have you done any before/after > timings on, say, the git test suite? Or am I remembering incorrectly? When I benchmark this on my (Linux) system it runs at 0% difference between the current test suite and the one with this series. The concern was about Windows, where forks are more expensive, so e.g. every time we shell out to sed/awk/perl/grep or git-sh-i18n--envsubst we incur a larger speed hit than on Unix systems. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] i18n: Add shell script translation infrastructure 2011-05-08 21:38 ` Ævar Arnfjörð Bjarmason @ 2011-05-08 21:45 ` Sverre Rabbelier 2011-05-08 21:52 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 13+ messages in thread From: Sverre Rabbelier @ 2011-05-08 21:45 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason, Johannes Schindelin, Stephan Beyer Cc: git, Junio C Hamano, Jonathan Nieder Heya, [+msysgit people] On Sun, May 8, 2011 at 23:38, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Sun, May 8, 2011 at 19:03, Sverre Rabbelier <srabbelier@gmail.com> wrote: >> On Sun, May 8, 2011 at 14:10, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >>> This adds skeleton no-op functions to git-sh-i18n.sh analogous to the >>> gettext.c skeleton functions for C, adds *.sh scripts to the "pot" >>> target for message extraction, and updates the git-sh-i18n--envsubst >>> tests to use the new test_i18ncmp function. >> >> I seem to remember there were some concerns about performance with a >> previous version of this series. Have you done any before/after >> timings on, say, the git test suite? Or am I remembering incorrectly? > > When I benchmark this on my (Linux) system it runs at 0% difference > between the current test suite and the one with this series. > > The concern was about Windows, where forks are more expensive, so > e.g. every time we shell out to sed/awk/perl/grep or > git-sh-i18n--envsubst we incur a larger speed hit than on Unix > systems. Perhaps one of the windows devs can benchmark this then? Ævar, where do you have a pullable version of this series? Your github fork has quite a few branches. -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/4] i18n: Add shell script translation infrastructure 2011-05-08 21:45 ` Sverre Rabbelier @ 2011-05-08 21:52 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-05-08 21:52 UTC (permalink / raw) To: Sverre Rabbelier Cc: Johannes Schindelin, Stephan Beyer, Sebastian Schuberth, Johannes Sixt, git, Junio C Hamano, Jonathan Nieder On Sun, May 8, 2011 at 23:45, Sverre Rabbelier <srabbelier@gmail.com> wrote: > Heya, > > [+msysgit people] > > On Sun, May 8, 2011 at 23:38, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >> On Sun, May 8, 2011 at 19:03, Sverre Rabbelier <srabbelier@gmail.com> wrote: >>> On Sun, May 8, 2011 at 14:10, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >>>> This adds skeleton no-op functions to git-sh-i18n.sh analogous to the >>>> gettext.c skeleton functions for C, adds *.sh scripts to the "pot" >>>> target for message extraction, and updates the git-sh-i18n--envsubst >>>> tests to use the new test_i18ncmp function. >>> >>> I seem to remember there were some concerns about performance with a >>> previous version of this series. Have you done any before/after >>> timings on, say, the git test suite? Or am I remembering incorrectly? >> >> When I benchmark this on my (Linux) system it runs at 0% difference >> between the current test suite and the one with this series. >> >> The concern was about Windows, where forks are more expensive, so >> e.g. every time we shell out to sed/awk/perl/grep or >> git-sh-i18n--envsubst we incur a larger speed hit than on Unix >> systems. > > Perhaps one of the windows devs can benchmark this then? Ævar, where > do you have a pullable version of this series? Your github fork has > quite a few branches. It's ab/i18n-sh-only in git://github.com/avar/git.git There was some discussion about this a while ago, not sure what came out of that, but there were some msysgit patches involved IIRC. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-05-09 7:52 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-05-08 12:10 [PATCH 0/4] i18n: Add shell script translation infrastructure Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 1/4] git-sh-i18n--envsubst: our own envsubst(1) for eval_gettext() Ævar Arnfjörð Bjarmason 2011-05-08 17:15 ` Junio C Hamano 2011-05-08 21:33 ` Ævar Arnfjörð Bjarmason 2011-05-09 3:17 ` Junio C Hamano 2011-05-09 7:52 ` Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 2/4] git-sh-i18n.sh: add no-op gettext() and eval_gettext() wrappers Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 3/4] git-sh-i18n.sh: add GIT_GETTEXT_POISON support Ævar Arnfjörð Bjarmason 2011-05-08 12:10 ` [PATCH 4/4] Makefile: add xgettext target for *.sh files Ævar Arnfjörð Bjarmason 2011-05-08 17:03 ` [PATCH 0/4] i18n: Add shell script translation infrastructure Sverre Rabbelier 2011-05-08 21:38 ` Ævar Arnfjörð Bjarmason 2011-05-08 21:45 ` Sverre Rabbelier 2011-05-08 21:52 ` Ævar Arnfjörð Bjarmason
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.