Bug #15627

feature/14594-asp-gui FTBFS due to merge conflicts

Added by intrigeri 2018-05-29 19:23:08 . Updated 2018-09-05 16:11:55 .

Status:
Resolved
Priority:
High
Assignee:
Category:
Target version:
Start date:
2018-05-29
Due date:
% Done:

100%

Feature Branch:
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Additional Software Packages
Deliverable for:
299

Description

Merging the base branch (devel) fails due to merge conflicts. Can you please fix this?


Subtasks


Related issues

Blocks Tails - Bug #15566: Additional Software on newly created partition are not shown in configuration window Resolved 2018-05-03
Blocked by Tails - Bug #15472: Rebase our Tor Browser AppArmor policy on top of torbrowser-launcher 0.2.9-2's Resolved 2018-03-28

History

#1 Updated by intrigeri 2018-06-05 14:36:01

  • blocks Bug #15566: Additional Software on newly created partition are not shown in configuration window added

#2 Updated by intrigeri 2018-06-05 14:36:08

  • Priority changed from Normal to High

#3 Updated by intrigeri 2018-06-05 15:59:12

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 10
  • QA Check set to Ready for QA

Merged devel and resolved conflicts by preferring what was on devel. Let’s hope that did not overwrite some translation work that would have been done on our topic branch already.

#4 Updated by intrigeri 2018-06-05 16:53:45

  • Assignee changed from alant to intrigeri
  • QA Check changed from Ready for QA to Dev Needed

The perl5lib patch does not apply cleanly anymore.

#5 Updated by intrigeri 2018-06-05 17:26:32

  • Assignee changed from intrigeri to alant
  • QA Check changed from Dev Needed to Ready for QA

Fixed that other conflict.

#6 Updated by intrigeri 2018-06-11 08:59:49

  • QA Check deleted (Ready for QA)

It’s broken again. I’ll let you deal with it this time.

#7 Updated by intrigeri 2018-06-26 16:28:02

  • Target version changed from Tails_3.8 to Tails_3.9

#8 Updated by intrigeri 2018-06-28 13:18:12

  • blocked by Bug #15472: Rebase our Tor Browser AppArmor policy on top of torbrowser-launcher 0.2.9-2's added

#9 Updated by intrigeri 2018-06-28 13:18:46

You’ll need to merge current devel and the branch for Bug #15472 (if it’s not been merged into devel yet) in order to work on this.

#10 Updated by intrigeri 2018-06-30 07:17:02

  • QA Check set to Ready for QA

intrigeri wrote:
> It’s broken again. I’ll let you deal with it this time.

Well, well, well. I’ve fixed it again.

#11 Updated by alant 2018-07-02 23:42:56

Thanks and sorry for not having done it before.
It looks good but I don’t know hav to review merge conflicts.

#12 Updated by intrigeri 2018-07-05 12:31:13

I have only done (twice) what I suggested you to do at last meeting: git checkout origin/devel -- po && ./refresh-translations

#13 Updated by intrigeri 2018-07-05 12:32:48

Also, you might learn useful stuff in the “Diff Formatting” section of git-log(1).

#14 Updated by alant 2018-07-27 09:58:50

  • % Done changed from 10 to 50

I merged again, but merged POTFILES.in from ASP branch (else new POs were missing). Then intltool-update --maintain still complains about configuration-window.ui not being in use, while it’s actually handeled a few lines above in refresh-translations. I added it to POTFILES.skip which fixes the build but may be wrong.

#15 Updated by alant 2018-07-27 10:56:42

  • Assignee changed from alant to intrigeri
  • QA Check changed from Ready for QA to Info Needed

#16 Updated by intrigeri 2018-07-28 02:52:49

  • Assignee changed from intrigeri to segfault
  • QA Check changed from Info Needed to Ready for QA

It seems to me that you’re now asking for a review of your fix => assigning to your reviewer.

#17 Updated by segfault 2018-07-29 17:31:16

  • Assignee changed from segfault to alant
  • QA Check changed from Ready for QA to Dev Needed

alant wrote:
> Then intltool-update --maintain still complains about configuration-window.ui not being in use, while it’s actually handeled a few lines above in refresh-translations. I added it to POTFILES.skip which fixes the build but may be wrong.

We had the same problem with VeraCrypt Mounter (see Feature #15043#note-23 and following). We ended up not using xgettext --language=Glade, but instead added the glade files to POTFILES.in, in the section that is used with intltool-merge --xml-style. If you want to do that too, make sure you:

  • Rename the glade files to *.ui.in
  • Add the *.ui files that are generated by intltool to .gitignore
  • Remove the glade files from refresh-translations and POTFILES.skip

#18 Updated by alant 2018-07-30 22:09:48

  • Assignee changed from alant to segfault
  • QA Check changed from Dev Needed to Info Needed

> We had the same problem with VeraCrypt Mounter (see Feature #15043#note-23 and following). We ended up not using xgettext --language=Glade, but instead added the glade files to POTFILES.in, in the section that is used with intltool-merge --xml-style.

What would be the advantage of this approach?

#19 Updated by segfault 2018-07-30 22:27:21

  • Assignee changed from segfault to alant
  • QA Check deleted (Info Needed)

alant wrote:
> > We had the same problem with VeraCrypt Mounter (see Feature #15043#note-23 and following). We ended up not using xgettext --language=Glade, but instead added the glade files to POTFILES.in, in the section that is used with intltool-merge --xml-style.
>
> What would be the advantage of this approach?

The one advantage I see is that the file doesn’t have to be added to POTFILES.skip. But this was not implemented by me, but by intrigeri, see d3e24f668fc7b55010081f1d0e4dda92f8c01ed2 - so maybe ask him if you don’t think that’s good enough.

#20 Updated by alant 2018-07-30 23:15:13

  • Assignee changed from alant to segfault
  • QA Check set to Info Needed

> The one advantage I see is that the file doesn’t have to be added to POTFILES.skip.

Yes, but one drawback is to have .ui.in instead of directly usable .ui.

> But this was not implemented by me, but by intrigeri, see d3e24f668fc7b55010081f1d0e4dda92f8c01ed2 - so maybe ask him if you don’t think that’s good enough.

This commit doesn’t explain why it would be better than the solution I implemented, and the ticket reads “Actually, I think that intltool-update behaves exactly as documented and the workaround I had put in place simply made it not notice that the Python file had translatable strings. So adding the file to POTFILES.skip seems correct.” (https://labs.riseup.net/code/issues/15043#note-28).

#21 Updated by segfault 2018-07-30 23:35:05

  • Assignee changed from segfault to alant
  • QA Check deleted (Info Needed)

alant wrote:
> > But this was not implemented by me, but by intrigeri, see d3e24f668fc7b55010081f1d0e4dda92f8c01ed2 - so maybe ask him if you don’t think that’s good enough.
>
> This commit doesn’t explain why it would be better than the solution I implemented

Yes, but it does change the solution of using xgettext --language=Glade to the one including the .ui files in po/POTFILES.in. Like I already said, if you want further explanation, please ask intrigeri.

> and the ticket reads “Actually, I think that intltool-update behaves exactly as documented and the workaround I had put in place simply made it not notice that the Python file had translatable strings. So adding the file to POTFILES.skip seems correct.” (https://labs.riseup.net/code/issues/15043#note-28).

That is unrelated, it is about Python files, not Glade files.

#22 Updated by Anonymous 2018-08-07 13:48:35

@alant: you seem to have found a solution (using .skip). Considering that we have not much time left to make this work, and not seeing an explanation of why the other approach is considered better, I suggest to keep this solution for now and get back to it after the release. Deal?

#23 Updated by Anonymous 2018-08-07 13:49:06

Otherwise, @intrigeri: if you have a remark about this, your input is highly appreciated.

#24 Updated by alant 2018-08-09 17:43:58

  • Status changed from In Progress to Fix committed
  • % Done changed from 50 to 100

u wrote:
> @alant: you seem to have found a solution (using .skip). Considering that we have not much time left to make this work, and not seeing an explanation of why the other approach is considered better, I suggest to keep this solution for now and get back to it after the release. Deal?

OK.

#25 Updated by alant 2018-08-09 20:07:13

  • Assignee deleted (alant)

#26 Updated by intrigeri 2018-09-05 16:11:55

  • Status changed from Fix committed to Resolved