bugGNU roff - Bugs: bug #51330, preconv fails to detect utf-8...

 
 

bug #51330: preconv fails to detect utf-8 without BOM

Submitter:  Bertrand Garrigues <bgarrigues>
Submitted:  Tue 27 Jun 2017 10:31:10 PM UTC
   
 
Category:  Preprocessor preconv Severity:  4 - Important
Item Group:  Incorrect behaviour Status:  Fixed
Privacy:  Public Assigned to:  bgarrigues
Open/Closed:  Closed Planned Release:  1.22.4
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 02 May 2022 01:15:00 AM UTC, comment #8: 


commit 75c0873c3f0d9f353b967e316c20ee1bb06d1e86
Author: Bertrand Garrigues <bertrand.garrigues@laposte.net>
Date:   Tue Jul 4 23:40:46 2017 +0200

    Use uchardet library in `preconv' to detect input file encoding

    * m4/groff.m4 (GROFF_UCHARDET): new macro that use pkg-config to
    check if uchardet library is available and define new option
    `--with-uchardet':
    - If `yes' is passed and uchardet is not found, configure fails.
    - If `no' is passed we don't use uchardet and don't display any
    warning.
    - If `auto' (or any other option) is passed of if --with-uchardet
    is not passed, we use uchardet if available, otherwise the build
    goes on but a warning is displayed at the end of the configuration
    phase.

    * configure.ac: use pkg-config and `GROFF_UCHARDET' to detect the
    presence of uchardet library.

    * src/preproc/preconv/preconv.am: link against uchardet library if
    available.

    * src/preproc/preconv/preconv.cpp (detect_file_encoding): new
    function that uses uchardet library to detect input file encoding.
    (do_file): `detect_file_encoding'.

    * src/preproc/preconv/preconv.1.man: update `preconv'
    documentation.

    See https://savannah.gnu.org/bugs/?51330

commit fa41eeba06987a12bc0434f8b51896d4618a3997
Author: Bertrand Garrigues <bertrand.garrigues@laposte.net>
Date:   Wed Jul 12 00:36:16 2017 +0200

    Add gnulib module fprintf-posix

    * bootstrap.conf: do it.

    * Fix compilation in some files that needed inclusion of
    `config.h'.

    See https://savannah.gnu.org/bugs/?51330


G. Branden Robinson <gbranden>
Group administrator
Sat 22 Jul 2017 12:29:25 AM UTC, comment #7: 

I've taken into account your last remark and commited on master. Have a nice vacation! (by the way, I'm also on vacation, starting today).

Bertrand Garrigues <bgarrigues>
Group administrator
Mon 17 Jul 2017 08:45:31 PM UTC, comment #6: 

Sorry, I'm more or less on vacation right now and can only quickly skim your refinements...

It looks good (again :-), please commit.

Only one minor issue: Please don't talk about `libuchardet.so' in `configure.ac' and the documentation – on other platforms this library file might be called differently.  It's better to talk about the `uchardet library', omitting the details how it gets linked to groff.

Werner LEMBERG <wl>
Group administrator
Wed 12 Jul 2017 11:53:39 AM UTC, comment #5: 

I've also fixed missing space after semi-colon in fprintf calls and updated the man page.

Bertrand Garrigues <bgarrigues>
Group administrator
Wed 12 Jul 2017 11:51:52 AM UTC, comment #4: 

My previous patch was not correct, I don't remember how I tested it, but it cannot work as I have to rewind the file in function detect_file_encoding.  I've made more serious tests and now I have 2 patches ready for commit.

- 0001-Add-gnulib-module-fprintf-posix.patch: this adds gnulib's `fprintf-posix' module to `bootstrap-conf'.  It's somehow difficult for me to think 'pre-C99'.  I had to add inclusions of 'config.h' in some files because I had errors like:

./lib/math.h:33:3: error: #error "Please include config.h first."
  #error "Please include config.h first.

- 0002-Use-libchardet-for-preconv.patch: new patch that fixes the previous one.  I've also fixed the style problem on single-line ELSE clause. As for the auto-detection we now have a `--with-uchardet' option that works like this (tested with and without uchardet installed):

. If `yes' is passed and uchardet is not found, configure fails.

. If `no' is passed we don't use uchardet and don't display any warning.

. If `auto' (or any other option) is passed of if --with-uchardet is not passed, we use uchardet if available, otherwise the build goes on but a warning is displayed at the end of the configuration phase.

With this patch `typesetting.pdf' is correctly generated.


(file #41186, file #41187)

Bertrand Garrigues <bgarrigues>
Group administrator
Wed 05 Jul 2017 04:15:48 AM UTC, comment #3: 

Looks good, thanks.  Some nits.

. I suggest to use an auto-detection mode for uchardet, this is, having an option


--with-uchardet[=auto|no|yes]


where option `auto' is the default.

. In error messages, insert a space after the colon, e.g.


fprintf(stderr, "fstat: %s\n", strerror(errno));
                       ^


. The flag `z' you are using in `fprintf' is available only in C99 – AFAIK, we don't insist on such a `modern' compiler.  So if you really want to use that flag, you have to add gnulib's `fprintf-posix' module to `bootstrap-conf'.

. In groff, single-line ELSE clauses in C or C++ code are not surrounded by braces.

Werner LEMBERG <wl>
Group administrator
Tue 04 Jul 2017 10:29:54 PM UTC, comment #2: 

Yes you are right, libmagic returns a textual description that must be parsed.  Uchardet is really easy to use, here is a draft patch to automatically detects if libuchardet is available with pkg-config, and use it in preconv if available. Not the definitive version though (man doc should be updated, and I also need to check which is the 1st usable version of uchardet, for the moment I put version 0.0.1).


(file #41121)

Bertrand Garrigues <bgarrigues>
Group administrator
Wed 28 Jun 2017 08:58:27 AM UTC, comment #1: 

I like the idea of using a library to guess the charset and encoding.  However, I think that libmagic is not suited to that – as far as I can see, it returns a textual description of the data that preconv had to parse manually.  Please correct me if I'm wrong.

Looking around, the probably best choice is uchardet:

https://www.freedesktop.org/wiki/Software/uchardet/

We could make preconv use it optionally if it is available.

Werner LEMBERG <wl>
Group administrator
Tue 27 Jun 2017 10:31:10 PM UTC, original submission:  

(See also comment #1 from bug #50989)

typesetting.pdf (utf-8 file without BOM; contains some characters with French accents) is not correctly generated in the build tree because LC_ALL=C is passed: this causes `preconv' to use "latin1" as default encoding, which is the expected behaviour according to the man page of `preconv', and therefore characters with accents are not properly handled.

There are several quick fixes to the generation of mom examples:
- Add a BOM to the .mom files.
- Use '-K utf8' instead of just '-k'
- Add a tag to the .mom files.

However it seems to me that `preconv' should not rely on the locale to detect the file encoding.

Would it make sense to use, for example, libmagic (from the `file' utility) to make preconv correctly detect the input file encoding?

Bertrand Garrigues <bgarrigues>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by wl (Posted a comment)
  • -email is unavailable- added by bgarrigues (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-05-02 gbranden CategoryNone Preprocessor preconv
        Planned ReleaseNone 1.22.4
    2017-07-22 bgarrigues Open/ClosedOpen Closed
    2017-07-22 bgarrigues StatusReady for Merge Fixed
    2017-07-12 bgarrigues Attached File- Added 0001-Add-gnulib-module-fprintf-posix.patch, #41186
        Attached File- Added 0002-Use-libchardet-for-preconv.patch, #41187
        Severity3 - Normal 4 - Important
        Item GroupNone Incorrect behaviour
        StatusConfirmed Ready for Merge
    2017-07-04 bgarrigues Attached File- Added 0001-Use-libchardet-for-preconv.patch, #41121

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code