Last Comment Bug 300136 - Full update archives may need to contain remove commands
: Full update archives may need to contain remove commands
Status: RESOLVED FIXED
: fixed1.8
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- major (vote)
: mozilla1.8beta4
Assigned To: Scott MacGregor
:
Mentors:
Depends on: 308864
Blocks: 290390
  Show dependency treegraph
 
Reported: 2005-07-08 12:59 PDT by Darin Fisher
Modified: 2006-03-12 18:41 PST (History)
7 users (show)
benjamin: blocking1.8b5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Package a removed-files list, rev. 1 (15.36 KB, patch)
2005-09-14 12:46 PDT, Benjamin Smedberg [:bsmedberg]
darin.moz: review+
asa: approval1.8b5+
Details | Diff | Splinter Review
tbird changes to match (18.87 KB, patch)
2005-09-15 11:49 PDT, Benjamin Smedberg [:bsmedberg]
no flags Details | Diff | Splinter Review
tbird changes to match (18.86 KB, patch)
2005-09-15 12:07 PDT, Benjamin Smedberg [:bsmedberg]
mscott: review+
mscott: approval1.8b5+
Details | Diff | Splinter Review
v1 patch: mar packaging scripts (9.20 KB, patch)
2005-09-22 10:25 PDT, Darin Fisher
chase: review+
asa: approval1.8b5+
Details | Diff | Splinter Review

Description Darin Fisher 2005-07-08 12:59:18 PDT
Full update archives may need to contain remove commands.

When we remove or move some file in a distribution, we need to include a
corresponding "remove" command in the full update archive.  For partial
updates, this is already done by virtue of the fact that the partial
update is generated from the differences between two distributions.

After discussing this problem with bsmedberg and chase, we settled on
leveraging packages-static's supposed support for "-" prefixed lines that
specify files that should not be part of the distribution.  We'll need to
add a way to specify a remove list to make_full_update.sh, but that should
be pretty straightforward.
Comment 1 Benjamin Smedberg [:bsmedberg] 2005-09-02 12:53:42 PDT
We definitely do need this.
Comment 2 Darin Fisher 2005-09-02 13:34:46 PDT
benjamin: thanks for offering to help with this patch :)  how do you plan to
implement it?  as i described in comment #0, or are you thinking about something
else?
Comment 3 Benjamin Smedberg [:bsmedberg] 2005-09-02 13:38:08 PDT
As described, using the "-" listings in the manifest to generate a file
"removed.files" which would be shipped with the zip builds. The MAR-creator
would then take this file and generate remove commands from it.

The MAR process can remove entire directory trees, can't it?
Comment 4 Benjamin Smedberg [:bsmedberg] 2005-09-14 12:46:18 PDT
Created attachment 196070 [details] [diff] [review]
Package a removed-files list, rev. 1

This packages a removed-files list, and also uses the same list to generate
files which need to be removed by the installer. The MAR creation script will
need to be taught to read this file and generate the proper remove
instructions.
Comment 5 Darin Fisher 2005-09-14 13:39:46 PDT
Comment on attachment 196070 [details] [diff] [review]
Package a removed-files list, rev. 1

>Index: browser/installer/removed-files.in

>+defaults/pref/bug307259.js
...
>+defaults/pref/bug259708.js

Perhaps this should be listed up near the other bug's pref file?


This patch looks good to me except that I don't understand why
we need to (or want to) include removed-files in the resulting
distribution.  It would seem to be a file that is only needed
by our distribution packaging scripts.	I would not bother
copying it onto users' systems.  Or, do you believe that it might
be useful one day to have that info on users' systems?


