Add jinja templating, payload as tar, tests by lrandersson · Pull Request #2 · lrandersson/constructor

lrandersson · 2026-02-05T16:12:49Z

Description

This PR is a continuation of conda#1164, but split into a separate PR to hopefully simplify review, summarizing the changes:

Add jinja templating for post_install.bat and pre_uninstall.bat (although this file is currently empty).
Move write_pyproject_toml into class Payload
Add new class TemplateFile to better organize files that are templated using jinja.
Add unit tests

Checklist - did you ...

Add a file to the news directory (using the template) for the next release's release notes?
Add / update necessary tests?
Add / update outdated documentation?

lrandersson · 2026-02-05T16:18:25Z

constructor/briefcase.py

+
+        archive_path = dst / self.archive_name
+
+        with tarfile.open(archive_path, mode="w:gz", compresslevel=1) as tar:


I chose tar.gz here over tar.bz2, the reason is that MSI already compresses the contents, therefor the format of the compression does not do much of a difference in terms of disk space. However, .tar.gz is in general a little bit faster over .tar.bz2, so I have intentionally chose speed here over disk space.

Could you add some benchmarks to this?

lrandersson · 2026-02-05T16:18:50Z

constructor/briefcase/run_installation.bat

+set "PAYLOAD_TAR=%INSTDIR%\{{ archive_name }}"

-"%INSTDIR%\_conda.exe" constructor --prefix "%BASE_PATH%" --extract-conda-pkgs
+tar -xzf "%PAYLOAD_TAR%" -C "%INSTDIR%"


This will be updated in the future to instead use conda-standalone instead of built-in tar ...

There's no time like the present!

lrandersson · 2026-02-05T16:19:22Z

constructor/briefcase.py


-# Create a Briefcase configuration file. Using a full TOML writer rather than a Jinja
-# template allows us to avoid escaping strings everywhere.
-def write_pyproject_toml(tmp_dir, info):


This was not deleted, but moved into class Payload

lrandersson · 2026-02-05T16:20:23Z

constructor/template_file.py

+from .jinja import render_template
+
+
+@dataclass(frozen=True)


We can utilize this later also for the other installers.

This seems to overly abstract. The name property appears to be too tailor-made (which other format needs it besides MSI) and then we just have a source and destination path. With all my other comments, I'm not sure we need this class.

The render_template_files function could then just handle one file and deal with all the file handling operations (which I think is nice).

marcoesters

My initial thoughts on this. Overall, I think we could do with a little less abstraction here.

marcoesters · 2026-02-05T22:27:00Z

constructor/briefcase/run_installation.bat

+set "PAYLOAD_TAR=%INSTDIR%\{{ archive_name }}"

-"%INSTDIR%\_conda.exe" constructor --prefix "%BASE_PATH%" --extract-conda-pkgs
+tar -xzf "%PAYLOAD_TAR%" -C "%INSTDIR%"


There's no time like the present!

marcoesters · 2026-02-05T22:28:57Z

constructor/briefcase/run_installation.bat

+
+rem Truncates the payload to 0 bytes
+type nul > "%PAYLOAD_TAR%"


Why not delete it? That zero-byte file doesn't need to be there until the uninstallation.

There should probably also be a comment about why this is needed.

marcoesters · 2026-02-05T22:30:29Z

constructor/briefcase.py

+
+        archive_path = dst / self.archive_name
+
+        with tarfile.open(archive_path, mode="w:gz", compresslevel=1) as tar:


Could you add some benchmarks to this?

marcoesters · 2026-02-06T17:12:40Z

tests/test_briefcase.py

+def test_payload_templates_are_rendered():
+    """Test that templates are rendered when the payload is prepared."""
+
+    def assert_no_jinja_markers(path: Path) -> None:


That function looks simple enough to be inlined.

marcoesters · 2026-02-06T17:21:14Z

constructor/briefcase.py

    root: Path | None = None
+    archive_name: str = "payload.tar.gz"
+    conda_exe_name: str = "_conda.exe"
+    rendered_templates: list[TemplateFile] | None = None


I don't know if we want to make that distinction. Rendered templates are like any other file we add to the installer payload.

marcoesters · 2026-02-06T17:23:09Z

constructor/briefcase.py

+
+        return archive_path
+
+    def _convert_into_archive(self, src: Path, dst: Path) -> Path:


That function could be folded into make_tar_gz or vice versa

marcoesters · 2026-02-06T17:35:28Z

constructor/briefcase.py

+        """Render all configured Jinja templates into the payload root directory.
+        The set of successfully rendered templates is recorded on the instance and returned to the caller.
+        """
+        root = self._ensure_root()


This is what I meant about either using __init__ to create the directory or not allowing a None value/non-existing directory. We have to manually add self._ensure_root() in all appropriate places.

marcoesters · 2026-02-06T17:36:31Z

constructor/briefcase.py

+            },
+        }
+        # Render the template files and add them to the necessary config field
+        rendered_templates = self.render_templates()


I think this should go into prepare like with any other payload preparation. Since we will always have pre_uninstall and pre_install scripts for our installers, we can add those hard-coded into the config object above.

marcoesters · 2026-02-06T17:37:53Z

constructor/template_file.py

@@ -0,0 +1,26 @@
+from collections.abc import Mapping


Let's merge the two template functions into one. There doesn't seem to be enough here to justify a separate module.

marcoesters · 2026-02-06T17:54:45Z

constructor/template_file.py

+from .jinja import render_template
+
+
+@dataclass(frozen=True)


This seems to overly abstract. The name property appears to be too tailor-made (which other format needs it besides MSI) and then we just have a source and destination path. With all my other comments, I'm not sure we need this class.

The render_template_files function could then just handle one file and deal with all the file handling operations (which I think is nice).

lrandersson · 2026-02-06T18:59:00Z

@marcoesters To answer above comments.
The requested benchmarking, using a dummy tar with 800 files of 32kB each, below are the results for different compression levels:

cl=1: 0.225s, size=209.1 KiB
cl=3: 0.215s, size=210.1 KiB
cl=6: 0.264s, size=116.9 KiB
cl=9: 0.285s, size=108.3 KiB

I just pushed a bunch of commits that did change a lot based on the review, I hope to summarize it:

Removed template_file.py, merged the rendering functions into one
Using cached properties now for both Payload.root and Payload.rendered_templates. Let me know what you think, this also removes the call to _ensure_root(). I think this looks pretty good and we don't get side-effects from class instantiation (as mentioned in the other PR).
Updated the scripts run_installation.bat and pre_uninstall.bat, we now call conda-standalone instead of `tar.
Folded/merged the make_tar_gz functionality.
Inlined the test utility function to verify we are free from unrendered jinja tokens.

marcoesters

Thank you! I think it's much closer to being ready now.

marcoesters · 2026-02-06T21:21:02Z

constructor/briefcase.py

+                    "post_install_script": str(self.rendered_templates["post_install_script"].dst),
+                    "pre_uninstall_script": str(self.rendered_templates["pre_uninstall_script"].dst),


Suggested change

"post_install_script": str(self.rendered_templates["post_install_script"].dst),

"pre_uninstall_script": str(self.rendered_templates["pre_uninstall_script"].dst),

"post_install_script": layout.root / "post_install.bat",

"pre_uninstall_script": layout.root / "pre_uninstall.bat",

We know where these files go and that location is not configurable, so we might as well hard-code them here. That also gives layout even more of a purpose and makes the TemplateParameter dataclass obsolete and you don't need to store the template files in the class (the fact that they are templates isn't that important, I think).

Applied here 3327439

marcoesters · 2026-02-06T21:23:22Z

constructor/briefcase.py

+    def make_tar_gz(self, src: Path, dst: Path) -> Path:
+        """Create a .tar.gz of the directory 'src'.


I think it would overall be good see a benchmark of tar.gz vs. tar.bz2 in terms of space and extraction speed, especially compared to the native MSI compression. While registry handling becomes much easier with one file, it would be good to say what the cost is.

Agree, will do that separately.

marcoesters · 2026-02-06T21:24:15Z

constructor/briefcase/run_installation.bat

-"%INSTDIR%\_conda.exe" install --offline --file "%BASE_PATH%\conda-meta\initial-state.explicit.txt" -yp "%BASE_PATH%"
+"%CONDA_EXE%" install --offline --file "%BASE_PATH%\conda-meta\initial-state.explicit.txt" -yp "%BASE_PATH%"
+
+rem Delete the payload to save disk space, a truncated placeholder of 0 bytes is recreated during uninstall


Suggested change

rem Delete the payload to save disk space, a truncated placeholder of 0 bytes is recreated during uninstall

rem Delete the payload to save disk space.

rem A truncated placeholder of 0 bytes is recreated during uninstall

rem because MSI expects the file to be there to clean the registry.

Applied here 3327439

lrandersson · 2026-02-06T21:33:42Z

@marcoesters Applied the fixes here 3327439

Add jinja templating, payload as tar, tests

109693f

lrandersson mentioned this pull request Feb 5, 2026

MSI: Reorganize payload preparation conda/constructor#1164

Open

3 tasks

lrandersson commented Feb 5, 2026

View reviewed changes

Update briefcase.py

5e97190

marcoesters reviewed Feb 6, 2026

View reviewed changes

lrandersson added 5 commits February 6, 2026 13:07

Inline test utility function

2019d4f

Remove payload tar, update pre_uninstall.bat

b80642e

Use conda-standalone for extracting tar

8aebd7c

Merge archive functions, update root as cached property

e4bc141

Remove template_file.py improve handling of templates

313f7f2

lrandersson requested a review from marcoesters February 6, 2026 18:59

lrandersson added 2 commits February 6, 2026 14:17

Add missing .dst

e642b75

Remove compresslevel arg

839e290

marcoesters reviewed Feb 6, 2026

View reviewed changes

Review fixes

3327439

lrandersson requested a review from marcoesters February 6, 2026 21:33


		archive_path = dst / self.archive_name

		with tarfile.open(archive_path, mode="w:gz", compresslevel=1) as tar:


		rem Truncates the payload to 0 bytes
		type nul > "%PAYLOAD_TAR%"


		return archive_path

		def _convert_into_archive(self, src: Path, dst: Path) -> Path:

		"post_install_script": str(self.rendered_templates["post_install_script"].dst),
		"pre_uninstall_script": str(self.rendered_templates["pre_uninstall_script"].dst),

		def make_tar_gz(self, src: Path, dst: Path) -> Path:
		"""Create a .tar.gz of the directory 'src'.

-rem Delete the payload to save disk space, a truncated placeholder of 0 bytes is recreated during uninstall
+rem Delete the payload to save disk space.
+rem A truncated placeholder of 0 bytes is recreated during uninstall
+rem because MSI expects the file to be there to clean the registry.

Conversation

lrandersson commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist - did you ...

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcoesters left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrandersson commented Feb 6, 2026

Uh oh!

marcoesters left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrandersson commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lrandersson commented Feb 5, 2026 •

edited

Loading