Fix issue #204 (asdf/core metadata order in the tree)

include/asdf/value.h

@@ -296,6 +296,14 @@ typedef struct {
  */
 ASDF_EXPORT asdf_mapping_iter_t *asdf_mapping_iter_init(asdf_mapping_t *mapping);
 
+/**
+ * Create a new reverse iterator over ``mapping``
+ *
+ * :param mapping: The `asdf_mapping_t *` to iterate over
+ * :return: A new `asdf_mapping_iter_t *` handle, or ``NULL`` on allocation failure
+ */
+ASDF_EXPORT asdf_mapping_iter_t *asdf_mapping_reverse_iter_init(asdf_mapping_t *mapping);
+
 /**
  * Advance the iterator to the next mapping entry
  *
@@ -544,6 +552,14 @@ typedef struct {
  */
 ASDF_EXPORT asdf_sequence_iter_t *asdf_sequence_iter_init(asdf_sequence_t *sequence);
 
+/**
+ * Create a new reverse iterator over ``sequence``
+ *
+ * :param sequence: The `asdf_sequence_t *` to iterate over
+ * :return: A new `asdf_sequence_iter_t *` handle, or ``NULL`` on allocation failure
+ */
+ASDF_EXPORT asdf_sequence_iter_t *asdf_sequence_reverse_iter_init(asdf_sequence_t *sequence);
+
 /**
  * Advance the iterator to the next sequence element
  *
@@ -714,6 +730,16 @@ typedef struct {
  */
 ASDF_EXPORT asdf_container_iter_t *asdf_container_iter_init(asdf_value_t *container);
 
+/**
+ * Create a new reverse iterator over ``container`` (mapping or sequence)
+ *
+ * Returns ``NULL`` if ``container`` is neither a mapping nor a sequence.
+ *
+ * :param container: `asdf_value_t *` of mapping or sequence type
+ * :return: A new `asdf_container_iter_t *` handle, or ``NULL`` on failure
+ */
+ASDF_EXPORT asdf_container_iter_t *asdf_container_reverse_iter_init(asdf_value_t *container);
+
 /**
  * Advance the generic container iterator to the next element
  *

src/emitter.c

@@ -298,6 +298,7 @@ static int emit_prepare_root_node(asdf_emitter_t *emitter) {
     if (!meta)
         goto cleanup;
 
+    // Replace the original meta->asdf_library, if any
     asdf_software_destroy(meta->asdf_library);
 
     if (asdf_emitter_has_opt(emitter, ASDF_EMITTER_OPT_NO_EMIT_ASDF_LIBRARY)) {
@@ -314,6 +315,14 @@ static int emit_prepare_root_node(asdf_emitter_t *emitter) {
     // that ought to be avoided somehow.  Either change these to use STC
     // vectors or at least an internal sized array type...
     if (file->history_entries && *file->history_entries) {
+        // NOTE: If the history entries were added *before* a call to
+        // asdf_library_set_version then these history entries may show the
+        // "wrong", non-overridden version.  In principle should fix up their
+        // attached software version as is done above with meta->asdf_library.
+        // We opt for now not to go out of our way to fix this, because
+        // overriding the library version is only needed for debug/tests.  The
+        // workaround is to set the version override *before* adding any history
+        // entries.
         meta->history.entries = (const asdf_history_entry_t **)asdf_array_concat(
             (void **)meta->history.entries, (const void **)file->history_entries);
 
@@ -340,7 +349,7 @@ static int emit_prepare_root_node(asdf_emitter_t *emitter) {
     if (asdf_get_mapping(file, "/", &root) != ASDF_VALUE_OK)
         goto cleanup;
 
-    if (asdf_mapping_update(root, meta_map) != ASDF_VALUE_OK)
+    if (asdf_mapping_update_ex(root, meta_map, true) != ASDF_VALUE_OK)
         goto cleanup;
 
     ret = 0;

src/value.c

@@ -495,8 +495,8 @@ void asdf_mapping_destroy(asdf_mapping_t *mapping) {
 
 
 /** Helper for asdf_mapping_set_* methods */
-static inline asdf_value_err_t asdf_mapping_set_node(
-    asdf_mapping_t *mapping, const char *key, struct fy_node *value) {
+static inline asdf_value_err_t asdf_mapping_set_node_ex(
+    asdf_mapping_t *mapping, const char *key, struct fy_node *value, bool prepend) {
     if (!mapping)
         return ASDF_VALUE_ERR_UNKNOWN;
 
@@ -518,15 +518,28 @@ static inline asdf_value_err_t asdf_mapping_set_node(
         if (fy_node_pair_set_value(pair, value) != 0) {
             return ASDF_VALUE_ERR_OOM;
         }
-    } else if (fy_node_mapping_append(mapping->value.node, key_node, value) != 0) {
-        fy_node_free(key_node);
-        return ASDF_VALUE_ERR_OOM;
+    } else if (prepend) {
+        if (fy_node_mapping_prepend(mapping->value.node, key_node, value) != 0) {
+            fy_node_free(key_node);
+            return ASDF_VALUE_ERR_OOM;
+        }
+    } else {
+        if (fy_node_mapping_append(mapping->value.node, key_node, value) != 0) {
+            fy_node_free(key_node);
+            return ASDF_VALUE_ERR_OOM;
+        }
     }
 
     return ASDF_VALUE_OK;
 }
 
 
+static inline asdf_value_err_t asdf_mapping_set_node(
+    asdf_mapping_t *mapping, const char *key, struct fy_node *value) {
+    return asdf_mapping_set_node_ex(mapping, key, value, false);
+}
+
+
 asdf_value_err_t asdf_mapping_set_string(
     // NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
     asdf_mapping_t *mapping,
@@ -588,8 +601,10 @@ asdf_value_err_t asdf_mapping_set_null(asdf_mapping_t *mapping, const char *key)
     }
 
 
-asdf_value_err_t asdf_mapping_set(asdf_mapping_t *mapping, const char *key, asdf_value_t *value) {
-    asdf_value_err_t err = asdf_mapping_set_node(mapping, key, asdf_value_normalize_node(value));
+static asdf_value_err_t asdf_mapping_set_ex(
+    asdf_mapping_t *mapping, const char *key, asdf_value_t *value, bool prepend) {
+    asdf_value_err_t err = asdf_mapping_set_node_ex(
+        mapping, key, asdf_value_normalize_node(value), prepend);
     /* fy_node_mapping_append implicitly frees the original node, so here set it
      * to null to avoid double-freeing it and then just destroy the asdf_value_t */
     value->node = NULL;
@@ -598,6 +613,11 @@ asdf_value_err_t asdf_mapping_set(asdf_mapping_t *mapping, const char *key, asdf
 }
 
 
+asdf_value_err_t asdf_mapping_set(asdf_mapping_t *mapping, const char *key, asdf_value_t *value) {
+    return asdf_mapping_set_ex(mapping, key, value, false);
+}
+
+
 #define ASDF_MAPPING_SET_CONTAINER_TYPE(type) \
     asdf_value_err_t asdf_mapping_set_##type( \
         asdf_mapping_t *mapping, const char *key, asdf_##type##_t *value) { \
@@ -620,7 +640,7 @@ ASDF_MAPPING_SET_CONTAINER_TYPE(mapping);
 ASDF_MAPPING_SET_CONTAINER_TYPE(sequence);
 
 
-asdf_mapping_iter_t *asdf_mapping_iter_init(asdf_mapping_t *mapping) {
+static asdf_mapping_iter_t *asdf_mapping_iter_init_ex(asdf_mapping_t *mapping, bool reverse) {
     asdf_mapping_iter_impl_t *impl = calloc(1, sizeof(asdf_mapping_iter_impl_t));
 
     if (!impl) {
@@ -629,10 +649,21 @@ asdf_mapping_iter_t *asdf_mapping_iter_init(asdf_mapping_t *mapping) {
     }
 
     impl->mapping = mapping;
+    impl->reverse = reverse;
     return (asdf_mapping_iter_t *)impl;
 }
 
 
+asdf_mapping_iter_t *asdf_mapping_iter_init(asdf_mapping_t *mapping) {
+    return asdf_mapping_iter_init_ex(mapping, false);
+}
+
+
+asdf_mapping_iter_t *asdf_mapping_reverse_iter_init(asdf_mapping_t *mapping) {
+    return asdf_mapping_iter_init_ex(mapping, true);
+}
+
+
 void asdf_mapping_iter_destroy(asdf_mapping_iter_t *iter) {
     if (!iter)
         return;
@@ -661,9 +692,15 @@ bool asdf_mapping_iter_next(asdf_mapping_iter_t **iter_ptr) {
         goto cleanup;
     }
 
-    struct fy_node_pair *pair = fy_node_mapping_iterate(mapping->value.node, &impl->fy_iter);
+    struct fy_node_pair *pair = NULL;
 
-    if (!pair)
+    if (impl->reverse) {
+        pair = fy_node_mapping_reverse_iterate(mapping->value.node, &impl->fy_iter);
+    } else {
+        pair = fy_node_mapping_iterate(mapping->value.node, &impl->fy_iter);
+    }
+
+    if (UNLIKELY(!pair))
         goto cleanup;
 
     struct fy_node *key_node = fy_node_pair_key(pair);
@@ -702,10 +739,20 @@ bool asdf_mapping_iter_next(asdf_mapping_iter_t **iter_ptr) {
 
 
 // NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
-asdf_value_err_t asdf_mapping_update(asdf_mapping_t *mapping, asdf_mapping_t *update) {
-    asdf_mapping_iter_t *iter = asdf_mapping_iter_init(update);
+asdf_value_err_t asdf_mapping_update_ex(
+    asdf_mapping_t *mapping, asdf_mapping_t *update, bool prepend) {
+    asdf_mapping_iter_t *iter = NULL;
+
+    if (prepend) {
+        // Reverse iterate so each item gets prepended in their original order
+        iter = asdf_mapping_reverse_iter_init(update);
+    } else {
+        iter = asdf_mapping_iter_init(update);
+    }
+
     if (!iter)
         return ASDF_VALUE_ERR_OOM;
+
     asdf_value_err_t err = ASDF_VALUE_OK;
 
     while (asdf_mapping_iter_next(&iter)) {
@@ -714,7 +761,7 @@ asdf_value_err_t asdf_mapping_update(asdf_mapping_t *mapping, asdf_mapping_t *up
             asdf_mapping_iter_destroy(iter);
             return ASDF_VALUE_ERR_OOM;
         }
-        err = asdf_mapping_set(mapping, iter->key, clone);
+        err = asdf_mapping_set_ex(mapping, iter->key, clone, prepend);
         if (err) {
             asdf_mapping_iter_destroy(iter);
             break;
@@ -725,6 +772,11 @@ asdf_value_err_t asdf_mapping_update(asdf_mapping_t *mapping, asdf_mapping_t *up
 }
 
 
+asdf_value_err_t asdf_mapping_update(asdf_mapping_t *mapping, asdf_mapping_t *update) {
+    return asdf_mapping_update_ex(mapping, update, false);
+}
+
+
 asdf_value_t *asdf_mapping_pop(asdf_mapping_t *mapping, const char *key) {
     if (UNLIKELY(!mapping || !key))
         return NULL;
@@ -840,7 +892,7 @@ void asdf_sequence_destroy(asdf_sequence_t *sequence) {
 }
 
 
-asdf_sequence_iter_t *asdf_sequence_iter_init(asdf_sequence_t *sequence) {
+asdf_sequence_iter_t *asdf_sequence_iter_init_ex(asdf_sequence_t *sequence, bool reverse) {
     asdf_sequence_iter_impl_t *impl = calloc(1, sizeof(asdf_sequence_iter_impl_t));
 
     if (UNLIKELY(!impl)) {
@@ -849,11 +901,28 @@ asdf_sequence_iter_t *asdf_sequence_iter_init(asdf_sequence_t *sequence) {
     }
 
     impl->sequence = sequence;
-    impl->pub.index = -1;
+    impl->reverse = reverse;
+
+    if (reverse) {
+        impl->pub.index = asdf_sequence_size(sequence);
+    } else {
+        impl->pub.index = -1;
+    }
+
     return (asdf_sequence_iter_t *)impl;
 }
 
 
+asdf_sequence_iter_t *asdf_sequence_iter_init(asdf_sequence_t *sequence) {
+    return asdf_sequence_iter_init_ex(sequence, false);
+}
+
+
+asdf_sequence_iter_t *asdf_sequence_reverse_iter_init(asdf_sequence_t *sequence) {
+    return asdf_sequence_iter_init_ex(sequence, true);
+}
+
+
 void asdf_sequence_iter_destroy(asdf_sequence_iter_t *iter) {
     if (!iter)
         return;
@@ -882,12 +951,23 @@ bool asdf_sequence_iter_next(asdf_sequence_iter_t **iter_ptr) {
         goto cleanup;
     }
 
-    struct fy_node *value_node = fy_node_sequence_iterate(sequence->value.node, &impl->fy_iter);
+    struct fy_node *value_node = NULL;
+
+    if (impl->reverse) {
+        value_node = fy_node_sequence_reverse_iterate(sequence->value.node, &impl->fy_iter);
+    } else {
+        value_node = fy_node_sequence_iterate(sequence->value.node, &impl->fy_iter);
+    }
 
     if (UNLIKELY(!value_node))
         goto cleanup;
 
-    impl->pub.index++;
+    if (impl->reverse) {
+        impl->pub.index--;
+    } else {
+        impl->pub.index++;
+    }
+
     asdf_value_t *value = asdf_value_create_ex(
         sequence->value.file, value_node, &sequence->value, NULL, impl->pub.index);
 
@@ -1205,7 +1285,7 @@ asdf_value_t *asdf_sequence_pop(asdf_sequence_t *sequence, int index) {
 
 
 /** Generic container functions */
-asdf_container_iter_t *asdf_container_iter_init(asdf_value_t *container) {
+asdf_container_iter_t *asdf_container_iter_init_ex(asdf_value_t *container, bool reverse) {
     if (!container)
         return NULL;
 
@@ -1226,16 +1306,25 @@ asdf_container_iter_t *asdf_container_iter_init(asdf_value_t *container) {
 
     impl->container = container;
     impl->is_mapping = (container->raw_type == ASDF_VALUE_MAPPING);
-    impl->pub.index = -1;
+
+    if (reverse) {
+        if (impl->is_mapping) {
+            impl->pub.index = asdf_mapping_size((asdf_mapping_t *)container);
+        } else {
+            impl->pub.index = asdf_sequence_size((asdf_sequence_t *)container);
+        }
+    } else {
+        impl->pub.index = -1;
+    }
 
     if (impl->is_mapping) {
-        impl->iter.mapping = asdf_mapping_iter_init((asdf_mapping_t *)container);
+        impl->iter.mapping = asdf_mapping_iter_init_ex((asdf_mapping_t *)container, reverse);
         if (!impl->iter.mapping) {
             free(impl);
             return NULL;
         }
     } else {
-        impl->iter.sequence = asdf_sequence_iter_init((asdf_sequence_t *)container);
+        impl->iter.sequence = asdf_sequence_iter_init_ex((asdf_sequence_t *)container, reverse);
         if (!impl->iter.sequence) {
             free(impl);
             return NULL;
@@ -1246,6 +1335,16 @@ asdf_container_iter_t *asdf_container_iter_init(asdf_value_t *container) {
 }
 
 
+asdf_container_iter_t *asdf_container_iter_init(asdf_value_t *container) {
+    return asdf_container_iter_init_ex(container, false);
+}
+
+
+asdf_container_iter_t *asdf_container_reverse_iter_init(asdf_value_t *container) {
+    return asdf_container_iter_init_ex(container, true);
+}
+
+
 void asdf_container_iter_destroy(asdf_container_iter_t *iter) {
     if (!iter)
         return;
@@ -1273,8 +1372,13 @@ bool asdf_container_iter_next(asdf_container_iter_t **iter_ptr) {
             goto cleanup;
 
         impl->pub.key = impl->iter.mapping->key;
-        impl->pub.index++;
         impl->pub.value = impl->iter.mapping->value;
+
+        if (((asdf_mapping_iter_impl_t *)impl->iter.mapping)->reverse) {
+            impl->pub.index--;
+        } else {
+            impl->pub.index++;
+        }
         return true;
     }