Fixed bug when using custom_extensions and initials together, made lo…

…ckfile alphabetical to prevent annoying git diffs when nothing changed (#13)

py_rust/src/config/engine.rs

@@ -199,14 +199,13 @@ impl Engine {
                 Ok::<_, error_stack::Report<Zerr>>(())
             })?;
 
-            // Consume current contents of PY_USER_FUNCS and add to minijinja env:
-            let mut custom_funcs_global = PY_USER_FUNCS.lock();
-
-            // Extract all the loaded funcs from the global mutex to be passed to individual closures:
-            let custom_funcs = std::mem::take(&mut *custom_funcs_global);
-            *custom_funcs_global = HashMap::new();
-
+            // Extract a copy of the user funcs to add to minijinja env:
+            // Note copying as env might be created multiple times (e.g. initial)
+            // TODO: instead of this maybe reusing an env? In general need a bit of a refactor!
+            let custom_funcs = PY_USER_FUNCS.lock().clone();
             for (name, py_fn) in custom_funcs.into_iter() {
+                debug!("Registering custom function: '{}'", name);
+
                 // Confirm doesn't clash with config var:
                 if conf.context.contains_key(&name) {
                     return Err(zerr!(

py_rust/src/render/lockfile.rs

@@ -15,6 +15,8 @@ pub static LOCKFILE_NAME: &str = ".zetch.lock";
 #[derive(Debug, serde::Serialize, serde::Deserialize)]
 struct Contents {
     version: String,
+    // Keep ordering in lockfile static to reduce git conflicts and diff noise:
+    #[serde(serialize_with = "crate::utils::ordered_map_serializer")]
     files: HashMap<String, String>,
 }
 

py_rust/src/render/mod.rs

@@ -4,7 +4,6 @@ use bitbazaar::{
 };
 use colored::Colorize;
 use minijinja::context;
-use tracing::{debug, info, warn};
 
 mod args_validate;
 mod debug;
@@ -81,7 +80,7 @@ pub fn render(args: &crate::args::Args, render_args: &RenderCommand) -> Result<b
             .change_context(Zerr::InternalError)?;
     }
 
-    info!(
+    println!(
         "{} {} template{} written, {} identical. Lockfile {}. {} elapsed.",
         "zetch:".bold(),
         written.len(),

py_rust/src/utils/mod.rs

@@ -1 +1,3 @@
+mod ordered_map_serializer;
 pub mod user_input;
+pub use ordered_map_serializer::ordered_map_serializer;

py_rust/src/utils/ordered_map_serializer.rs

@@ -0,0 +1,19 @@
+use std::collections::{BTreeMap, HashMap};
+
+use serde::{Serialize, Serializer};
+
+/// For use with serde's [serialize_with] attribute.
+///
+/// Will make a hashmap alphabetically ordered by key on serialization.
+///
+/// Source: https://stackoverflow.com/questions/42723065/how-to-sort-hashmap-keys-when-serializing-with-serde
+pub fn ordered_map_serializer<S, K: Ord + Serialize, V: Serialize>(
+    value: &HashMap<K, V>,
+    serializer: S,
+) -> Result<S::Ok, S::Error>
+where
+    S: Serializer,
+{
+    let ordered: BTreeMap<_, _> = value.iter().collect();
+    ordered.serialize(serializer)
+}

py_rust/tests/render/test_custom_extensions.py

@@ -411,6 +411,41 @@ def test_custom_ext_nice_user_invalid_errs(
             )
 
 
+def test_custom_ext_multi_render():
+    """Check custom extensions don't break when renderer runs multiple times (e.g. when cli var initials used).
+
+    This test was made for a real bug where extensions were lost after the first renderer usage.
+    """
+    with TmpFileManager() as manager:
+        ext = manager.tmpfile(
+            """import zetch
+@zetch.register_function
+def capitalize(s):
+    return s.capitalize()
+""",
+            suffix=".py",
+        )
+
+        check_single(
+            manager,
+            manager.create_cfg(
+                {
+                    "engine": {"custom_extensions": [str(ext)]},
+                    "context": {
+                        "cli": {
+                            "var_with_initial": {
+                                "commands": ["echo bar"],
+                                "initial": "init",
+                            }
+                        }
+                    },
+                }
+            ),
+            "{{ capitalize('foo') }} {{ var_with_initial }}",
+            "Foo bar",
+        )
+
+
 # DONE duplicate pkg names/sys paths
 # DONE conflict with ctx/in built filter/in built function
 # DONE check module and importing between files works
@@ -434,7 +469,8 @@ def test_custom_ext_nice_user_invalid_errs(
 # DONE allow input as string instead of file, that should print to stdout for read, write, del
 # DONE probably remove most engine config, maybe making top level if minimal enough, we don't want to mess with files and error early (so enforce no_undefined and keep_trailing_newline)
 # DONE fix schema - not sure why its not working
-# TODO: order everything in the lockfile to prevent diffs when nothings actually changed.
+# DONE: order everything in the lockfile to prevent diffs when nothings actually changed.
+# TODO: Before thinking about modes/light etc as other solutions to circular deps. I think by default the repeated rendering should be improved to be a first class citizen, then, every run, static+env rendered in first + initials when not existing in lockfile + others clis as empty strings + custom extensions as empty strings, only then a second render with cli commands, this could get rid of a bunch of circular dep problems natively.
 # TODO some sort of heavy/light/modes solution to caching values and not recomputing, maybe also for ban-defaults etc. maybe a modes top level config section, where a mode can override any config, set ban-defaults etc, need to think, but also need a way to only run certain post and pre in certain modes, need to think on best api.
 # TODO think about interop with jinja,cookiecutter,copier,etc
 # TODO decide and document optimal formatting, probably using scolvins and making sure it can working with custom extensions.

py_rust/tests/render/test_lockfile.py

@@ -1,4 +1,6 @@
+import collections
 import json
+import uuid
 from pathlib import Path
 
 import pytest
@@ -155,3 +157,27 @@ def test_lockfile_only_write_when_needed():
                     ),
                 },
             }
+
+
+def test_lockfile_deterministic_ordering():
+    """Make sure the lockfile always serializes object keys in alphabetical order.
+
+    This makes git diffs on the lockfile much less likely to conflict, and only show changes when real.
+    """
+    with TmpFileManager() as manager:
+        filenames = [
+            str(manager.tmpfile(content="Hello!", full_name=f"{uuid.uuid4()}.zetch.txt").name)
+            for _ in range(10)
+        ]
+        cli.render(
+            manager.root_dir,
+            manager.create_cfg({}),
+        )
+        with open(get_lockfile_path(manager.root_dir), "r") as file:
+            # In cur python think will be ordered by default, but older pythons might not be,
+            # so use ordered dict in json decoder to be sure order unaffected by loading into python:
+            lock = json.load(file, object_pairs_hook=collections.OrderedDict)
+            assert lock["version"] == zetch.__version__
+
+            # They should come out in alphabetical order:
+            assert [k for k in lock["files"]] == sorted(filenames)