Skip to content

Commit 3002371

Browse files
ppisarppisar
committed
Double-quote strings in scalar values only when they look like a number
Commit da8e77d started to quote many YAML scalars just in case they could be mistaken for numbers. This caused quoting on places where not necessary. E.g.: license: module: - "MIT" That was disturbing and broke DNF tests which compare serialized YAML documents as strings. This patch introduces an inteligency into the quoting: Only strings which could be mistaken for a number (1.0 versus "1.0") are quoted now. This approach should restore brevity and provide more stability into the YAML serialization. Note that this patch only corrects overquoting which was introduced in da8e77d commit. Autoquoting of other fields, including thorough tests, will be implemented in subsequent commits. #587
1 parent 8827603 commit 3002371

12 files changed

+228
-144
lines changed

‎modulemd/include/private/modulemd-yaml.h‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,25 @@ mmd_emitter_scalar (yaml_emitter_t *emitter,
619619
GError **error);
620620

621621

622+
/**
623+
* mmd_emitter_scalar_string:
624+
* @emitter: (inout): A libyaml emitter object that is positioned at the start
625+
* of where a scalar will be written.
626+
* @scalar: (in) (nullable): The scalar string to be written. If the string
627+
* looks like a number or is empty or undefined, it will be explicitly quoted.
628+
* @error: (out): A #GError that will return the reason for any error.
629+
*
630+
* Returns: TRUE if the YAML scalar was written successfully. Returns FALSE if
631+
* an error occurred and sets @error appropriately.
632+
*
633+
* Since: 2.15
634+
*/
635+
gboolean
636+
mmd_emitter_scalar_string (yaml_emitter_t *emitter,
637+
const gchar *scalar,
638+
GError **error);
639+
640+
622641
/**
623642
* mmd_emitter_strv:
624643
* @emitter: (inout): A libyaml emitter object positioned at the start of where
@@ -1049,6 +1068,36 @@ skip_unknown_yaml (yaml_parser_t *parser, GError **error);
10491068
#define EMIT_SCALAR(emitter, error, value) \
10501069
EMIT_SCALAR_FULL (emitter, error, value, YAML_PLAIN_SCALAR_STYLE)
10511070

1071+
/**
1072+
* EMIT_SCALAR_STRING:
1073+
* @emitter: (inout): A libyaml emitter object positioned where a scalar
1074+
* belongs in the YAML document.
1075+
* @error: (out): A #GError that will return the reason for an output error.
1076+
* @value: (in): The scalar (string) to be written.
1077+
*
1078+
* Emits a string @value. Using style `YAML_DOUBLE_QUOTED_SCALAR_STYLE` style
1079+
* if the @value is empty or looks like a number. Otherwise, using
1080+
* `YAML_PLAIN_SCALAR_STYLE` style. This autoquoting of number-like
1081+
* strings is in place to prevent other YAML applications from trimming
1082+
* trailing null digits and to force them handle the values as a string (e.g.
1083+
* "1.0" will be serialized as "1.0" instead of 1.0 which some applications
1084+
* interpret as 1. We do not always quote to keep the YAML file concise and
1085+
* similar to previous serialization styles.
1086+
*
1087+
* Returns: Continues on if the YAML scalar was written successfully. Returns
1088+
* FALSE if an error occurred and sets @error appropriately.
1089+
*
1090+
* Since: 2.15
1091+
*/
1092+
#define EMIT_SCALAR_STRING(emitter, error, value) \
1093+
do \
1094+
{ \
1095+
if (!mmd_emitter_scalar_string ((emitter), (value), (error))) \
1096+
return FALSE; \
1097+
} \
1098+
while (0)
1099+
1100+
10521101
/**
10531102
* EMIT_KEY_VALUE_FULL:
10541103
* @emitter: (inout): A libyaml emitter object positioned where a scalar
@@ -1430,10 +1479,7 @@ skip_unknown_yaml (yaml_parser_t *parser, GError **error);
14301479
modulemd_ordered_str_keys (table, modulemd_strcmp_sort); \
14311480
for (i = 0; i < keys->len; i++) \
14321481
{ \
1433-
EMIT_SCALAR_FULL (emitter, \
1434-
error, \
1435-
g_ptr_array_index (keys, i), \
1436-
YAML_DOUBLE_QUOTED_SCALAR_STYLE); \
1482+
EMIT_SCALAR_STRING (emitter, error, g_ptr_array_index (keys, i)); \
14371483
} \
14381484
EMIT_SEQUENCE_END (emitter, error); \
14391485
} \

‎modulemd/modulemd-defaults-v1.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,8 +1124,7 @@ modulemd_defaults_v1_emit_profiles (GHashTable *profile_table,
11241124
continue;
11251125
}
11261126

1127-
if (!mmd_emitter_scalar (
1128-
emitter, stream_name, YAML_DOUBLE_QUOTED_SCALAR_STYLE, error))
1127+
if (!mmd_emitter_scalar_string (emitter, stream_name, error))
11291128
{
11301129
return FALSE;
11311130
}

‎modulemd/modulemd-yaml-util.c‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,45 @@ mmd_emitter_scalar (yaml_emitter_t *emitter,
309309
}
310310

311311

312+
/**
313+
* mmd_string_is_empty_or_a_number
314+
* @string (in) (nullable)
315+
*
316+
* Returns: True if the @string is %NULL, empty, or looks like a decimal
317+
* number. False otherwise (looks like a string).
318+
*
319+
* Since 2.15
320+
*/
321+
static gboolean
322+
string_is_empty_or_a_number (const gchar *string)
323+
{
324+
if (string == NULL)
325+
return TRUE;
326+
if (string[0] == '\0')
327+
return TRUE;
328+
if (string[0] >= '0' && string[0] <= '9')
329+
return TRUE;
330+
if ((string[0] == '+' || string[0] == '-' || string[0] == '.') &&
331+
(string[1] >= '0' && string[1] <= '9'))
332+
return TRUE;
333+
return FALSE;
334+
}
335+
336+
337+
gboolean
338+
mmd_emitter_scalar_string (yaml_emitter_t *emitter,
339+
const gchar *scalar,
340+
GError **error)
341+
{
342+
return mmd_emitter_scalar (emitter,
343+
scalar,
344+
string_is_empty_or_a_number (scalar) ?
345+
YAML_DOUBLE_QUOTED_SCALAR_STYLE :
346+
YAML_PLAIN_SCALAR_STYLE,
347+
error);
348+
}
349+
350+
312351
gboolean
313352
mmd_emitter_strv (yaml_emitter_t *emitter,
314353
yaml_sequence_style_t seq_style,

‎modulemd/tests/ModulemdTests/modulestream.py‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -680,15 +680,15 @@ def test_upgrade_v1_to_v2(self):
680680
Description
681681
license:
682682
module:
683-
- "DUMMY"
683+
- DUMMY
684684
dependencies:
685685
- buildrequires:
686-
ModuleA: ["streamZ"]
687-
ModuleB: ["streamY"]
686+
ModuleA: [streamZ]
687+
ModuleB: [streamY]
688688
requires:
689-
ModuleA: ["streamZ"]
690-
ModuleB: ["streamY"]
691-
platform: ["f33"]
689+
ModuleA: [streamZ]
690+
ModuleB: [streamY]
691+
platform: [f33]
692692
...
693693
""",
694694
)
@@ -1507,14 +1507,14 @@ def test_static_context(self):
15071507
real-time applications that run across distributed devices.
15081508
license:
15091509
module:
1510-
- "MIT"
1510+
- MIT
15111511
content:
1512-
- "DUMMY"
1512+
- DUMMY
15131513
dependencies:
15141514
- buildrequires:
1515-
platform: ["f29"]
1515+
platform: [f29]
15161516
requires:
1517-
platform: ["f29"]
1517+
platform: [f29]
15181518
references:
15191519
community: http://nodejs.org
15201520
documentation: http://nodejs.org/en/docs
@@ -1534,9 +1534,9 @@ def test_static_context(self):
15341534
- nodejs
15351535
api:
15361536
rpms:
1537-
- "nodejs"
1538-
- "nodejs-devel"
1539-
- "npm"
1537+
- nodejs
1538+
- nodejs-devel
1539+
- npm
15401540
components:
15411541
rpms:
15421542
nodejs:
@@ -1547,10 +1547,10 @@ def test_static_context(self):
15471547
buildorder: 10
15481548
artifacts:
15491549
rpms:
1550-
- "nodejs-1:8.11.4-1.module_2030+42747d40.x86_64"
1551-
- "nodejs-devel-1:8.11.4-1.module_2030+42747d40.x86_64"
1552-
- "nodejs-docs-1:8.11.4-1.module_2030+42747d40.noarch"
1553-
- "npm-1:5.6.0-1.8.11.4.1.module_2030+42747d40.x86_64"
1550+
- nodejs-1:8.11.4-1.module_2030+42747d40.x86_64
1551+
- nodejs-devel-1:8.11.4-1.module_2030+42747d40.x86_64
1552+
- nodejs-docs-1:8.11.4-1.module_2030+42747d40.noarch
1553+
- npm-1:5.6.0-1.8.11.4.1.module_2030+42747d40.x86_64
15541554
...
15551555
"""
15561556
self.maxDiff = None

‎modulemd/tests/test-modulemd-build-config.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -747,10 +747,10 @@ buildconfig_test_emit_yaml (void)
747747
"context: CTX1\n"
748748
"platform: f32\n"
749749
"buildrequires:\n"
750-
" appframework: [\"v1\"]\n"
751-
" doctool: [\"rolling\"]\n"
750+
" appframework: [v1]\n"
751+
" doctool: [rolling]\n"
752752
"requires:\n"
753-
" appframework: [\"v2\"]\n"
753+
" appframework: [v2]\n"
754754
"buildopts:\n"
755755
" rpms:\n"
756756
" macros: >-\n"

‎modulemd/tests/test-modulemd-defaults-v1.c‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,7 @@ defaults_test_emit_yaml (void)
632632
" module: foo\n"
633633
" stream: \"latest\"\n"
634634
" profiles:\n"
635-
" \"libonly\": []\n"
635+
" libonly: []\n"
636636
"...\n");
637637

