Merge lp:~psusi/ubuntu/vivid/grub-installer/fix-efi-multi-disk-installs into lp:ubuntu/vivid/grub-installer

Proposed by Phillip Susi
Status: Merged
Merge reported by: Phillip Susi
Merged at revision: not available
Proposed branch: lp:~psusi/ubuntu/vivid/grub-installer/fix-efi-multi-disk-installs
Merge into: lp:ubuntu/vivid/grub-installer
Diff against target: 31 lines (+13/-0)
2 files modified
debian/changelog (+7/-0)
grub-installer (+6/-0)
To merge this branch: bzr merge lp:~psusi/ubuntu/vivid/grub-installer/fix-efi-multi-disk-installs
Reviewer Review Type Date Requested Status
Mathieu Trudel-Lapierre Needs Fixing
Phillip Susi (community) Needs Resubmitting
Review via email: mp+247772@code.launchpad.net
To post a comment you must log in.
121. By dann frazier

Install grub-arm64-efi on arm64/efi platforms. (From Debian)

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

There's a comment further down that explicitly mentions avoiding touching the active flag on GPT, so in that sense your changes look fine, but it seems the code that actually did ignore GPT from grub-installer itself went away, and now that is only checked in ensure-active itself -- could you move that comment and reword it so that the intent of the change is clear?

Something like "doing this because we won't set partitions active on GPT, and if $bootdisk points to a drive with no partition table things will explode in fun ways"... In your own words ;)

review: Needs Fixing
Revision history for this message
Phillip Susi (psusi) wrote :

You just want me to change the comments? Ok, I'll resubmit.

review: Needs Resubmitting
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Hmm, but now we have an issue with changelog. Would you mind fixing this on top? No need to resubmit, just adjust the changelog to resolve conflicts there and I'll merge then.

review: Needs Fixing
122. By Phillip Susi

Don't try to mark a partition as active, except on grub-pc.
This was causing failures for grub-efi (LP: #1303790).

Revision history for this message
Phillip Susi (psusi) wrote :

Ahh, someone else touched it since I originally submitted this.. changelog merged.

Revision history for this message
Phillip Susi (psusi) wrote :

Mathieu, can you merge this now?

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2015-02-03 16:45:04 +0000
3+++ debian/changelog 2015-02-17 21:22:11 +0000
4@@ -1,3 +1,10 @@
5+grub-installer (1.78ubuntu25) vivid; urgency=medium
6+
7+ * Don't try to mark a partition as active, except on grub-pc.
8+ This was causing failures for grub-efi (LP: #1303790).
9+
10+ -- Phillip Susi <psusi@ubuntu.com> Tue, 27 Jan 2015 15:51:09 -0500
11+
12 grub-installer (1.78ubuntu24) vivid; urgency=medium
13
14 * Install grub-arm64-efi on arm64/efi platforms. (From Debian)
15
16=== modified file 'grub-installer'
17--- grub-installer 2015-02-03 16:45:04 +0000
18+++ grub-installer 2015-02-17 21:22:11 +0000
19@@ -1009,6 +1009,12 @@
20 }
21
22 make_active_partition () {
23+ if [ "$grub_package" != "grub-pc" ]; then
24+ # only do this for grub-pc since on EFI $bootdev is a dummy argument
25+ # and looking for a partition table on the wrong or non existing disk
26+ # crashes the installer LP:#1303790
27+ return
28+ fi
29 bootdisk=
30 bootpart=
31 case $bootdev in

Subscribers

People subscribed via source and target branches

to all changes: