Constructs like $HOME/path in the include and exclude lists are not honored

Bug #1549776 reported by Naël
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Déjà Dup
Fix Released
Low
Naël

Bug Description

When setting Déjà Dup's include-list and exclude-list GConf keys from the command line via GSettings, I just assumed constructs like ['$HOME/path'] would work since the default values are ['$HOME'] and ['$TRASH', '$DOWNLOAD'], respectively. Nope, didn't work. They're just ignored and don't show up in DEJA_DUP_DEBUG=1 deja-dup --backup | appropriate grepping.

It's fine if Déjà Dup doesn't support this syntax, I was just a bit surprised. Perhaps this could be explicitly mentioned in the documentation for the GConf keys (http://bazaar.launchpad.net/~deja-dup-hackers/deja-dup/34/view/head:/data/org.gnome.DejaDup.gschema.xml.in).

Using Déja Dup 30.0 w/ duplicity 0.6.23 on Ubuntu 14.04.4 LTS.

Related branches

Revision history for this message
Vej (vej) wrote :

Hello Nathanael,

thanks for submitting this bug report.

$USER is currently the only key, which is allowed to be inside of a larger string.

One could think about adding this support for $HOME and $DOWNLOADS or $MUSIC as well .

In the meantime I will provide (another) small fix for the documentation next week.

Best Regards

Vej

Changed in deja-dup:
status: New → Triaged
importance: Undecided → Low
assignee: nobody → Vej (vej)
Revision history for this message
Naël (nathanael-naeri) wrote :

Thanks for your answer Vej.

This documentation for the GConf keys is not easy to find, as it is only in the source code tree, as far as I know.

Is it possible to include it in the program's man page? Or at least include the link to it? Or perhaps include it in the help files (http://bazaar.launchpad.net/~deja-dup-hackers/deja-dup/34/view/head:/deja-dup/help), which apparently are installed to /usr/share/help/[lang]/deja-dup?

Revision history for this message
Vej (vej) wrote :

Hello Nathanael.

You can see this documentation if you use a tool like the DConf editor.

I dislike the idea of putting this into the manpage, cause setting the keys directly is AFAIK official not supported from the maintainer. This software is meant to be kept simple and only these simple and officially supported things should be a part of the manpage.

Changed in deja-dup:
status: Triaged → In Progress
Revision history for this message
Naël (nathanael-naeri) wrote :

I was not aware of the existence of DConf Editor. Handy tool! Now I feel silly.

Revision history for this message
Vej (vej) wrote :

Nathanael: Don't worry! Everyone here learns something new from time to time.

Revision history for this message
Vej (vej) wrote :

I commited the partial patch, including the changed descriptions.

Please feel free to continue with adding suport inside of larger paths if you like.

Changed in deja-dup:
status: In Progress → Triaged
assignee: Vej (vej) → nobody
Revision history for this message
Naël (nathanael-naeri) wrote :

Thanks Vej, I think your patch fixes this bug: it's fine if Déjà-Dup doesn't support this feature, as long as it's documented. We can follow-up with a wishlist bug if necessary.

That said, I've looked into what may be needed to support this particular feature, and it seems to me it's a fairly trivial alteration of the function parse_keywords in the file DirHandling.vala.

I'm including the following patch (my first one!), can you please review it? If you think it's fine I'll publish it in a branch (also my first one!). Please note that I know very little of both Vala and Déjà-Dup's code.

This patch compiles and passes all the included tests (make check), several of which include $HOME, $TRASH, $DOWNLOADS in include and exclude lists, so at least it should not introduce obvious regressions.

I'll test it more extensively as soon as I can find out how without messing up my own backups and backup settings.

Revision history for this message
Naël (nathanael-naeri) wrote :

I've been able to test my patch in the test shell that is included with Déjà Dup's source code and points to the build executables.

As far as I can tell, everything works fine, although the test shell seems to cause Déjà-Dup trouble finding the trash can and the user's special directories. See included report for more about this and a guide to reproducing the test. I've logged in as new user for testing.

I'll push this in a branch and request a review for merging upstream.

Revision history for this message
Naël (nathanael-naeri) wrote :
Revision history for this message
Vej (vej) wrote :

Hello Nathanael,

from the code this looks good to me.
I plan to do more testing this this evening (Dutch timezone).

Please note, that I am not allowed to merge it into the main branch (although I have the technical rights to do so).

@ Michael: To avoid confusion you shoul eiter apply the patch or merge the linked branch proposed for merging. The Branch will change the documentation to describe what is not possible , while the patch will add a new feature, to make the special characters available everywhere.

Best Reagrds

Vej

Changed in deja-dup:
status: Triaged → Fix Committed
Revision history for this message
Vej (vej) wrote :

Hello again.

Thanks for submitting that patch.
Unfortunately I found a problem.

Reproduction steps:

- Boot a machine running Ubuntu (might work on other systems as well)
- Apply the patch on top of commit 1570
- Compile
- Create a folder in your music directory and name it e.g. ggfgh
- Add the folder to your include list
- It will be presented as ~/Music/ggfgh (or what ever localized music folder you have) in the GUI.
- Change the entry inside of the gsettings value from /home/test/Music/ggfgh to $MUSIC/ggfgh
- Open the GUI again and look at the "Folders to save"

What happens: The folder is presented as /ggfgh.

What I expect to happen: The folder should be presented as ~/Music/ggfgh.

So this would need a change in the code, which generates the String for the GUI.

Best Regards

Vej

Changed in deja-dup:
status: Fix Committed → Triaged
Revision history for this message
Naël (nathanael-naeri) wrote :

Thanks for your feedback Vej. I'll examine this ASAP.

> - Compile

Did you install the executables afterward, or did you test in the test shell? In case you installed the executables, can you please explain how you did it? I couldn't find out how.

> - Add the folder to your include list

Using the GUI?

> - It will be presented as ~/Music/ggfgh (or what ever
> localized music folder you have) in the GUI.

GUI that you opened with the command deja-dup-preferences in the test shell?

> - Change the entry inside of the gsettings value from
> /home/test/Music/ggfgh to $MUSIC/ggfgh
> - Open the GUI again and look at the "Folders to save"
>
> What happens: The folder is presented as /ggfgh.
> What I expect to happen: The folder should be presented
> as ~/Music/ggfgh.

If you used the test shell, that might be "normal": I noticed that when testing things in the test shell, using the current revision 1570 (my patch has nothing to do with it), neither the trash nor the user's special directories are found, i.e.

  Environment.get_user_special_dir(UserDirectory.MUSIC)

returns an empty string, while

  Environment.get_home_dir()

returns /home/<user> as expected. So when using the test shell, $MUSIC/ggfgh would become /ggfgh indeed, and I suppose it would become /ggfgh whether or not you previously added ~/Music/ggfgh in the GUI or not.

See the remarks at the end of the file I joined to comment 8. If you did use the test shell, you can probably reproduce the behavior you mention in comment 11 by using the current, unpatched code at revision 1570.

If this non-support of special directories is indeed, as I understand it, a bug in the test shell, and if it is not beyond my abilities, I would be happy to try and fix it.

Revision history for this message
Vej (vej) wrote :

> Did you install the executables afterward, or did you test in the test shell? In case you installed the executables, can you please explain how you did it? I couldn't find out how.
I used the test shell and the GUI, which opens using deja-dup-preferences inside of that shell to add the path in the first place.

> If you used the test shell, that might be "normal": I noticed that when testing things in the test shell, using the current revision 1570 (my patch has nothing to do with it), neither the trash nor the user's special directories are found, i.e.
> Environment.get_user_special_dir(UserDirectory.MUSIC)
> returns an empty string

Oh. I was not aware of this. If this is the case, we need to find another way to test this.

> If this non-support of special directories is indeed, as I understand it, a bug in the test shell, and if it is not beyond my abilities, I would be happy to try and fix it.
Feel free, but do not put to much effort into this, because we will need to migrate the test shell to another command someday. See bug #1624861 which itself turned out to be too hard for me to fix it.

Best Regards

Vej

Revision history for this message
Naël (nathanael-naeri) wrote :

I have found the reason why the test shell appears to not support the user's special directories (Downloads, Music...), and leads to Déjà-Dup not finding them during tests (and not finding the trash can too). It only took me a full day :-\

As far as I can tell, this reason is unrelated to the d-bus problem in bug 1624861 that you mention. This bug is also beyond my skills.

Please see bug 1660174, bug 1660224, bug 1660342. There's a patch for the test shell that fixes these three bugs, in bug 1660342 comment 3.

Once this patch has been applied to the current revision 1570, it becomes possible to test the patch I submitted for this bug report (1549776) in comment 7, with other constructs than $HOME/subdir: $DOWNLOAD/subdir, $MUSIC/subdir, $TRASH/subdir, all should work.

I attach an updated set of instructions for testing that patch. It obsoletes the one in comment 8.

Revision history for this message
Naël (nathanael-naeri) wrote :

I also attach a log file from a test run, that obsoletes the one in comment 9.

Naël (nathanael-naeri)
Changed in deja-dup:
assignee: nobody → Naël (nathanael-naeri)
status: Triaged → In Progress
Michael Terry (mterry)
Changed in deja-dup:
status: In Progress → Fix Committed
Revision history for this message
Vej (vej) wrote :

This had been released as a part of Déjà Dup 34.4.

Changed in deja-dup:
status: Fix Committed → Fix Released
assignee: Nathanaël Naeri (Naël) (nathanael-naeri) → nobody
Naël (nathanael-naeri)
Changed in deja-dup:
assignee: nobody → Nathanaël Naeri (Naël) (nathanael-naeri)
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.