mhogomchungu / sirikali

A Qt/C++ GUI front end to sshfs, ecryptfs-simple, cryfs, gocryptfs, securefs, fscrypt and encfs
http://mhogomchungu.github.io/sirikali
GNU General Public License v3.0
770 stars 58 forks source link

A nice to have: Add ability for users to add their own backends by modifying some sort of a backend configuration file #95

Closed mhogomchungu closed 5 years ago

mhogomchungu commented 5 years ago

The git version supports this now and below images show how this functionality looks, i used "archivemount"[1] tool as a test case.

  1. The GUI window to create backend config file is generated by pressing "CTRL+B" key combo.
  2. The config file will be created in user's home directory.
  3. SiriKali search for the config file in "~/.config/SiriKali/backends" and in "$INSTALL_PREFIX/share/SiriKali/backends"

[1] https://linux.die.net/man/1/archivemount

Screenshot_20190518_081700

Screenshot_20190518_081335 Screenshot_20190518_081301

brainchild0 commented 5 years ago

Great work. I feel that this feature will greatly advance the breadth of usefulness of SiriKali. As with any new major feature, one can list a great many small incremental improvements that would continue to advance the essential utility of the new functionality. There is plenty of time to consider which incremental enhancements deserve attention moving forward.

In this case, one particular issue strikes me as worth mentioning immediately. Reviewing the set of fields available to populate for defining the backend, I have some concern that the way the application assembles the command to execute from the values of the defined fields is overly restrictive. It seems that the assumption is that the command can be generated in any particular case by concatenating a prescribed sequence of values, whether those values come from the mount configuration or the JSON backend definition. I fear that many backends will not be compatible with this model.

I would suggest an alternative, more flexible approach. The executable command could be constructed from a single field, describing a template for the command, in whatever structure is appropriate for the backend. Specific values can be resolved dynamically when represented by template fields encoded in the command string. A common pattern in applications with similar requirements is to interpolate fields from the template string represented with percent sign followed by a letter designated the field. In the above example, the command definition might contain, in part, archivemount --config %C. The application can then construct the actual command by interpolating the %C sequence with the name of the file, following a rule associating the letter C with the name of file.

In this way, if some backend requires a very different structure for the command, it is easy to define a template string that satisfies these requirements. Initially, such implementation may entail a larger amount of code change, but in the long run, this approach makes it easy to add functionality and flexibility without making major changes to number of fields.

Thanks for the work making this feature possible. Hopefully I am understanding the issues clearly, and you find the above considerations useful.

mhogomchungu commented 5 years ago

What you said is one of the most annoying thing in supporting multiple backends, they all do the same exact thing but chooses to do it differently and hence its impossible to have a single command structure that satisfies all of them.

The SiriKali code that calls the custom backend is here[1], m_createSwitch and m_mountSwitch variables are not set in the GUI backend creator but can be set by manually editing the configuration file after its being created.

m_createSwitch for gocryptfs for example is --init while securefs is create and its empty in encfs.

m_mountSwitch for securefs is mount while its empty in the rest of supported backends.

Some backends like encfs requires a mount point path when creating a volume and others like gocryptfs do not and to account for this, the configuration file has the option requireMountPathWhenCreating.

If you can understand the code, SiriKali will send a set a predefined arguments to the backend and my current thinking is that some users will have to create an intermediate script that sits between SiriKali and the actual backend and the job of the script will be to translate/rearrange SiriKali arguments to the actual backend arguments.

The executable field can take a full path to the backend executable and i made it this way to allow users to put the intermediate script anywhere they want and not where SiriKali look for backend executables.

I will need backends to test but the current functionality should work with overwhelming majority of fuse based backends.

[1] https://github.com/mhogomchungu/sirikali/blob/ce6ec90070c8e66e423e3d04a5389f26da6e96fc/src/engines/custom.cpp#L154-L200

brainchild0 commented 5 years ago

What you said is one of the most annoying thing in supporting multiple backends, they all do the same exact thing but chooses to do it differently and hence its impossible to have a single command structure that satisfies all of them.

The SiriKali code that calls the custom backend is here[1], m_createSwitch and m_mountSwitch variables are not set in the GUI backend creator but can be set by manually editing the configuration file after its being created.

I agree it is annoying, but I think that you can avoid much of the pain you are currently feeling, as well as the need for an intermediary script, by considering my suggestion.

The command structure itself can become a field in the backend definition. This field can contain sequences that are interpolated with instance-specific variables.

For example, suppose you create a new field for the command template. Then a backend author might make the value something like the following:

mymountexec --config %C %v %m

The letters C, v, and m would be understood by the application to represent the config file, the volume location, and the mount point. Such mapping would be a static feature of the application and documented for backend authors. The application would use this template defined by the backend to generate the specific command to execute to create the specific mount. If some other backend were to use a completely different command structure, then the author of the other backend could simply write a command template that is correct for the other backend, even if it is very different from the commands used by existing backends.

Of course you can support multiple such fields for different command types. You can support one for initialization and one for mounting. These choices depend on details of the desired user workflow that you can determine, if you have no already determined.

Also, the particular syntax of percent sign and letter is only a suggestion. I have seen such syntax in many related solutions, but it may prove too limited for this particular case. This decision is also outside the scope of my current comments.

mhogomchungu commented 5 years ago

The git version now allow users of custom backends to specify the order and number of arguments to send to backends.

I think using words is much better than those cryptic single character options.

