Conversation
|
Obvious AI slop aside, this seems like a non-feature when you can just pass |
This comment was marked as spam.
This comment was marked as spam.
|
Please stop using AI to communicate. If English is not your first language, that is perfectly fine. I would appreciate if you worded your own thoughts, in your own language and style, and let us figure out translation. I believe your intentions are good, but this form of communication where you dump your AI's output on us and expect us to sift through it is not feasible. |
|
I'm sorry. I will stop using AI on contributing hjem / smfh. Here is my thoughts in Korean. and manually translated to English. 저는 hjem을 사용해본 적이 없지만 chezmoi와 home-manager를 사용했습니다. I haven't used hjem yet. but I have used chezmoi and home-manager. |
|
Verbose and long answer is my style to give more information. I can answer short if needed. |
|
It is my second time to contributing. I'm still learning about that. I'm worried and unsure about contributor's strong opinion seems to be aggresive. |
|
Thank you for explaining without AI.
Don't worry, we will assume you are making your best effort to communicate clearly. |
|
It is difficult to explain.
Yes. I want to reducing repetition to simplify hjem implementation. mostly for convenience. If "recoverable rollback" described below is not needed, It could be purely convenience feature.
Also Yes. If you move your dotfiles repo to somewhere, rollback will fail. like: If this PR merged, it will be very easy to implement. If this PR not merged, it will be complicate problem. But I can't make more complicate PR without this. because I'm new in rust. I just learned it really briefly for contribute hjem/smfh. I should have ensure that I "understand" codes in PR even though it mostly written by AI assistant. Current PR seems not difficult to understand for me even though I'm new in rust. I could just ignore the edge case with throwing error, but there was simpler way(this PR) that solves simplification and edge case both. It seems logical to me. My only concern is that |
|
Thank you for communicating your own words, that is much easier to read and parse for me. As far as I understand, you seem to want to make live symlinks from a Nix-managed config to files in a Git checkout without hardcoding the checkout's absolute path in Nix. I think this is a good idea to prevent the previously implicit behaviour that was "just use the absolute path" (which was not ideal), and in this new model (which I think means that Hjem supplies the checkout path at runtime; smfh uses it to resolve relative sources) it is less ambiguous, and finally worth supporting as a first-class citizen. You also seem to want a moved checkout to remain usable during rollback. I don't quite see the benefit of this, but I'm all in for enabling more advanced usecases in smfh. That said, I have a few concerns. My first concern is whether it fits. While I want smfh to be more usable, I'd much rather avoid scope creep. This concept fits Hjem: supporting editable, out-of-store dotfiles is a clear use case for a Secondly, we have to settle I'm currently at work. Given the AI-assisted nature of this PR, I'd like to take some ample time to review and nitpick. In the meantime, we can clarify the scope, design, and semantics. |
I agree. The reason was these are pair and it could be used in future. I couldn't find actual use cases yet. So I can remove it from PR.
I have one idea that needs more review. Separate format and clean command in future.
I didn't implemented on hjem side yet. I will include in hjem PR before changing it to "ready for review". |
Summary
Add optional base directories for deterministic relative path resolution.
A manifest can define:
base_diras the default for relative source and target paths;source_base_diras the source-specific override; andtarget_base_diras the target-specific override.Relative paths are resolved when the manifest is read. Absolute paths are
unchanged.
For a relative source, the resolution order is:
source_base_dir;base_dir; and--impureis enabled.Relative targets follow the same order with
target_base_dirin place ofsource_base_dir. If no applicable option is available, the relative path isignored.
Motivation
smfh already supports relative paths in impure mode, but those paths depend on
the process current working directory. An explicit base directory makes the
path context deterministic without requiring callers to control the working
directory.
Hjem use case
Hjem can use
source_base_dirfor repository-relative live sources whilekeeping its existing absolute target semantics. This allows Hjem to inject the
checkout root at runtime while preserving the logical relative source in its
configuration and generation state.
Related Hjem integration:
The Hjem integration depends on this PR being merged first because it passes
source_base_dirthrough the manifest consumed by smfh.Other use cases
Generated configuration manifests used by CI, deployment scripts, or other
automation can refer to files relative to a known project or configuration
root instead of relying on the caller's current directory.
Manifest version
The manifest version is bumped from 3 to 4 because the manifest semantics
change when base directories are present. Older smfh versions do not apply
these fields and therefore cannot provide the same path resolution behavior.
The version bump is kept in a separate commit. If maintainers consider these
optional fields compatible with version 3, that commit can be removed from the
PR.
cleancleancontinues to read, verify, sort, and re-serialize the parsed manifest.When a base directory is provided, relative source and target paths are
resolved while reading the manifest, so
cleanserializes those resolvedpaths.
If maintainers prefer
cleanto preserve logical relative paths, I am happyto revise this behavior or add an explicit option for resolved output.
Changes
base_dir,source_base_dir, andtarget_base_dir.InvalidBaseDirread error.--impureinteraction.Validation
nix develop -c cargo fmt --checknix develop -c cargo checknix develop -c cargo testThe current macOS run has 27 passing tests out of 29. The two existing
failures compare
/tmp/...with/private/tmp/..., caused by macOS/tmpcanonicalization and unrelated to this change.
Review questions
version bump be removed?
cleanprint resolved paths or preserve logical relative paths?base_dirand source/target-specificoverrides appropriate for smfh's manifest model?
Sanity Checking
nix develop cargo fmt --check)x86_64-linuxaarch64-linuxaarch64-darwin