300136 - Full update archives may need to contain remove commands

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Darin Fisher]

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]

We definitely do need this.

Comment 2

[Darin Fisher]

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]

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]

Attachment: 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]

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]

I would like to preserve the possibility to go straight from tarball->MAR
without any extra metadata if possible.

Comment 7

[Darin Fisher]

(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]

> 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]

Attachment: tbird changes to match

Scott, since I'm on vacation next week could you review and land this?

Comment 10

[Benjamin Smedberg]

Attachment: tbird changes to match

argh, a little too much blind copy-paste

Comment 11

[Darin Fisher]

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]

Darin, that shouldn't be necessary, since removed-files is in the archive.

Comment 13

[Darin Fisher]

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]

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]

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]

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]

Attachment: v1 patch: mar packaging scripts

This patch includes changes to append the remove commands to all MAR files.

Comment 18

[Chase Phillips]

Comment on attachment 197051 [details] [diff] [review]
v1 patch: mar packaging scripts

Looks good.

Comment 19

[Darin Fisher]

Comment on attachment 197051 [details] [diff] [review]
v1 patch: mar packaging scripts

Requesting 1.8 branch approval.

Comment 20

[Darin Fisher]

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]

Attachment 197051 [details] [diff]: fixed1.8

Comment 22

[Scott MacGregor]

I'll check in the thunderbird changes once 308864 finds its way onto the branch.

Comment 23

[Benjamin Smedberg]

Attachment 196070 [details] [diff] - fixed1.8

This is not yet entirely fixed1.8 until the tbird changes land.

Comment 24

[Darin Fisher]

-> mscott for tbird changes

Comment 25

[Scott MacGregor]

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]

Yes, I removed "mail.jst" from the list because it is processed separately in
the rule below.

Comment 27

[Scott MacGregor]

fixed branch and trunk.