Project

General

Profile

Bug #15627

Feature #14568: Additional Software Packages

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

Added by intrigeri about 1 year ago. Updated 11 months ago.

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

100%

Feature Branch:
Type of work:
Code
Blueprint:
Starter:
Affected tool:
Additional Software Packages

Description

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


Related issues

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

History

#1 Updated by intrigeri about 1 year ago

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

#2 Updated by intrigeri about 1 year ago

  • Priority changed from Normal to High

#3 Updated by intrigeri about 1 year ago

  • 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 about 1 year ago

  • 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 about 1 year ago

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

Fixed that other conflict.

#6 Updated by intrigeri about 1 year ago

  • QA Check deleted (Ready for QA)

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

#7 Updated by intrigeri about 1 year ago

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

#8 Updated by intrigeri about 1 year ago

  • 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 about 1 year ago

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

#10 Updated by intrigeri about 1 year ago

  • 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 about 1 year ago

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 about 1 year ago

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 about 1 year ago

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

#14 Updated by alant 12 months ago

  • % 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 12 months ago

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

#16 Updated by intrigeri 12 months ago

  • 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 12 months ago

  • 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 #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 12 months ago

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

We had the same problem with VeraCrypt Mounter (see #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 12 months ago

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

alant wrote:

We had the same problem with VeraCrypt Mounter (see #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 12 months ago

  • 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 12 months ago

  • 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 u 12 months ago

@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 u 12 months ago

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

#24 Updated by alant 11 months ago

  • 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 11 months ago

  • Assignee deleted (alant)

#26 Updated by intrigeri 11 months ago

  • Status changed from Fix committed to Resolved

Also available in: Atom PDF