512477 - make_incremental_updates.py needs to have quotations around filenames in shell command

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Lukas Blakk [:lsblakk] use ?needinfo]

Attachment: puts filenames in quotations for make_incremental_updates.py

We ran into this issue while releasing Firefox 3.0.14 - the Default Plugin binary changed and because it has a space in its name, the mar creation failed.  

Attaching a patch that will put quotations around the file paths to avoid this in the future.

Comment 1

[[e-mail] (:bhearsum)]

Comment on attachment 396451 [details] [diff] [review]
puts filenames in quotations for make_incremental_updates.py

>? tools/build-environment/win32/.new.N3UKJ6
>Index: tools/update-packaging/make_incremental_updates.py
>===================================================================
>RCS file: /cvsroot/mozilla/tools/update-packaging/make_incremental_updates.py,v
>retrieving revision 1.9
>diff -u -8 -p -r1.9 make_incremental_updates.py
>--- tools/update-packaging/make_incremental_updates.py	17 Oct 2008 11:55:42 -0000	1.9
>+++ tools/update-packaging/make_incremental_updates.py	25 Aug 2009 16:00:27 -0000
>@@ -159,17 +159,17 @@ def bunzip_file(filename):
> 
> def extract_mar(filename, work_dir): 
>     """ Extracts the marfile intot he work_dir
>         assumes work_dir already exists otherwise will throw osError"""
>     print "Extracting "+filename+" to "+work_dir
>     saved_path = os.getcwd()
>     try:
>         os.chdir(work_dir)
>-        exec_shell_cmd("mar -x "+filename)    
>+        exec_shell_cmd('mar -x "'+filename+'"')    
>     finally:
>         os.chdir(saved_path)
> 
> def create_partial_patch_for_file(from_marfile_entry, to_marfile_entry, shas, patch_info):
>     """ Creates the partial patch file and manifest entry for the pair of files passed in
>     """
>     if not (from_marfile_entry.sha(),to_marfile_entry.sha()) in shas:
>         print "diffing: " + from_marfile_entry.name
>@@ -181,17 +181,17 @@ def create_partial_patch_for_file(from_m
>         # The patch file will be created in the working directory with the
>         # name of the file in the mar + .patch
>         patch_file_abs_path = os.path.join(patch_info.work_dir,from_marfile_entry.name+".patch")
>         patch_file_dir=os.path.dirname(patch_file_abs_path)
>         if not os.path.exists(patch_file_dir):
>             os.makedirs(patch_file_dir)
> 
>         # Create bzip'd patch file
>-        exec_shell_cmd("mbsdiff "+from_marfile_entry.abs_path+" "+to_marfile_entry.abs_path+" "+patch_file_abs_path)
>+        exec_shell_cmd('mbsdiff "'+from_marfile_entry.abs_path+'" "'+to_marfile_entry.abs_path+'" "'+patch_file_abs_path+'"')
>         bzip_file(patch_file_abs_path)
> 
>         # Create bzip's full file
>         full_file_abs_path =  os.path.join(patch_info.work_dir, to_marfile_entry.name)   
>         shutil.copy2(to_marfile_entry.abs_path, full_file_abs_path)
>         bzip_file(full_file_abs_path)
>   
>         ## TOODO NEED TO ADD HANDLING FOR FORCED UPDATES
>@@ -295,17 +295,19 @@ def create_partial_patch(from_dir_path, 
>         create_add_patch_for_file(to_dir_hash[filename], patch_info)
> 
>     process_explicit_remove_files(to_dir_path, patch_info)
>     
>     # Construct Manifest file
>     patch_info.create_manifest_file()
>     
>     # And construct the mar
>-    mar_cmd = 'mar -C '+patch_info.work_dir+' -c output.mar '+string.join(patch_info.archive_files, ' ')    
>+    mar_cmd = 'mar -C "'+patch_info.work_dir+'" -c output.mar '
>+    for archive_file in patch_info.archive_files:
>+        mar_cmd = mar_cmd +' "'+archive_file'"'
>     exec_shell_cmd(mar_cmd)

I just took this for a test run. It turns out that all of the entries in archive_file are already quoted - this block ended up causing a different error. Dropping this hunk seems to work - let's do that. I'll check in an updated patch.

Comment 2

[[e-mail] (:bhearsum)]

Attachment: fix make_incremental_updates.py for files with spaces

Checking in make_incremental_updates.py;
/cvsroot/mozilla/tools/update-packaging/make_incremental_updates.py,v  <--  make_incremental_updates.py
new revision: 1.10; previous revision: 1.9
done
I moved the UPDATE_PACKAGING_R9 tag, too:
bitters-2:update-packaging bhearsum$ cvs tag UPDATE_PACKAGING_R9 make_incremental_updates.py 
W make_incremental_updates.py : UPDATE_PACKAGING_R9 already exists on version 1.9 : NOT MOVING tag to version 1.10
bitters-2:update-packaging bhearsum$ cvs tag -F UPDATE_PACKAGING_R9 make_incremental_updates.py 
T make_incremental_updates.py