Skip to content

Commit

Permalink
fix(zypp): Fixed moving libzypp target to /mnt (#1352)
Browse files Browse the repository at this point in the history
## Problem

- Moving the libzypp target to `/mnt` (the target installed system)
happened too early, already in the "propose" step, it needs to happen
after partitioning the disk.
- The side effect of this is that mounting the `/mnt` partition can
shadow the libzypp cache so it could crash later. Also the next target
used after the product change would be the `/mnt` instead of the
`/run/agama/zypp`.

## Solution

- Move the target change to the "install" step

## Notes

- I found out that the `Pkg.SourceCacheCopyTo` call does not work
properly with target different than `/`. It always copies the data from
`/` ignoring the current target.
- It could copy some wrong data from the Live ISO to the target, as a
quick solution I have commented that out. I'll fix it later in a
separate PR.

## Testing

- Tested manually
- Updated unit tests
  • Loading branch information
lslezak authored Jun 17, 2024
2 parents 75c103a + 831790c commit a5caf80
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 15 deletions.
10 changes: 9 additions & 1 deletion service/lib/agama/software/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ def propose

# Installs the packages to the target system
def install
# move the target from the Live ISO to the installed system (/mnt)
Yast::Pkg.TargetFinish
Yast::Pkg.TargetInitialize(Yast::Installation.destdir)
Yast::Pkg.TargetLoad

steps = proposal.packages_count
start_progress(steps)
Callbacks::Progress.setup(steps, progress)
Expand All @@ -196,7 +201,10 @@ def finish
progress.step(_("Writing repositories to the target system")) do
Yast::Pkg.SourceSaveAll
Yast::Pkg.TargetFinish
Yast::Pkg.SourceCacheCopyTo(Yast::Installation.destdir)
# FIXME: Pkg.SourceCacheCopyTo works correctly only from the inst-sys
# (original target "/"), it does not work correctly when using
# "chroot" /run/agama/zypp, it needs to be reimplemented :-(
# Yast::Pkg.SourceCacheCopyTo(Yast::Installation.destdir)
registration.finish
end
end
Expand Down
6 changes: 1 addition & 5 deletions service/lib/agama/software/proposal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,8 @@ def languages=(value)

# Initializes the target, closing the previous one
def initialize_target
Yast::Pkg.TargetFinish # ensure that previous target is closed
Yast::Pkg.TargetInitialize(Yast::Installation.destdir)
Yast::Pkg.TargetLoad

preferred, *additional = languages
Yast::Pkg.SetPackageLocale(preferred) if preferred
Yast::Pkg.SetPackageLocale(preferred || "")
Yast::Pkg.SetAdditionalLocales(additional)

Yast::Pkg.SetSolverFlags("ignoreAlreadyRecommended" => false, "onlyRequires" => false)
Expand Down
11 changes: 9 additions & 2 deletions service/test/agama/software/manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@

before do
allow(Yast::Pkg).to receive(:TargetInitialize)
allow(Yast::Pkg).to receive(:TargetFinish)
allow(Yast::Pkg).to receive(:TargetLoad)
allow(Yast::Pkg).to receive(:SourceSaveAll)
allow(Yast::Pkg).to receive(:ImportGPGKey)
# allow glob to work for other calls
Expand Down Expand Up @@ -339,14 +341,19 @@
expect { subject.install }.to raise_error(RuntimeError)
end
end

it "moves the packaging target to /mnt" do
expect(Yast::Pkg).to receive(:TargetFinish)
expect(Yast::Pkg).to receive(:TargetInitialize).with(destdir)
expect(Yast::Pkg).to receive(:TargetLoad)
subject.install
end
end

describe "#finish" do
it "releases the packaging system" do
expect(Yast::Pkg).to receive(:SourceSaveAll)
expect(Yast::Pkg).to receive(:TargetFinish)
expect(Yast::Pkg).to receive(:SourceCacheCopyTo)
.with(Yast::Installation.destdir)

subject.finish
end
Expand Down
7 changes: 0 additions & 7 deletions service/test/agama/software/proposal_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,6 @@
end

describe "#calculate" do
it "initializes the packaging target" do
expect(Yast::Pkg).to receive(:TargetFinish)
expect(Yast::Pkg).to receive(:TargetInitialize).with(destdir)
expect(Yast::Pkg).to receive(:TargetLoad)
subject.calculate
end

it "makes a proposal" do
expect(Yast::Packages).to receive(:Proposal).and_return(result)
expect(Yast::Pkg).to receive(:PkgSolve)
Expand Down

0 comments on commit a5caf80

Please sign in to comment.