Tails

#29 Updated by alant 2018-03-05 22:24:59

Most of the code and GUI is done. We mostly miss:

• strings for uncommon case in the configuration window welcome screen
• the implementation of removal from the list in the configuration window
• notification of failure when adding/removing a package from the notification

Also there are probably lots of bugs, including:

• when a package is installed from packagekitd, removal from ASP works, but adding ASP fails

#30 Updated by bertagaz 2018-03-05 23:25:14

I do not mean to put pressure, but I have some and it seems the ISO fails to build with:

20:03:01 + ./refresh-translations
20:03:02 Files tails.pot.orig and tails.pot differ
20:03:02 Real changes in tails.pot: switching to the updated one
20:03:02 The following files contain translations and are currently not in use. Please
20:03:02 consider adding these to the POTFILES.in file, located in the po/ directory.
20:03:02 
20:03:02 config/chroot_local-includes/usr/share/tails/additional-software/configuration-window.ui
20:03:02 
20:03:02 If some of these files are left out on purpose then please add them to
20:03:02 POTFILES.skip instead of POTFILES.in. A file 'missing' containing this list
20:03:02 of left out files has been written in the current directory.

Should be easy to fix.

#31 Updated by intrigeri 2018-03-06 07:33:35

bertagaz wrote:
> it seems the ISO fails to build with:

Fixed but the build still FTBFS later on, in the 10-tbb hook, exactly like when the APT hook was run even in the live-build context. Looking at the diff since my last successful build, nothing obvious pops to mind. I bet that /usr/local/sbin/tails-additional-software fails before it had a chance to check whether it’s running in the context of live-build. Might it be that from tailslib.additionalsoftware.config import […] fails?

#32 Updated by intrigeri 2018-03-06 07:35:35

bertagaz wrote:
> I do not mean to put pressure, but I have some and it seems the ISO fails to build with:

For your needs, I bet that the ISO from the last successful build is good enough.

#33 Updated by intrigeri 2018-03-06 07:58:57

intrigeri wrote:
> Fixed but the build still FTBFS later on, in the 10-tbb hook

Fixed! … at build time (commit:646ae3cda18c22003cf33eea6f2be7f6e273de27). No idea if the breakage will affect runtime, I’ll let Alan handle this.

#34 Updated by intrigeri 2018-03-06 09:38:03

intrigeri wrote:
> No idea if the breakage will affect runtime, I’ll let Alan handle this.

tails-additional-software-config appears to be broken:

Mar 06 09:22:23 amnesia tails-persistence-setup.desktop[8283]: Traceback (most recent call last):
Mar 06 09:22:23 amnesia tails-persistence-setup.desktop[8283]:   File "/usr/local/bin/tails-additional-software-config", line 15, in <module>
Mar 06 09:22:23 amnesia tails-persistence-setup.desktop[8283]:     from tailslib.additionalsoftware.config import (            # NOQA: E402
Mar 06 09:22:23 amnesia tails-persistence-setup.desktop[8283]: ImportError: No module named 'tailslib.additionalsoftware'

#35 Updated by alant 2018-03-06 10:32:47

intrigeri wrote:
> No idea if the breakage will affect runtime, I’ll let Alan handle this.

I think I fixed this, I forgot to update tailslib’s setup.py.

#36 Updated by alant 2018-03-13 13:41:20

> * notification of failure when adding/removing a package from the notification
>
Done.

I added subtickets for the other remaining subtasks.

#37 Updated by intrigeri 2018-03-16 09:37:33

Alan, I’ve merged current devel into the branch, resolved conflicts, merged current master into your pythonlib branch, but the branch FTBFS:

08:23:15 Get:1 http://time-based.snapshots.deb.tails.boum.org/debian/2018031601 stretch/main amd64 python3-setuptools all 33.1.1-1 [215 kB]
08:23:17 Traceback (most recent call last):
08:23:17   File "/usr/local/sbin/tails-additional-software", line 15, in <module>
08:23:17     from tailslib.additionalsoftware.config import (
08:23:17 ImportError: No module named 'tailslib'

I think that’s a cyclic dependency situation caused by the combination of the APT hooks you’re introducing in this branch + commit:e0b24a215182fe386ce2940639b115039cdfadaa. I’ve fixed it with commit:3ba1893b4a317071308d658d8dca47338fe46876, please take a look.

#38 Updated by alant 2018-03-18 10:42:52

intrigeri wrote:
> I think that’s a cyclic dependency situation caused by the combination of the APT hooks you’re introducing in this branch + commit:e0b24a215182fe386ce2940639b115039cdfadaa. I’ve fixed it with commit:3ba1893b4a317071308d658d8dca47338fe46876, please take a look.

Your fix looks good, thanks!

However, I’m not convinced at all that using (not standard) setuptools to workaround a wierd apparmor behaviour is a good idea. I won’t argue endlessly on that, but it looks to me like piling patches (because apparmor behaves wierdly, use setuptools. Then install setuptools and remove it wierdly…)

#39 Updated by intrigeri 2018-03-18 11:26:23

> However, I’m not convinced at all that using (not standard) setuptools to workaround a wierd apparmor behaviour is a good idea.

I agree it would have been preferable to fix the AppArmor policy but I also understand why segfault preferred to implement a solution that’s closer to his comfort zone.

(Nitpicking: AppArmor was not behaving weirdly at all. It was doing exactly what the policy we ship says.)

#40 Updated by bertagaz 2018-03-19 15:35:07

I’ve broken the build when I merged devel into the branch, I’ll repair that ASAP.

#41 Updated by bertagaz 2018-03-20 13:16:28

bertagaz wrote:
> I’ve broken the build when I merged devel into the branch, I’ll repair that ASAP.

I’ve pushed commit:8dd945efef3e4f47f11742862cf34b098ae42cc2 which fixes the build and sounds more robust for future changes in the build hooks. Please alant have a look.

#42 Updated by intrigeri 2018-03-21 14:24:05

bertagaz wrote:
> bertagaz wrote:
> > I’ve broken the build when I merged devel into the branch, I’ll repair that ASAP.
>
> I’ve pushed commit:8dd945efef3e4f47f11742862cf34b098ae42cc2 which fixes the build and sounds more robust for future changes in the build hooks. Please alant have a look.

LGTM but then:

• commit:3ba1893b4a317071308d658d8dca47338fe46876 can probably be reverted
• the code in t-a-s-p that detects a live-build environment should probably be removed
  *.Please rename the hook to s/DPKG/dpkg/ (AFAIK “DPKG” is not a thing :)

#43 Updated by alant 2018-03-22 23:54:56

bertagaz wrote:
> I’ve pushed commit:8dd945efef3e4f47f11742862cf34b098ae42cc2 which fixes the build and sounds more robust for future changes in the build hooks. Please alant have a look.

Thanks for fixing the build.

The commit looks good in terms of result. I agree that if we keep it we should revert the previous fix as well as the specific code in tails-additional-software (I can do that part).

However, I’m afraid that having the APT configuration saved inside a build hook will make it difficult to find when working on this feature later. I propose to have config/chroot_local-includes/etc/apt/apt.conf.d/80tails-additional-software.disabled contain the configuaration (man apt.conf says that it will be ignored by APT) and move it to config/chroot_local-includes/etc/apt/apt.conf.d/80tails-additional-software in the build hook. If you agree, I can implement that.

#46 Updated by bertagaz 2018-03-23 12:43:00

alant wrote:
> The commit looks good in terms of result. I agree that if we keep it we should revert the previous fix as well as the specific code in tails-additional-software (I can do that part).

I agree! I was busy with Feature #15496 and just wanted the build to work, so I did not follow on that. If you want to do it, be my guest. :)

> However, I’m afraid that having the APT configuration saved inside a build hook will make it difficult to find when working on this feature later. I propose to have config/chroot_local-includes/etc/apt/apt.conf.d/80tails-additional-software.disabled contain the configuaration (man apt.conf says that it will be ignored by APT) and move it to config/chroot_local-includes/etc/apt/apt.conf.d/80tails-additional-software in the build hook. If you agree, I can implement that.

OK, that sounds like a good idea. Please do that. :)

#47 Updated by alant 2018-03-26 00:18:34

bertagaz wrote:
> alant wrote:
> > The commit looks good in terms of result. I agree that if we keep it we should revert the previous fix
Done.

> > as well as the specific code in tails-additional-software (I can do that part).

This breaks the build. It seems the hooks are not the last part of the build process, and APT is needed after the last hook.

> > However, I’m afraid that having the APT configuration saved inside a build hook will make it difficult to find when working on this feature later. I propose to have config/chroot_local-includes/etc/apt/apt.conf.d/80tails-additional-software.disabled contain the configuaration (man apt.conf says that it will be ignored by APT) and move it to config/chroot_local-includes/etc/apt/apt.conf.d/80tails-additional-software in the build hook. If you agree, I can implement that.
>
> OK, that sounds like a good idea. Please do that. :)

