Full update archives may need to contain remove commands

RESOLVED FIXED in mozilla1.8beta4

Status

()

Core
Build Config
--
major
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Darin Fisher, Assigned: Scott MacGregor)

Tracking

({fixed1.8})

Trunk
mozilla1.8beta4
fixed1.8
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
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.
(Reporter)

Updated

12 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta4
(Reporter)

Updated

12 years ago
Flags: blocking-aviary1.1?

Updated

12 years ago
Flags: blocking-aviary1.5? → blocking1.8b4?

Updated

12 years ago
Flags: blocking1.8b4? → blocking1.8b4+

Updated

12 years ago
Flags: blocking1.8b4-
Flags: blocking1.8b4+
Flags: blocking-aviary1.5+

Updated

12 years ago
Flags: blocking1.8b5?
Flags: blocking1.8b5-
Flags: blocking-aviary1.5+
We definitely do need this.
Assignee: darin → benjamin
Status: ASSIGNED → NEW
Flags: blocking1.8b5? → blocking1.8b5+
(Reporter)

Comment 2

12 years ago
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?
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?
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.
Attachment #196070 - Flags: review?(darin)
(Reporter)

Comment 5

12 years ago
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
Attachment #196070 - Flags: review?(darin) → review+
I would like to preserve the possibility to go straight from tarball->MAR
without any extra metadata if possible.
(Reporter)

Comment 7

12 years ago
(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?
> 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.
Assignee: benjamin → darin
Attachment #196070 - Flags: approval1.8b5?
Created attachment 196218 [details] [diff] [review]
tbird changes to match

Scott, since I'm on vacation next week could you review and land this?
Attachment #196218 - Flags: review?(mscott)
Created attachment 196222 [details] [diff] [review]
tbird changes to match

argh, a little too much blind copy-paste
Attachment #196218 - Attachment is obsolete: true
Attachment #196222 - Flags: review?(mscott)
Attachment #196218 - Flags: review?(mscott)
(Reporter)

Comment 11

12 years ago
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.
Darin, that shouldn't be necessary, since removed-files is in the archive.
(Reporter)

Comment 13

12 years ago
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.
(Reporter)

Comment 14

12 years ago
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?

Updated

12 years ago
Attachment #196070 - Flags: approval1.8b5? → approval1.8b5+

Updated

12 years ago
Depends on: 308864

Comment 15

12 years ago
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.
(Reporter)

Comment 16

12 years ago
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.
(Reporter)

Comment 17

12 years ago
Created attachment 197051 [details] [diff] [review]
v1 patch: mar packaging scripts

This patch includes changes to append the remove commands to all MAR files.
Attachment #197051 - Flags: review?(chase)

Comment 18

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

Looks good.
Attachment #197051 - Flags: review?(chase) → review+
(Reporter)

Comment 19

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

Requesting 1.8 branch approval.
Attachment #197051 - Flags: approval1.8b5?
(Reporter)

Comment 20

12 years ago
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?
Status: NEW → ASSIGNED

Updated

12 years ago
Whiteboard: [needs review mscott]

Updated

12 years ago
Attachment #197051 - Flags: approval1.8b5? → approval1.8b5+
(Reporter)

Comment 21

12 years ago
Attachment 197051 [details] [diff]: fixed1.8
Keywords: fixed1.8
(Assignee)

Comment 22

12 years ago
I'll check in the thunderbird changes once 308864 finds its way onto the branch.
Attachment 196070 [details] [diff] - fixed1.8

This is not yet entirely fixed1.8 until the tbird changes land.
Keywords: fixed1.8
(Reporter)

Comment 24

12 years ago
-> mscott for tbird changes
Assignee: darin → mscott
Status: ASSIGNED → NEW
(Assignee)

Comment 25

12 years ago
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.
Attachment #196222 - Flags: review?(mscott) → review+
Yes, I removed "mail.jst" from the list because it is processed separately in
the rule below.
(Assignee)

Updated

12 years ago
Attachment #196222 - Flags: approval1.8b5+
(Assignee)

Comment 27

12 years ago
fixed branch and trunk.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Whiteboard: [needs review mscott]
You need to log in before you can comment on or make changes to this bug.