638638
/* Add a real profile default and emit again */
@@ -653,8 +653,8 @@ defaults_test_emit_yaml (void)
653653
" module: foo\n"
654654
" stream: \"latest\"\n"
655655
" profiles:\n"
656-
" \"latest\": [bar]\n"
657-
" \"libonly\": []\n"
656+
" latest: [bar]\n"
657+
" libonly: []\n"
658658
"...\n");
659659

660660
/* Add another real profile default to the same stream and emit again */
@@ -675,8 +675,8 @@ defaults_test_emit_yaml (void)
675675
" module: foo\n"
676676
" stream: \"latest\"\n"
677677
" profiles:\n"
678-
" \"latest\": [bar, baz]\n"
679-
" \"libonly\": []\n"
678+
" latest: [bar, baz]\n"
679+
" libonly: []\n"
680680
"...\n");
681681

682682

@@ -697,8 +697,8 @@ defaults_test_emit_yaml (void)
697697
" module: foo\n"
698698
" stream: \"latest\"\n"
699699
" profiles:\n"
700-
" \"latest\": [bar, baz]\n"
701-
" \"libonly\": []\n"
700+
" latest: [bar, baz]\n"
701+
" libonly: []\n"
702702
" intents:\n"
703703
" intense:\n"
704704
" stream: \"earliest\"\n"
@@ -722,13 +722,13 @@ defaults_test_emit_yaml (void)
722722
" module: foo\n"
723723
" stream: \"latest\"\n"
724724
" profiles:\n"
725-
" \"latest\": [bar, baz]\n"
726-
" \"libonly\": []\n"
725+
" latest: [bar, baz]\n"
726+
" libonly: []\n"
727727
" intents:\n"
728728
" intense:\n"
729729
" stream: \"earliest\"\n"
730730
" profiles:\n"
731-
" \"earliest\": [client]\n"
731+
" earliest: [client]\n"
732732
"...\n");
733733
}
734734

