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

Proposed by Yu Ning
Status: Rejected
Rejected by: Mathieu Trudel-Lapierre
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
Mathieu Trudel-Lapierre Pending
usb-creator hackers 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.
Revision history for this message
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.

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

Yu Ning,

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

Revision history for this message
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

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

Revision history for this message
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

Revision history for this message
Yu Ning (yuningdodo) wrote :
review: Disapprove

Unmerged revisions

462. By Yu Ning

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

461. By Yu Ning

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: