Merge lp:~yuningdodo/usb-creator/usb-creator.lp1325801v2-sync-syslinux-c32-files into lp:usb-creator

Proposed by Yu Ning on 2015-02-15
Status: Rejected
Rejected by: Mathieu Trudel-Lapierre on 2015-03-05
Proposed branch: lp:~yuningdodo/usb-creator/usb-creator.lp1325801v2-sync-syslinux-c32-files
Merge into: lp:usb-creator
Diff against target: 31 lines (+14/-0)
1 file modified
usbcreator/install.py (+14/-0)
To merge this branch: bzr merge lp:~yuningdodo/usb-creator/usb-creator.lp1325801v2-sync-syslinux-c32-files
Reviewer Review Type Date Requested Status
Yu Ning (community) Disapprove on 2015-02-25
Mathieu Trudel-Lapierre 2015-02-17 Pending
usb-creator hackers 2015-02-15 Pending
Review via email: mp+249769@code.launchpad.net

Description of the change

Make sure we use the same version of mbr.bin and syslinux *.c32 files. (LP: #1325801)

This patch was originally uploaded in bug #1325801 comment #83, I have made some tests on trusty, it can correctly create the usbstick for ubuntu 12.04, 14.10, etc., and even xubuntu.

To post a comment you must log in.
Yu Ning (yuningdodo) wrote :

This proposal is based on a previous one:
https://code.launchpad.net/~yuningdodo/usb-creator/usb-creator.lp1325801-sync-syslinux-c32-files/+merge/249454

What has been improved is that now we will search for the host *.c32 files in two paths:

/usr/lib/syslinux/
/usr/lib/syslinux/modules/bios/

On 14.10 there are actually three group of modules, /usr/lib/syslinux/modules/{bios,efi32,efi64}, I guess the efi support on cdrom is provided by grub2, so we only need syslinux to provide the legacy support, that is group "bios".

I haven't got a chance to test it yet, I'll get a testing environment about two weeks later after the local holidays in my country, so I'll update the test results later.

Mathieu Trudel-Lapierre (cyphermox) wrote :

Yu Ning,

Any news on this? Have you had a chance to test it?

Yu Ning (yuningdodo) wrote :

Hi Mathieu,

Sorry for the late update. Unfortunately the patch doesn't work when host system is 14.10 and target system is 14.04. It will report an error that some other c32 files are missing. However even if we modify the patch to copy all the host c32 files to target it still doesn't work as expected. It does show the menu, but in a very low resolution (maybe 20x10 ?), and no text is displayed. We can still operate blindly, such as press F6 to show the boot args menu, the menu can be popped in a low resolution, you just can't see any text.

In such a case I would rather reject this patch and propose another merge request with https://launchpadlibrarian.net/194872381/use-source-syslinux.patch , you can see a simple compare between these two patches here: https://bugs.launchpad.net/ubuntu/+source/usb-creator/+bug/1325801/comments/92 . I'll make some tests and propose the request later.

Or maybe we should use different solutions for <=14.04 and >=14.10 ?

462. By Yu Ning on 2015-02-25

Copy all the host c32 files to target in case dependences missing.

Yu Ning (yuningdodo) wrote :

Anyway I have updated this patch to copy all the host c32 files to target.

To test this patch w/o installing it:

bzr branch lp:~yuningdodo/usb-creator/usb-creator.lp1325801v2-sync-syslinux-c32-files test
cd test

# in terminal 1
sudo killall usb-creator-helper
sudo ./bin/usb-creator-helper

# in terminal 2
./bin/usb-creator-gtk

Unmerged revisions

462. By Yu Ning on 2015-02-25

Copy all the host c32 files to target in case dependences missing.

461. By Yu Ning on 2015-02-15

Make sure we use the same version of mbr.bin and syslinux *.c32 files. (LP: #1325801)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'usbcreator/install.py'
2--- usbcreator/install.py 2013-01-28 12:44:46 +0000
3+++ usbcreator/install.py 2015-02-25 05:04:14 +0000
4@@ -19,6 +19,7 @@
5 from usbcreator.misc import (
6 USBCreatorProcessException,
7 callable,
8+ find_on_path,
9 fs_size,
10 popen,
11 )
12@@ -323,6 +324,19 @@
13 if f:
14 f.close()
15 self.check()
16+ if self.need_syslinux_legacy() and find_on_path('syslinux-legacy'):
17+ syslinux = 'syslinux-legacy'
18+ else:
19+ syslinux = 'syslinux'
20+ for dstname in glob.iglob(os.path.join(self.target, 'syslinux', '*.c32')):
21+ os.remove(dstname)
22+ for pathname in ['', os.path.join('modules', 'bios')]:
23+ srcdir = os.path.join(os.sep, 'usr', 'lib', syslinux, pathname)
24+ for srcname in glob.iglob(os.path.join(srcdir, '*.c32')):
25+ filename = os.path.basename(srcname)
26+ dstname = os.path.join(self.target, 'syslinux', filename)
27+ shutil.copyfile(srcname, dstname)
28+ self.check()
29
30 def create_persistence(self):
31 logging.debug('create_persistence')

Subscribers

People subscribed via source and target branches

to all changes: