Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Set CHEZMOI_SOURCE_FILE env var for scripts #3518

Merged
merged 1 commit into from
Jan 28, 2024
Merged

feat: Set CHEZMOI_SOURCE_FILE env var for scripts #3518

merged 1 commit into from
Jan 28, 2024

Conversation

twpayne
Copy link
Owner

@twpayne twpayne commented Jan 27, 2024

Fixes #2934. It only took nine months.

@VorpalBlade this sets the $CHEZMOI_SOURCE_FILE environment variable for modify_ scripts (and also normal scripts), which should be useful for your excellent chezmoi_modify_manager.

As background info, this change became possible after 7203d6b (in chezmoi v2.40.3).

Would you be able to test this?

Copy link
Collaborator

@halostatue halostatue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable, but I have not tested it.

@VorpalBlade
Copy link
Contributor

I will try: I'm down with a nasty cold at the moment, so it may take a few days before I have energy to deal with this.

It will be interesting to see if it speeds up chezmoi diff (currently a no-op one takes about 85 ms on my desktop iirc, and I have run out of ways to speed it up on my side, a single chezmoi_modify_manager invocation is about 2-3 ms)

@twpayne
Copy link
Owner Author

twpayne commented Jan 27, 2024

No rush at all and wishing you a speedy recovery!

@VorpalBlade
Copy link
Contributor

VorpalBlade commented Jan 27, 2024

I actually got the energy to give this a try now. The mechanism in itself doesn't really offer any speed-up (as expected).

But this means that every modify_ script that doesn't otherwise need templating can now be a non-templated one. That does offer a speedup. In my own chezmoi source state I could convert 16 (out of 20) modify scripts to no longer be templates.

The runtime of ./chezmoi diff (binary built with "go build" in the branch of this PR) went from

$ # With lookup based on env vars but still .tmpl files
$ hyperfine "./chezmoi diff"
Benchmark 1: ./chezmoi diff
  Time (mean ± σ):      87.2 ms ±   1.6 ms    [User: 72.5 ms, System: 32.4 ms]
  Range (min … max):    84.9 ms …  92.8 ms    33 runs

$ # After converting to non-.tmpl files
$ hyperfine "./chezmoi diff"
Benchmark 1: ./chezmoi diff
  Time (mean ± σ):      78.5 ms ±   0.8 ms    [User: 65.7 ms, System: 27.8 ms]
  Range (min … max):    76.9 ms …  80.2 ms    37 runs

That is ~90% of the original time, not bad. (Another benefit is not needing the complicated templated line [as shown below] in the modify script.)

(The difference between .tmpl with source "{{ .chezmoi.sourceDir }}/{{ .chezmoi.sourceFile | trimSuffix ".tmpl" | replace "modify_" "" }}.src.ini" and source auto while still a .tmpl file was not measurable let alone statistically significant.)

Further improvement idea

Do you still always make a copy of the modify_ scripts into /tmp? I bet avoiding that could save a bunch of time, though it would require setting chmod +x on the script in the source directory (this is fine, git tracks +x file attribute).

To support legacy scripts you would need to support copying if required though I guess. Might still be worth detecting the file modes via a stat/lstat/fstat/fstatat() syscall. (Or if you want to be fancy and not fetch more than you need I believe there are newer selective syscalls, including allowing you to get the data at directory traversal time) I assume that would be quicker than copying the file, but you would need to benchmark to be sure of course.

@VorpalBlade
Copy link
Contributor

VorpalBlade commented Jan 27, 2024

(Another note, I found it slightly annoying that I need to combine CHEZMOI_SOURCE_DIR and CHEZMOI_SOURCE_FILE to get the path. But if you don't already have that combined on your side for other purposes, it is probably faster to do the string manipulation in the rust code on my side anyway.)

@twpayne
Copy link
Owner Author

twpayne commented Jan 28, 2024

Thank you for testing!

Do you still always make a copy of the modify_ scripts into /tmp? I bet avoiding that could save a bunch of time, though it would require setting chmod +x on the script in the source directory (this is fine, git tracks +x file attribute).

Yes, the scripts are always written to a temporary directory before being executed. This is always necessary if the script is a template. I agree that if the modify_ script is not a template and already has the executable attribute then this can be skipped.

Even though git preserves the executable attribute, chezmoi's source state is designed to only use regular files and directories. This is important to provide compatibility with all filesystems and version control systems. In theory, chezmoi should work even if the only way to transfer your dotfiles is on a FAT32-formatted USB stick.

Although milliseconds can be shaved by avoiding the write to a temporary filesystem, I think the real benefits for the user will come from #2814 and #2670, which should improve chezmoi's running time by multiple seconds.

(Another note, I found it slightly annoying that I need to combine CHEZMOI_SOURCE_DIR and CHEZMOI_SOURCE_FILE to get the path. But if you don't already have that combined on your side for other purposes, it is probably faster to do the string manipulation in the rust code on my side anyway.)

Concatenating two strings should only take fractional nanoseconds.

Thank you again, and hope that you're on your way to recovery. I'll merge this PR.

@twpayne twpayne merged commit 431ec39 into master Jan 28, 2024
18 checks passed
@twpayne twpayne deleted the fix-2934 branch January 28, 2024 01:28
@VorpalBlade
Copy link
Contributor

Two questions:

  1. Which version number of chezmoi will the the minimum to support this?
  2. Is the output of chezmoi --version stable? (I.e. will always contain a substring on the form v[0-9]+\.[0-9]+\.[0-9]+, where the version presumably follows semantic versioning?)

@twpayne
Copy link
Owner Author

twpayne commented Jan 28, 2024

  1. Which version number of chezmoi will the the minimum to support this?

The next one, which will be either 2.46.1 or 2.47.0 depending on what other features and bug fixes land in the next few weeks. However, you can also test for the presence of the CHEZMOI_SOURCE_FILE env var directly, which should be more robust.

2. Is the output of chezmoi --version stable? (I.e. will always contain a substring on the form v[0-9]+\.[0-9]+\.[0-9]+, where the version presumably follows semantic versioning?)

Yes. To be extra robust, look for the first occurrence of this regexp in the output of chezmoi --version, which may contain multiple lines.

@VorpalBlade
Copy link
Contributor

  1. Which version number of chezmoi will the the minimum to support this?

The next one, which will be either 2.46.1 or 2.47.0 depending on what other features and bug fixes land in the next few weeks. However, you can also test for the presence of the CHEZMOI_SOURCE_FILE env var directly, which should be more robust.

Not where I need to: I have a flag --add where the user invokes chezmoi_modify_manager directly. This is basically like chezmoi add but sets up a template modify script etc, and handles the case of converting managed-by-chezmoi to managed-by-chezmoi_modify_manager. Here is where I need to decide on if the added modify script should be templated or not, etc.

I wrote some documentation on how all of this works (with placeholder chezmoi version numbers):

https://github.com/VorpalBlade/chezmoi_modify_manager/blob/567f1b2069d36fdac5c909714a25421b2f5a95fe/doc/source_specification.md

I cannot just switch over to CHEZMOI_SOURCE_FILE if set, since the user may not be following the official naming scheme (it was never documented as unsupported to do so, and as such would be a breaking change).

  1. Is the output of chezmoi --version stable? (I.e. will always contain a substring on the form v[0-9]+\.[0-9]+\.[0-9]+, where the version presumably follows semantic versioning?)

Yes. To be extra robust, look for the first occurrence of this regexp in the output of chezmoi --version, which may contain multiple lines.

Awesome.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

modify script executed from weird path (even when modify script is not templated)
3 participants