binpash / try

Inspect a command's effects before modifying your live system
MIT License
5.17k stars 64 forks source link

Implement Multi-Lower-Layer Overlay Support through Merging #122

Closed gliargovas closed 6 months ago

gliargovas commented 9 months ago

This Pull Request introduces the -L flag to try, enabling the specification and merging of multiple lower directories for overlay. The lower directories are specified as a colon-separated list (dir_1:dir_2:...:dir_n), operating with a precedence logic where directories on the left have a higher precedence over those on the right. This feature allows users to overlay multiple directories together during the command execution within a single overlay environment.

gliargovas commented 9 months ago

Should I also add some tests for this?

angelhof commented 9 months ago

Could you rebase this to not have Eric's and Michael's commits? Maybe you just need to start your branch from future instead of main?

gliargovas commented 9 months ago

@mgree I have addressed all your comments. Feel free to merge!

ezrizhu commented 9 months ago

before we merge, i’d like to take a bit and look into why CI is failing, i’ll try to repro this on my machines.

mgree commented 9 months ago

Talking with @angelhof about this, we realized there's a tricky interaction with process_changes when you have multiple lowerdirs: you have to replay the merge to do the comparison.

The correct solution is to mergerfs mount the layers of lowerdir and compare against that merged lowerdir (rather than the $local_file which just absolutizes to the real system root).

But until we have a use case where someone would want to use summaries and multiple lowerdirs (hs doesn't want the summaries), we should punt on this and give a warning that -L implies -n.

gliargovas commented 8 months ago

Rebased this off main

gliargovas commented 8 months ago

@mgree, I addressed the comments you left.

gliargovas commented 8 months ago

Main workflow/test is expected to fail because the workflow is changed to run ./scripts/setup.sh in the' future' branch.

gliargovas commented 8 months ago

Cleaned everything unrelated to this PR

ezrizhu commented 8 months ago

Oh, is this supposed to merge into future or main branch?

ezrizhu commented 7 months ago

Could you also rebase off latest main or add in my latest commit so CI tests pass :pray: nvm i got it!