‎modulemd/tests/test-modulemd-dependencies.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -511,10 +511,10 @@ dependencies_test_emit_yaml (void)
511511
"---\n"
512512
"- buildrequires:\n"
513513
" builddef: []\n"
514-
" buildmod1: [\"stream1\", \"stream2\"]\n"
514+
" buildmod1: [stream1, stream2]\n"
515515
" requires:\n"
516516
" rundef: []\n"
517-
" runmod1: [\"stream3\", \"stream4\"]\n"
517+
" runmod1: [stream3, stream4]\n"
518518
"...\n");
519519

520520

@@ -546,7 +546,7 @@ dependencies_test_emit_yaml (void)
546546
"---\n"
547547
"- buildrequires:\n"
548548
" builddef: []\n"
549-
" buildmod1: [\"stream1\", \"stream2\"]\n"
549+
" buildmod1: [stream1, stream2]\n"
550550
"...\n");
551551

552552
/* Test with only adding a runtime requires */
@@ -578,7 +578,7 @@ dependencies_test_emit_yaml (void)
578578
"---\n"
579579
"- requires:\n"
580580
" rundef: []\n"
581-
" runmod1: [\"stream3\", \"stream4\"]\n"
581+
" runmod1: [stream3, stream4]\n"
582582
"...\n");
583583
}
584584

‎modulemd/tests/test-modulemd-moduleindex.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ module_index_test_dump (void)
167167
" A test stream's description\n"
168168
" license:\n"
169169
" module:\n"
170-
" - \"DUMMY1\"\n"
170+
" - DUMMY1\n"
171171
"...\n"
172172
"---\n"
173173
"document: modulemd\n"
@@ -182,7 +182,7 @@ module_index_test_dump (void)
182182
" A second stream's description\n"
183183
" license:\n"
184184
" module:\n"
185-
" - \"DUMMY2\"\n"
185+
" - DUMMY2\n"
186186
"...\n");
187187
}
188188

0 commit comments

Comments
 (0)