git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v1] convert: add support for 'encoding' attribute

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Dec 11, 2017, 18:39 UTC
Message-ID
<CAPig+cQ6VSXXSYJOiZeTqUpwijVhvvUYzXF8U3KCBsOQ91HPZQ@mail.gmail.com>
In-Reply-To
<20171211155023.1405-1-lars.schneider@autodesk.com>
On Mon, Dec 11, 2017 at 10:50 AM,  <lars.schneider@autodesk.com> wrote:
Show 48 quoted lines
> From: Lars Schneider <larsxschneider@gmail.com>
>
> Git and its tools (e.g. git diff) expect all text files in UTF-8
> encoding. Git will happily accept content in all other encodings, too,
> but it might not be able to process the text (e.g. viewing diffs or
> changing line endings).
>
> Add an attribute to tell Git what encoding the user has defined for a
> given file. If the content is added to the index, then Git converts the
> content to a canonical UTF-8 representation. On checkout Git will
> reverse the conversion.
>
> Reviewed-by: Patrick Lühne <patrick@luehne.de>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> diff --git a/convert.c b/convert.c
> @@ -256,6 +257,149 @@ static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
> +static int encode_to_git(const char *path, const char *src, size_t src_len,
> +                        struct strbuf *buf, struct encoding *enc)
> +{
> +#ifndef NO_ICONV
> +       char *dst, *re_src;
> +       int dst_len, re_src_len;
> +
> +       /*
> +        * No encoding is specified or there is nothing to encode.
> +        * Tell the caller that the content was not modified.
> +        */
> +       if (!enc || (src && !src_len))
> +               return 0;
> +
> +       /*
> +        * Looks like we got called from "would_convert_to_git()".
> +        * This means Git wants to know if it would encode (= modify!)
> +        * the content. Let's answer with "yes", since an encoding was
> +        * specified.
> +        */
> +       if (!buf && !src)
> +               return 1;
> +
> +       if (enc->to_git == invalid_conversion) {
> +               enc->to_git = iconv_open(default_encoding, encoding->name);
> +               if (enc->to_git == invalid_conversion)
> +                       warning(_("unsupported encoding %s"), encoding->name);
> +       }
> +
> +       if (enc->to_worktree == invalid_conversion)
> +               enc->to_worktree = iconv_open(encoding->name, default_encoding);

Do you need to be calling iconv_close() somewhere on the result of the iconv_open() calls? [Answering myself after reading the rest of the patch: You're caching these opened 'iconv' descriptors, so you don't plan on closing them.]

Show 11 quoted lines
> + [...]
> +       /*
> +        * Encode dst back to ensure no information is lost. This wastes
> +        * a few cycles as most conversions are round trip conversion
> +        * safe. However, content that has an invalid encoding might not
> +        * match its original byte sequence after the UTF-8 conversion
> +        * round trip. Let's play safe here and check the round trip
> +        * conversion.
> +        */
> +       re_src = reencode_string_iconv(dst, dst_len, enc->to_worktree, &re_src_len);
> +       if (!re_src || strcmp(src, re_src)) {

You're using strcmp() as opposed to memcmp() because you expect 're_src' will unconditionally be UTF-8-encoded, right?

Show 14 quoted lines
> +               die(_("encoding '%s' from %s to %s and back is not the same"),
> +                       path, enc->name, default_encoding);
> +       }
> +       free(re_src);
> +
> +       strbuf_attach(buf, dst, dst_len, dst_len + 1);
> +       return 1;
> +#else
> +       warning(_("cannot encode '%s' from %s to %s because "
> +               "your Git was not compiled with encoding support"),
> +               path, enc->name, default_encoding);
> +       return 0;
> +#endif
> +}
Previous: lars.schneider@autodesk.comNext: Lars Schneider
Message 2 of 36 in “convert: add support for 'encoding' attribute”
  1. convert: add support for 'encoding' attributelars.schneider@autodesk.com, Dec 11, 2017
  2. Eric SunshineDec 11, 2017
  3. Lars SchneiderDec 11, 2017
  4. Eric SunshineDec 11, 2017
  5. Lars SchneiderDec 12, 2017
  6. Johannes SixtDec 11, 2017
  7. Lars SchneiderDec 11, 2017
  8. Junio C HamanoDec 12, 2017
  9. Johannes SixtDec 12, 2017
  10. Lars SchneiderDec 12, 2017
  11. Junio C HamanoDec 12, 2017
  12. Lars SchneiderDec 13, 2017
  13. Junio C HamanoDec 13, 2017
  14. Lars SchneiderDec 13, 2017
  15. Junio C HamanoDec 14, 2017
  16. Johannes SixtDec 12, 2017
  17. Torsten BögershausenDec 18, 2017
  18. Jeff KingDec 18, 2017
  19. Torsten BögershausenDec 23, 2017
  20. 0/2 git diff --UTF-8tboegi@web.de, Dec 29, 2017
  21. 2/2 git diff: Allow to reencode into UTF-8tboegi@web.de, Dec 29, 2017
  22. 1/2 convert_to_git(): checksafe becomes an integertboegi@web.de, Dec 29, 2017
  23. 1/1 Auto diff of UTF-16 files in UTF-8tboegi@web.de, Feb 26, 2018
  24. Peter KreftingFeb 26, 2018
  25. Jeff KingFeb 27, 2018
  26. Junio C HamanoDec 18, 2017
  27. Johannes SixtDec 18, 2017
  28. Jeff KingDec 15, 2017
  29. Lars SchneiderDec 18, 2017
  30. Jeff KingDec 18, 2017
  31. Torsten BögershausenDec 17, 2017
  32. Lars SchneiderDec 28, 2017
  33. Torsten BögershausenDec 29, 2017
  34. Lars SchneiderDec 29, 2017
  35. Junio C HamanoJan 3, 2018
  36. Lars SchneiderJan 3, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.