Project

General

Profile

Bug #12527

Feature #5630: Reproducible builds

Duplicate gpg-agent killing code introduced in the build-tails script

Added by intrigeri 9 months ago. Updated 9 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
Build system
Target version:
Start date:
05/10/2017
Due date:
% Done:

100%

QA Check:
Pass
Feature Branch:
bugfix/12527-really-fix-unmounting-on-build-cleanup
Type of work:
Code
Blueprint:
Starter:
Affected tool:

Description

75060eeee5f87e1aac472ebeb4596b2a7e3df2f3 feels wrong: cleanup runs remove_build_dirs that kills blocking processes already, in a way that's both more generic (even processes other than gpg-agent are killed) and more accurate (unrelated gpg-agent processes are not affected). So I'm not quite sure why we need to duplicate this. Reading https://jenkins.tails.boum.org/view/RM/job/build_Tails_ISO_feature-stretch-unfrozen/lastFailedBuild/console I see "fuser: command not found"; I suspect that's why the existing code doesn't work. So I suggest making the existing (and IMO better) code work, instead of duplicating it. What do you think?


Related issues

Related to Tails - Bug #12537: Killall is not installed in the VM Duplicate 05/12/2017

Associated revisions

Revision 2fbc3ea0 (diff)
Added by bertagaz 9 months ago

Build: remove duplicate call to kill remaining processes.

It seems fuser binary was missing in the build VM for the cleanup code
to correctly work and kill processes that prevent the unmounting of the
build directory.

Will-fix: #12527

Revision e82b1b6e
Added by bertagaz 9 months ago

Merge remote-tracking branch 'origin/bugfix/12527-really-fix-unmounting-on-build-cleanup' into feature/stretch

Fix-committed: #12527

History

#1 Updated by intrigeri 9 months ago

  • Parent task set to #5630

#2 Updated by bertagaz 9 months ago

  • Status changed from Confirmed to In Progress

#3 Updated by bertagaz 9 months ago

  • Assignee changed from bertagaz to intrigeri
  • % Done changed from 0 to 50
  • QA Check set to Ready for QA
  • Feature Branch set to bugfix/12527-really-fix-unmounting-on-build-cleanup

intrigeri wrote:

75060eeee5f87e1aac472ebeb4596b2a7e3df2f3 feels wrong: cleanup runs remove_build_dirs that kills blocking processes already, in a way that's both more generic (even processes other than gpg-agent are killed) and more accurate (unrelated gpg-agent processes are not affected). So I'm not quite sure why we need to duplicate this. Reading https://jenkins.tails.boum.org/view/RM/job/build_Tails_ISO_feature-stretch-unfrozen/lastFailedBuild/console I see "fuser: command not found"; I suspect that's why the existing code doesn't work. So I suggest making the existing (and IMO better) code work, instead of duplicating it. What do you think?

Indeed, I misunderstood the error message. I've pushed a branch removing the duplicated code, and it works fine with just the addition of 'psmisc' to the vagrant build VM, see https://jenkins.tails.boum.org/job/build_Tails_ISO_bugfix-12527-really-fix-unmounting-on-build-cleanup/1/console

Please merge if happy.

#4 Updated by intrigeri 9 months ago

  • Assignee changed from intrigeri to bertagaz
  • % Done changed from 50 to 70
  • QA Check changed from Ready for QA to Dev Needed

Wait, we need this fix on the stable branch too, no? It seems to be based on feature/stretch. Other than that, the commit looks good, so feel free to merge the branch yourself in stable, devel and feature/stretch once you've first rebased it on stable.

#5 Updated by anonym 9 months ago

intrigeri wrote:

Wait, we need this fix on the stable branch too, no?

Yes.

Also, since these fixes are "later" in the Git history than the base box version bump, we are affected by the issue identified in #12409#note-32. For instance, I had actually built the current base box before the commit installing psmisc into the basebox, so the invocation of killall failed => build failure due to set -e. So, please also release a new base box (or review'n'merge feature/12409-improved-vagrant-base-box-versioning).

#6 Updated by bertagaz 9 months ago

anonym wrote:

intrigeri wrote:

Wait, we need this fix on the stable branch too, no?

Yes.

I did base it on feature/stretch cause I didn't see this error on stable or devel. I'll rebase that branch on stable.

Also, since these fixes are "later" in the Git history than the base box version bump, we are affected by the issue identified in #12409#note-32. For instance, I had actually built the current base box before the commit installing psmisc into the basebox, so the invocation of killall failed => build failure due to set -e. So, please also release a new base box (or review'n'merge feature/12409-improved-vagrant-base-box-versioning).

Ok, I ought to r'n'm the #12409 branch before the end of the week, so I'll do that.

#7 Updated by intrigeri 9 months ago

I did base it on feature/stretch cause I didn't see this error on stable or devel.

Right, this can only happen with GnuPG 2.x.

I'll rebase that branch on stable.

No need to then, you can merge it as-is into feature/stretch :)

Ok, I ought to r'n'm the #12409 branch before the end of the week, so I'll do that.

Please save some time (after r'n'm-ing) for stabilizing by the end of the week anything it might break once deployed ;)

#8 Updated by bertagaz 9 months ago

  • Related to Bug #12537: Killall is not installed in the VM added

#9 Updated by bertagaz 9 months ago

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

#10 Updated by bertagaz 9 months ago

  • Status changed from Fix committed to Resolved
  • Assignee deleted (bertagaz)
  • QA Check changed from Dev Needed to Pass

Merged!

Also available in: Atom PDF