Done too.

#48 Updated by bertagaz 2018-04-03 12:50:52

alant wrote:
> > > as well as the specific code in tails-additional-software (I can do that part).
>
> This breaks the build. It seems the hooks are not the last part of the build process, and APT is needed after the last hook.

ACK.

> > > However, I’m afraid that having the APT configuration saved inside a build hook will make it difficult to find when working on this feature later. I propose to have config/chroot_local-includes/etc/apt/apt.conf.d/80tails-additional-software.disabled contain the configuaration (man apt.conf says that it will be ignored by APT) and move it to config/chroot_local-includes/etc/apt/apt.conf.d/80tails-additional-software in the build hook. If you agree, I can implement that.
> Done too.

Awesome.

I’ve gone a good deal forward on Feature #14596, and I have one remark: the ‘Installing your additional software from persistent storage’ notification is not permanent. It is closed when the APT command ends. I can understand the reason why: another notification is then showed up. But it makes this first notification hard to catch in the test suite.

So there’s two options about it for Feature #14596: either we ignore this notification (because its detection is not robust enough in the test suite), either we turn this notification in a more ‘permanent’ one.

I don’t care much of the choice. I’ve pushed commit:bce7961b in the feature/14596-automated-tests-for-ASP-gui-with-installing-notification that removes the _close_notification calls and function. It’s running in Jenkins with the tests for Feature #14596, just in case this option is chosen, to see if it’s more robust.

I think it could be OK to set this notification as ‘permanent’ in the user point of view, that way they’ll have a sort of full notification history in the notification area, which is a bit how I understood this area. But alant and sojolida are probably the best to make the final call about this.

#49 Updated by alant 2018-04-05 12:43:10

> I’ve gone a good deal forward on Feature #14596, and I have one remark: the ‘Installing your additional software from persistent storage’ notification is not permanent. It is closed when the APT command ends. I can understand the reason why: another notification is then showed up. But it makes this first notification hard to catch in the test suite.
>
> So there’s two options about it for Feature #14596: either we ignore this notification (because its detection is not robust enough in the test suite), either we turn this notification in a more ‘permanent’ one.
>
> I don’t care much of the choice. I’ve pushed commit:bce7961b in the feature/14596-automated-tests-for-ASP-gui-with-installing-notification that removes the _close_notification calls and function. It’s running in Jenkins with the tests for Feature #14596, just in case this option is chosen, to see if it’s more robust.
>
> I think it could be OK to set this notification as ‘permanent’ in the user point of view, that way they’ll have a sort of full notification history in the notification area, which is a bit how I understood this area. But alant and sojolida are probably the best to make the final call about this.

It would simplify the code to keep this notification as ‘permanent’. This was what I did first when implementing the feature. But in terms of UX I was feeling it was wierd and then I implemented the closing. Unless Sajolida thinks the opposite I would be in favor of closing the notification like I do now, even if it can’t be easily tested.

Also, would it be possible to listen to org.freedesktop.Notifications and test the call to Notifiy call? This looks robust to me.

#51 Updated by sajolida 2018-04-07 09:57:08

> Unless Sajolida thinks the opposite I would be in favor of closing the notification like I do now, even if it can’t be easily tested.

I’m fine with that! Let’s favor making the interface nicer to humans
over making it nicer to machines :)