The config file now has the following two entries default and authors of backend can customize to rearrange them or add any entries of their choosing

        "createControlStructure": [
                "createOptions",
                "cipherFolder",
                "mountPoint"
        ],
        "mountControlStructure": [
                "mountOptions",
        "cipherFolder",
                "mountPoint",
                "fuseOpts"
        ],
brainchild0 commented 5 years ago

The solution you have provided is not entirely clear to me, but I sense that a template string is likely to be more intuitive and flexible.

I have indicated that i have no opinion about words versus letters. Words is a fine choice.

Using words, you can easily interpolate variable names from a template string:

"command_template": "execname --someswitch --someoption %{someoptionvalue} %{sourcelocation} %{mountpoint}"

In this example, variable names for interpolation are indicated by being surrounded by a squiggly brace pair preceded by a percent sign. Many other syntactical choices are equally appropriate.

Mainly, the template is like a literal command, but with variable names inserted, for dynamic substitution. the template is easy to read and to debug because of the clear similarity to the literal command, and easy to understand how to write or to change. One can quickly generalize a literal command into a template, or test a template by manually substituting test values.

Meanwhile, a non-rigorous list of potential issues with the solution you suggest is 1) the verbose layout of a JSON list, 2) the inability to distinguish between literal values and variable names without adding further verbosity, and 3) the assumption that the command comprises simple tokens separated by single space characters. All of these limitations are easy to avoid.

You will find that command templates are a common pattern for this type of problem. I suggest that you consider seriously template strings as an alternative to what you have described above.

mhogomchungu commented 5 years ago

The json library i use is from here[1[ and it doesn't like the sentence format you describe for some reason and simply return an empty string(Its probably a bug) and as a workaround, i started using the array structure that you see because it works.

Your sentence format will be stored in the config file as follows so there is not that much difference

        "mountControlStructure": [
                "--someswitch",
        "--someoption",
                "%{someoptionvalue}",
                "%{sourcelocation}",
                "%{mountpoint}"
        ],
3) the assumption that the command comprises simple tokens separated 
by single space characters

The above is fundamental property of how i do things, currently, the layout of a command sent to a backend looks like this:

<executable full path> <executable specific options> <cipher path> <mount path> <fuse options>

The example sample command is:

"/usr/bin/archivemount"  "/home/ink/SiriKali-1.3.7.tar.xz" "/home/_/SiriKali-1.3.7.tar.xz"  -o rw,fsname=archivemount@"/home/ink/SiriKali-1.3.7.tar.xz",subtype=archivemount

Adding executable specific options is easy. Adding fuse options is tricky because some backends to do like multiple -o options.

The execuatble specific options part is missing in the above example and all fuse based program follows this simple structure and i will be very much surprised if i see a fuse based solution that does not follow it,

Will switch to the "%{someoptionvalue}".

Above sections are separated by a single space character [1] https://github.com/nlohmann/json

brainchild0 commented 5 years ago

The json library i use is from here[1[ and it doesn't like the sentence format you describe for some reason and simply return an empty string(Its probably a bug) and as a workaround, i started using the array structure that you see because it works.

I don't understand. I am suggesting using string values to represent a command template. All JSON libraries support string values. Your original example JSON file uses numerous string values that are not part of any array. You are using string values, so clearly the JSON library you are using supports string values.

mhogomchungu commented 5 years ago

Can you spot an error in the following code?

https://github.com/mhogomchungu/sirikali/blob/14aba23d8a877f6c28c81ce54c46d8998f077369/src/engines/custom.cpp#L53-L56

The error was on my end and i just fixed it and it now uses a statement like your prefer.

The default mount command is: %{mountOptions} %{cipherFolder} %{mountPoint} %{fuseOpts} The default create command is: %{createOptions} %{cipherFolder} %{mountPoint}

mhogomchungu commented 5 years ago

Closing this one as "fixed" but feel free to comment if anything comes up.

brainchild0 commented 5 years ago

So the current format is the same one from the example of the JSON document you gave originally, or you have made changes since the original comment? It would be helpful to give a full, updated example representing the changes, if any.

mhogomchungu commented 5 years ago

Current code generates a config file with contents like below:

{
        "autoMountsOnVolumeCreation": false,
        "configFileArgument": "--config",
        "configFileNames": [],
        "createControlStructure": "%{createOptions} %{cipherFolder} %{mountPoint}",
        "executableName": "archivemount",
        "failedToMountTextList": [],
        "fuseNames": [
                "fuse.archivemount"
        ],
        "idleString": "",
        "mountControlStructure": "%{mountOptions} %{cipherFolder} %{mountPoint} %{fuseOpts}",
        "names": [
                "archivemount"
        ],
        "requiresAPassword": false,
        "reverseString": "",
        "successfullyMountedList": [],
        "supportsMountPointPaths": false,
        "version": 1.0,
        "wrongPasswordErrorCode": "",
        "wrongPasswordText": ""
}
mhogomchungu commented 5 years ago
"createControlStructure": "%{createOptions} %{cipherFolder} %{mountPoint}",
"mountControlStructure": "%{mountOptions} %{cipherFolder} %{mountPoint} %{fuseOpts}",

The above two solved the two changes you wanted.

  1. Options are surrounded by %{}
  2. The commands are in a single string and not in a json array structure.
mhogomchungu commented 5 years ago

Version 1.3.9 is out and it supports custom backends.

Documentation on how to create a custom backend is here: https://github.com/mhogomchungu/sirikali/wiki/How-to-create-SiriKali-custom-backend

Examples of custom backends are here: https://github.com/mhogomchungu/sirikali/wiki/Example-custom-backends