Closed
Bug 759469
Opened 13 years ago
Closed 11 years ago
Add new updater instruction to add a file if it doesn't exist in the destination.
Categories
(Toolkit :: Application Update, defect)
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)
References
Details
Attachments
(3 files, 8 obsolete files)
(deleted),
patch
|
robert.strong.bugs
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
It would be handy to be able to add a file if it doesn't exist in the destination. In the case of Bug 756325 the channel-prefs.js file was accidentally moved to a new location and since we don't update that file so release build mar file updates can be applied to beta builds without changing the channel installations before the change have the channel-prefs.js file in one location (defaults/pref/) and installations performed after the change have the channel-prefs.js file in a different location (defaults/preferences). This would at least allow us to add the file to the location we decide upon if it doesn't exist in that location. This will require adding a new manifest as well.
Assignee | ||
Comment 1•13 years ago
|
||
Assignee: nobody → robert.bugzilla
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•11 years ago
|
||
I still need to finish up the tests which I'll do in this bug and the packaging scripts which I'll do in another bug.
Attachment #628179 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #784063 -
Attachment description: bug759469 → patch rev1
Attachment #784063 -
Flags: review?(netzen)
Assignee | ||
Comment 3•11 years ago
|
||
Filed bug 900251 for updating the mar generation scripts.
Depends on: 900251
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 784063 [details] [diff] [review]
patch rev1
Messed up a couple of lines and fixed locally
> // extract the manifest
>- int rv = gArchiveReader.ExtractFile("updatev2.manifest", manifest);
>+ int rv = gArchiveReader.ExtractFile("updatev3.manifest", manifest);
> if (rv) {
>- rv = gArchiveReader.ExtractFile("update.manifest", manifest);
>+ rv = gArchiveReader.ExtractFile("updatev2.manifest", manifest);
> if (rv) {
>- LOG(("DoUpdate: error extracting manifest file"));
>- return rv;
>- }
>+ rv = gArchiveReader.ExtractFile("update.manifest", manifest);
>+ if (rv) {
>+ LOG(("DoUpdate: error extracting manifest file\n"));
>+ return rv;
>+ }
fixed locally
- }
+ }
+ }
> }
>
>@@ -3717,16 +3787,19 @@ int DoUpdate()
> action = new AddFile();
> }
> else if (NS_tstrcmp(token, NS_T("patch")) == 0) {
> action = new PatchFile();
> }
> else if (NS_tstrcmp(token, NS_T("add-if")) == 0) { // Add if exists
> action = new AddIfFile();
> }
>+ else if (NS_tstrcmp(token, NS_T("add-if-not")) == 0) { // Add if not exists
>+ action = new AddNotFile();
fixed locally
- action = new AddNotFile();
+ action = new AddIfNotFile();
>+ }
> else if (NS_tstrcmp(token, NS_T("patch-if")) == 0) { // Patch if exists
Comment 5•11 years ago
|
||
Comment on attachment 784063 [details] [diff] [review]
patch rev1
Review of attachment 784063 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/mozapps/update/updater/updater.cpp
@@ +1661,5 @@
> +
> +int
> +AddIfNotFile::Execute()
> +{
> + if (mTestFile)
This should be if (!mTestFile)
@@ +1670,5 @@
> +
> +void
> +AddIfNotFile::Finish(int status)
> +{
> + if (mTestFile)
This should be if (!mTestFile)
@@ +3717,2 @@
> if (rv) {
> + rv = gArchiveReader.ExtractFile("update.manifest", manifest);
I update.manifest even needed anymore if we are required to upgrade to an intermediate update?
@@ +3792,5 @@
> else if (NS_tstrcmp(token, NS_T("add-if")) == 0) { // Add if exists
> action = new AddIfFile();
> }
> + else if (NS_tstrcmp(token, NS_T("add-if-not")) == 0) { // Add if not exists
> + action = new AddNotFile();
seen comment thx.
Attachment #784063 -
Flags: review?(netzen)
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Brian R. Bondy [:bbondy] from comment #5)
> Comment on attachment 784063 [details] [diff] [review]
> patch rev1
>
> Review of attachment 784063 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: toolkit/mozapps/update/updater/updater.cpp
> @@ +1661,5 @@
> > +
> > +int
> > +AddIfNotFile::Execute()
> > +{
> > + if (mTestFile)
>
> This should be if (!mTestFile)
>
> @@ +1670,5 @@
> > +
> > +void
> > +AddIfNotFile::Finish(int status)
> > +{
> > + if (mTestFile)
>
> This should be if (!mTestFile)
>
> @@ +3717,2 @@
> > if (rv) {
> > + rv = gArchiveReader.ExtractFile("update.manifest", manifest);
>
> I update.manifest even needed anymore if we are required to upgrade to an
> intermediate update?
I thought I removed this in bug 896224... I'll do it here.
Assignee | ||
Comment 7•11 years ago
|
||
Comments addressed.
I still have to finish the python script for mar packaging and the tests which will be next.
Attachment #784063 -
Attachment is obsolete: true
Attachment #784436 -
Flags: review?(netzen)
Comment 8•11 years ago
|
||
Comment on attachment 784436 [details] [diff] [review]
patch rev2
Review of attachment 784436 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/mozapps/update/updater/updater.cpp
@@ +3717,2 @@
> if (rv) {
> + LOG(("DoUpdate: error extracting manifest file\n"));
nit: \n is included already
Attachment #784436 -
Flags: review?(netzen) → review+
Assignee | ||
Comment 9•11 years ago
|
||
carrying forward r+
Attachment #784436 -
Attachment is obsolete: true
Attachment #784439 -
Flags: review+
Assignee | ||
Comment 10•11 years ago
|
||
Attachment #8384537 -
Flags: review?(netzen)
Assignee | ||
Comment 11•11 years ago
|
||
Attachment #8384538 -
Flags: review?(netzen)
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #8384539 -
Flags: review?(netzen)
Assignee | ||
Comment 13•11 years ago
|
||
I have a tad more cleanup to do but wanted to get what I have so far reviewed. I'll submit the rest which is mainly marAppApply* test cleanup in another patch.
Try push
https://tbpl.mozilla.org/?tree=Try&rev=73c4aaa87094
Attachment #8384540 -
Flags: review?(netzen)
Assignee | ||
Updated•11 years ago
|
Attachment #784439 -
Attachment description: patch rev3 comment addressed → 1. main patch rev3 comment addressed
Assignee | ||
Comment 14•11 years ago
|
||
All patched pushed to try
https://tbpl.mozilla.org/?tree=Try&rev=5453ea12fdf9
Attachment #8385199 -
Flags: review?(netzen)
Assignee | ||
Comment 15•11 years ago
|
||
fixed up more of the comments
Attachment #8385199 -
Attachment is obsolete: true
Attachment #8385199 -
Flags: review?(netzen)
Attachment #8385413 -
Flags: review?(netzen)
Updated•11 years ago
|
Attachment #8384537 -
Flags: review?(netzen) → review+
Updated•11 years ago
|
Attachment #8384538 -
Flags: review?(netzen) → review+
Updated•11 years ago
|
Attachment #8384540 -
Flags: review?(netzen) → review+
Updated•11 years ago
|
Attachment #8385413 -
Flags: review?(netzen) → review+
Comment 16•11 years ago
|
||
Comment on attachment 8384539 [details] [diff] [review]
4. updated tests and cleanup
Review of attachment 8384539 [details] [diff] [review]:
-----------------------------------------------------------------
8| as far as I can tell this looks good :)
Attachment #8384539 -
Flags: review?(netzen) → review+
Assignee | ||
Comment 17•11 years ago
|
||
Combined patch for test changes in patches 3 - 6.
Attachment #8384538 -
Attachment is obsolete: true
Attachment #8384539 -
Attachment is obsolete: true
Attachment #8384540 -
Attachment is obsolete: true
Attachment #8385413 -
Attachment is obsolete: true
Attachment #8386282 -
Flags: review+
Assignee | ||
Comment 18•11 years ago
|
||
Pushed to mozilla-central
https://hg.mozilla.org/mozilla-central/rev/75aef72bf1be
https://hg.mozilla.org/mozilla-central/rev/5265254ba734
https://hg.mozilla.org/mozilla-central/rev/c1c3aa2b09d8
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
status-firefox30:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Assignee | ||
Comment 19•11 years ago
|
||
Comment on attachment 784439 [details] [diff] [review]
1. main patch rev3 comment addressed
[Approval Request Comment]
Bug caused by (feature/regressing bug #): None
User impact if declined: Releng will need to perform additional work to create update mar files for additional cycles to update beta users to release bits.
Testing completed (on m-c, etc.): This has been on m-c for several days, I've manually verified thoroughly, tests in bug 759469
Risk to taking this patch (and alternatives if risky): Minimal
String or IDL/UUID changes made by this patch: None
Attachment #784439 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
status-firefox29:
--- → affected
Updated•11 years ago
|
Attachment #784439 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 20•11 years ago
|
||
Comment 21•10 years ago
|
||
Moving milestone to 29 to reflect uplift to aurora.
Target Milestone: mozilla30 → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•