improve recursion and fix various issues with unstowing / --dotfiles
#107
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a ton of duplication which is not maintainable, so refactor everything into a single test which still covers the differences. This in turn revealed some issues in the unstowing logic: - We shouldn't conflict if we find a file which isn't a link or a directory; we can just skip over it. - Unstowing with `--dotfiles` was using the wrong variable to obtain the package path, and as a result having to perform an unnecessary call to `adjust_dotfile()`. So fix those at the same time.
Stow walks the package and target tree hierarchies by using mutually recursive pairs of functions: - `stow_contents()` and `stow_node()` - `unstow_contents()` and `unstow_node()` As Stow runs its planning from the target directory (`plan_*()` both call `within_target_do()`), previously the parameters for these included: - `$target_subpath` (or `$target_subdir` in the `*_node()` functions): the relative path from the target top-level directory to the target subdirectory (initially `.` at the beginning of recursion). For example, this could be `dir1/subdir1/file1`. - `$source`: the relative path from the target _subdirectory_ (N.B. _not_ top-level directory) to the package subdirectory. For example, if the relative path to the Stow directory is `../stow`, this could be `../../../stow/pkg1/dir1/subdir1/file1`. This is used when stowing to construct a new link, or when unstowing to detect whether the link can be unstowed. Each time it descends into a further subdirectory of the target and package, it appends the new path segment onto both of these, and also prefixes `$source` with another `..`. When the `--dotfiles` parameter is enabled, it adjusts `$target_subdir`, performing the `dot-foo` => `.foo` adjustment on all segments of the path in one go. In this case, `$target_subpath` could be something like `.dir1/subdir1/file1`, and the corresponding `$source` could be something like `../../../stow/pkg1/dot-dir1/subdir1/file1`. However this doesn't leave an easy way to obtain the relative path from the target _top-level_ directory to the package subdirectory (i.e. `../stow/pkg1/dot-dir1/subdir1/file1`), which is needed for checking its existence and if necessary iterating over its contents. The current implementation solves this by including an extra `$level` parameter which tracks the recursion depth, and uses that to strip the right number of leading path segments off the front of `$source`. (In the above example, it would remove `../..`.) This implementation isn't the most elegant because: - It involves adding things to `$source` and then removing them again. - It performs the `dot-` => `.` adjustment on every path segment at each level, which is overkill, since when recursing down a level, only adjustment on the final subdirectory is required since the higher segments have already had any required adjustment. This in turn requires `adjust_dotfile` to be more complex than it needs to be. It also prevents a potential future where we might want Stow to optionally start iterating from within a subdirectory of the whole package install image / target tree, avoiding adjustment at higher levels and only doing it at the levels below the starting point. - It requires passing an extra `$level` parameter which can be automatically calculated simply by counting the number of slashes in `$target_subpath`. So change the `$source` recursion parameter to instead track the relative path from the top-level package directory to the package subdirectory or file being considered for (un)stowing, and rename it to avoid the ambiguity caused by the word "source". Also automatically calculate the depth simply by counting the number of slashes, and reconstruct `$source` when needed by combining the relative path to the Stow directory with the package name and `$target_subpath`. Closes #33.
This should make it harder for Stow to do the right thing.
If the target directory as a file named X and a package has a directory named X, or vice-versa, then it is impossible for Stow to stow that entry X from the package, even if --adopt is supplied. However we were previously only handling the former case, and not the latter, and the test for the former was actually broken. So fix stow_contents() to handle both cases correctly, fix the broken test, and add a new test for the latter case.
We use the term "directory" (or "dir" for short) rather than "folder". Also explicitly say whether a test is stowing or unstowing, and fix the odd typo.
Unstowing with `--dotfiles` didn't work with `--compat`, because when traversing the target tree rather than the package tree, there was no mechanism for mapping a `.foo` file or directory back to its original `dot-foo` and determine whether it should be unstowed. So add a reverse `unadjust_dotfile()` mapping mechanism to support this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #33.