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

Add ability to exclude arbitrary directories from backup #86

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

b02860de585071a2
Copy link

Fixes #85

@b02860de585071a2
Copy link
Author

b02860de585071a2 commented Oct 8, 2023

A few notes on this solution:

  • Using read is flaky at best, I'd much prefer to manually enter exclusions with a real editor. However, this does not follow the stylistic choices of the rest of the project + adds another layer of complexity (we have to call toybox utils to do anything useful, this is likely inconsistent across Android versions).
    • Alternatively, this could be implemented with another whiptail menu
  • I am not 100% certain if this affects user hooks, so I have tried to make it as self-contained and close to the original code as possible.
  • A command which works in an interactive shell session may not work when called with adb shell/adb exec-out - this caused me some grief when my test cases only failed after integration into the main script.

@mrrfv
Copy link
Owner

mrrfv commented Oct 8, 2023

Looks good overall, thank you. I'll test this next weekend and then merge.
It looks like the exclusions aren't used if the user chooses the adb exporting method, correct? In that case, it might be a good idea to force the tar version and let the user know that's going to happen on the "You may optionally choose to exclude certain directories and/or files from being backed up" screen.
Also, since the exclusions are space-delimited, it won't be possible to exclude files or folders with spaces - that could pose an issue.

@b02860de585071a2
Copy link
Author

b02860de585071a2 commented Oct 8, 2023

You actually brought up a couple things I wanted to get your input on.

It looks like the exclusions aren't used if the user chooses the adb exporting method, correct?

Correct. There is a way to do something equivalent with adb pull, but I haven't finished testing edge cases yet.

It currently exits with an error message on my test branch, but I don't think I've merged that change in yet.

since the exclusions are space-delimited, it won't be possible to exclude files or folders with spaces

That's something I was hoping to discuss. The plan is to change this to comma-delimited, with some other filtering to make any possible inputs shell-safe.

Regardless of the delimiter though, I see 3 possible options for making the input more stable/robust:

  1. Continue using read for user input, with filtering for escape sequences (ie, you could input [...]/some\ dir)

or

  1. Hand off the task to a real editor (for example, calling toybox vi) - but I have concerns about this possibly being inconsistent across ROMs, plus it requires users to be comfortable with vi

or

  1. Create another whiptail menu with some form of interactive selection (probably harder than it sounds?)

Not sure what your thoughts are on that?

Honestly I just wanted to get a proof of concept out, compare notes, and see if you had any feedback before I progress further. The whole project seems designed for simplicity and accessibility to non-technical users, and I'd like to not break that.

@mrrfv
Copy link
Owner

mrrfv commented Oct 9, 2023

I've done my own experiments with adding directory/file exclusions in the past (didn't merge it into the main branch because I couldn't figure out how to reliably exclude files with adb pull), and a whiptail menu wasn't that hard to implement - I think I just used find or ls to get the device's directories and fed that to whiptail in the form of a list/array. I could dig up that code if you'd like.

@b02860de585071a2
Copy link
Author

That would be helpful, if it's not too much work.

Whenever I get some free time this week, I'll see if I can make a functional menu.

I couldn't figure out how to reliably exclude files with adb pull

Bash scripting is nothing new to me, but there have been some very bizarre side effects/behaviors from the adb shell/mksh. It seems to be quite picky about the way scripts are fed to it.

@mrrfv
Copy link
Owner

mrrfv commented Oct 11, 2023

image

Here's a screenshot.


My experimental code added a new Exclusions option to the "What do you want to do?" list (currently containing just Backup and Restore features). When the option is selected, a choose_exclusions_func function is called, which:

  • gets the device ID from adb
  • uses find to get all the directories on the device (potential problem: machine-generated directory structures with lots of items polluting the list)
  • loops over the device's directories to determine if they've been already excluded and convert the output to a format whiptail accepts
  • shows a whiptail checklist
  • saves the exclusions to the home directory for later use, with the device ID used to separate the info per-device.
    # ...
    # Get device identifier from adb
    device_id=$(adb devices | sed -n 2p | cut -f 1)
    # Get the exclusions file location
    exclusions_file="$HOME/.open_android_backup_exclusions_$device_id.txt"

    # Define the items and their status
    directories=$(adb shell find /storage/emulated/0 -type d | tr -d '\r')
    options=()
    for dir in $directories; do
        # Dynamically set ON or OFF based on whether the directory already exists in the file
        if [[ -f "$exclusions_file" && $(grep -c "^$dir$" "$exclusions_file") -eq 1 ]]; then
            options+=("$dir" "" ON)
        else
            options+=("$dir" "" OFF)
        fi
    done

    # Show the checklist and capture the output
    selected=$(whiptail --title "Select directories to exclude" --checklist "Choose directories to exclude:" "$(expr $LINES - 8)" "$(expr $COLUMNS - 10)" "$(expr $LINES - 16)" "${options[@]}" 3>&1 1>&2 2>&3)
    # ...
    # Save exclusions to the home directory using the device id, each exclusion separated by a newline
    echo "$selected" | tr ' ' '\n' | tr -d '"' > "$exclusions_file"
    cecho "Exclusions saved to $exclusions_file - you can now run the script again and your exclusions will be loaded."
    cecho "Warning! Exclusions are set on a per-device basis."

Although it looks like my version doesn't seem to support directories with spaces either, as evidenced by the tr ' ' '\n'. I hope this is helpful.

@b02860de585071a2
Copy link
Author

Thanks for this!

I've had very little free time lately, but am 90% sure I can get a working implementation for adb.

Will use this PR to document anything in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] Allow user to exclude arbitrary directories from backup
2 participants