r=darin
Comment 6 Benjamin Smedberg [:bsmedberg] 2005-09-14 13:50:44 PDT
I would like to preserve the possibility to go straight from tarball->MAR
without any extra metadata if possible.
Comment 7 Darin Fisher 2005-09-14 15:13:39 PDT
(In reply to comment #6)
> I would like to preserve the possibility to go straight from tarball->MAR
> without any extra metadata if possible.

Why does that matter?  Why would you want removed-files to be in the tarball?
Comment 8 Benjamin Smedberg [:bsmedberg] 2005-09-15 11:39:18 PDT
> Why does that matter?  Why would you want removed-files to be in the tarball?

Mainly because we repackage the l10n builds from the tarball, so we need to have
all the information there to make the MAR. But also "why not"?

Checked in on trunk. I'm not quite sure how to process this list into a set of
remove instructions in make_full_update.sh (and make_incremental_update.sh), so
Darin or Chase needs to do that bit.
Comment 9 Benjamin Smedberg [:bsmedberg] 2005-09-15 11:49:35 PDT
Created attachment 196218 [details] [diff] [review]
tbird changes to match

Scott, since I'm on vacation next week could you review and land this?
Comment 10 Benjamin Smedberg [:bsmedberg] 2005-09-15 12:07:44 PDT
Created attachment 196222 [details] [diff] [review]
tbird changes to match

argh, a little too much blind copy-paste
Comment 11 Darin Fisher 2005-09-15 14:04:10 PDT
OK.  I'll hack on make_full_update.sh and make_incremental_update.sh.  I'll
probably just add a command line option to specify the path to the
"removed-files" file.
Comment 12 Benjamin Smedberg [:bsmedberg] 2005-09-15 14:09:42 PDT
Darin, that shouldn't be necessary, since removed-files is in the archive.
Comment 13 Darin Fisher 2005-09-15 14:20:57 PDT
Fair enough.  So, if I find a file by the name of removed-files in the given
directory, then I'll treat that as a list of files to be removed.
Comment 14 Darin Fisher 2005-09-15 15:11:38 PDT
I'm not sure how to handle these lines in removed-files:

  chrome/overlayinfo/
  defaults/profile/US/

updater.cpp does not understand how to recursively delete a directory.  I
suspect that asking it to remove a directory will simply fail, and it would
probably cause the update archive to not apply.  I'm considering skipping any
lines in removed-files that end with a slash.  I could extend updater.cpp to
support recursive delete, but in this case it doesn't seem worth it since these
directories should not exist anyways by the time software update runs (since
they should have been removed by the installer).  One concern I have is with
tarball and zip based over-installs.  Thoughts?
Comment 15 Mark Mentovai 2005-09-16 16:12:34 PDT
This broke packaging of non-Firefox builds on the trunk, since MOZ_PKG_REMOVALS
is currently only defined for Firefox.  A fix is in bug 308864.
Comment 16 Darin Fisher 2005-09-20 17:31:11 PDT
FYI: I've got partial patches to make_full_update.sh and
make_incremental_update.sh in my tree.  I'm hoping to find
the time to finish the changes tomorrow.
Comment 17 Darin Fisher 2005-09-22 10:25:44 PDT
Created attachment 197051 [details] [diff] [review]
v1 patch: mar packaging scripts

This patch includes changes to append the remove commands to all MAR files.
Comment 18 Chase Phillips 2005-09-22 11:07:52 PDT
Comment on attachment 197051 [details] [diff] [review]
v1 patch: mar packaging scripts

Looks good.
Comment 19 Darin Fisher 2005-09-22 11:16:51 PDT
Comment on attachment 197051 [details] [diff] [review]
v1 patch: mar packaging scripts

Requesting 1.8 branch approval.
Comment 20 Darin Fisher 2005-09-22 11:20:16 PDT
Attachment 197051 [details] [diff] is now checked in on the trunk.  It looks like the only
remaining thing to be done here is to land the patch for tbird (once mscott
approves), right?
Comment 21 Darin Fisher 2005-09-23 12:28:47 PDT
Attachment 197051 [details] [diff]: fixed1.8
Comment 22 Scott MacGregor 2005-09-23 14:37:05 PDT
I'll check in the thunderbird changes once 308864 finds its way onto the branch.
Comment 23 Benjamin Smedberg [:bsmedberg] 2005-09-26 12:40:07 PDT
Attachment 196070 [details] [diff] - fixed1.8

This is not yet entirely fixed1.8 until the tbird changes land.
Comment 24 Darin Fisher 2005-09-26 13:14:20 PDT
-> mscott for tbird changes
Comment 25 Scott MacGregor 2005-09-26 14:22:47 PDT
Comment on attachment 196222 [details] [diff] [review]
tbird changes to match

benjamin, this looked good to me. Did you intend to remove mail.jst from the
list of installer_files in windows\Makefile.in though?

I can check this in on the branch if you need me too.
Comment 26 Benjamin Smedberg [:bsmedberg] 2005-09-26 14:32:06 PDT
Yes, I removed "mail.jst" from the list because it is processed separately in
the rule below.
Comment 27 Scott MacGregor 2005-09-27 10:44:53 PDT
fixed branch and trunk.

Note You need to log in before you can comment on or make changes to this bug.