Ticket 2791 - UTF-8 string handling support
Summary: UTF-8 string handling support
Status: OPEN
Alias: None
Product: Slurm
Classification: Unclassified
Component: Contributions (show other tickets)
Version: 17.02.x
Hardware: Linux Linux
: 5 - Enhancement
Assignee: Unassigned Developer
QA Contact:
URL:
Depends on:
Blocks:
 
Reported: 2016-06-02 05:00 MDT by Moe Jette
Modified: 2017-09-08 09:41 MDT (History)
1 user (show)

See Also:
Site: SchedMD
Slinky Site: ---
Alineos Sites: ---
Atos/Eviden Sites: ---
Confidential Site: ---
Coreweave sites: ---
Cray Sites: ---
DS9 clusters: ---
Google sites: ---
HPCnow Sites: ---
HPE Sites: ---
IBM Sites: ---
NOAA SIte: ---
NoveTech Sites: ---
Nvidia HWinf-CS Sites: ---
OCF Sites: ---
Recursion Pharma Sites: ---
SFW Sites: ---
SNIC sites: ---
Tzag Elita Sites: ---
Linux Distro: ---
Machine Name:
CLE Version:
Version Fixed:
Target Release: 18.08
DevPrio: ---
Emory-Cloud Sites: ---


Attachments
Handle column width when printing UTF-8 (11.72 KB, patch)
2017-06-12 04:31 MDT, Janne Blomqvist
Details | Diff

Note You need to log in before you can comment on or make changes to this ticket.
Description Moe Jette 2016-06-02 05:00:21 MDT
On 2016-06-01 15:25, Loris Bennett wrote:

With Slurm 15.08.8, sreport does not handle non-ASCII characters in the
'Proper Name' column properly:

Top 3 Users 2016-05-31T00:00:00 - 2016-05-31T23:59:59 (86400 secs)
Use reported in TRES Minutes
--------------------------------------------------------------------------------

  Cluster     Login     Proper Name         Account     Used Energy
--------- --------- --------------- --------------- -------- --------
  soroban    albert        Einstein         physics 230400        0
  soroban     erwin    Schrödinger         physics 200569        0
  soroban    werner      Heisenberg         physics 139680        0
--------------------------------------------------------------------------------


The presence of the umlaut in 'Schrödinger' causes the name to be
justified incorrectly.  In addition, all the lines from the line with
the column names to the final line of data have an additional space at
the end of the line.

The terminal space is not much of a problem, but it would be nice if
justification problem could be fixed.
Comment 1 Moe Jette 2016-06-02 05:01:09 MDT
On 2016-06-01, Tim Wickberg wrote:

On I've looked into this, and it's not as bad as it seems at first glance. Enhancement bug 2265 covers some of this thinking, from the standpoint of preventing malicious junk from wrecking other users' (or sysadmins') terminals by feeding control characters into job names.

wchar_t is considered to have been a mistake by a lot of people. [1] char32_t is not much better - there are now Unicode sequences that don't fix 32-bits now.

UTF8 is almost always easier to work with - most of the same char routines will naively handle this (and Slurm already naively supports UTF8 through this mechanism - hence my occasional testing of a node named ☃) - with the exception of multi-byte characters being inadvertently truncated when using fixed-width output formatting.

"Proper" UTF8 support wouldn't be too difficult to handle - for each string it would require calculating the display-width versus characters used, and also need to prevent splitting multi-byte characters when truncating any strings. The backend code doesn't need to change (with the possible exception of some debug messages) since slurmctld leaves the full character array intact.

Since most of the output formatting routines are shared, it may not be hard to add, although we'd need to import a library ([2] seems like a good option) to give us a way to calculate utf8 string-widths.

[1] http://utf8everywhere.org/
[2] https://github.com/JuliaLang/utf8proc
Comment 2 Janne Blomqvist 2017-06-09 13:28:49 MDT
FWIW, I have a preliminary patch for this. I'll clean it up a bit and wire up more of the user commands (so far I have tested only squeue, specifically the user comment field) before submitting a proper patch.
Comment 3 Moe Jette 2017-06-09 13:30:58 MDT
I've change this bug to severity 4 / contribution

Please attach your patch when you are ready to share it.
Comment 4 Janne Blomqvist 2017-06-12 04:31:47 MDT
Created attachment 4752 [details]
Handle column width when printing UTF-8

Here's the patch, in git format-patch format including a hopefully useful commit msg.

I haven't used any external libraries except functionality which should be present in any C99/POSIX.1-2001 libc. Since AFAIU this is more or less required for slurm in general, I haven't done any autoconf tests for the functions either and just assume that they are present.

This uses the much-maligned wchar_t, but only in the internals of a few helper functions, and thus does not pollute the slurm API.
Comment 6 Janne Blomqvist 2017-06-27 06:38:50 MDT
ping!?
Comment 7 Tim Wickberg 2017-06-27 19:14:26 MDT
I've looked at it briefly, but as you'd correctly surmised I'm not especially enthused by the use of wchar_t and reliance on <locale.h>. But I haven't come up with a better explanation than "it feels wrong". Given the rest of our team's silence on this I think they're in the same category currently.

My current preference would be to use libutf8proc or similar, and use it if available at configure time (then fall back to a naive strlen call if unavailable). I think that could be used with a bit of care along with a modified version of your patch if you're interested in exploring that.
Comment 8 Tim Wickberg 2017-08-01 19:38:01 MDT
Updating some meta records here, and moving back to an enhancement request for now.