302721 - Implement update service channels

Status: VERIFIED FIXED

(Toolkit :: Application Update, defect)

Description

[Darin Fisher]

Implement update service channels

Comment 1

[Darin Fisher]

Attachment: v1 patch

This is probably an incomplete patch.  I just wanted to post something so
people could get an idea of what this might look like.	Right now, I'm naming
the channel "default" when update-channel.txt cannot be read.  I thought about
simply using "nightly" as the default, but this seems better since we probably
don't want file read errors and the like to end up being treated as nightly
updates.  This patch does not include changes to insert update-channel.txt into
the app directory.  I'll do that as a follow-up patch.

Chase: can you please review the value for app.update.url.  Thanks!

Comment 2

[Chase Phillips]

Comment on attachment 191014 [details] [diff] [review]
v1 patch

>Index: toolkit/locales/en-US/chrome/mozapps/update/updates.properties
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/update/updates.properties,v
>retrieving revision 1.22
>diff -p -u -1 -0 -r1.22 updates.properties
>--- toolkit/locales/en-US/chrome/mozapps/update/updates.properties	7 Jul 2005 05:49:44 -0000	1.22
>+++ toolkit/locales/en-US/chrome/mozapps/update/updates.properties	30 Jul 2005 01:39:14 -0000
...
>-app.update.url=https://aus.mozilla.org/update2/0/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/update.xml
>+app.update.url=https://aus.mozilla.org/update2/1/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/update.xml

Looks good to me.  Once AUS 2's dedicated hostname is up and running we'll need
another patch to change the default hostname to aus2.mozilla.org, but that's a
couple of days away atm.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

might it be better to put the channel name first, so that all files for a
channel are in the same directory hierarchy?

Comment 4

[Darin Fisher]

> might it be better to put the channel name first, so that all files for a
> channel are in the same directory hierarchy?

The path doesn't actually correspond to a directory hierarchy.

Comment 5

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 191014 [details] [diff] [review]
v1 patch

r=[e-mail]

Comment 6

[Chase Phillips]

Comment on attachment 191014 [details] [diff] [review]
v1 patch

a=chase to land once the server side is capable of handling the new format.

We should change the Software Update wiki to reflect the new format.

Comment 7

[Michael Morgan [:morgamic]]

I will be working on getting the server side up-to-date this afternoon so it can
properly handle the new URL scheme.  I will test that on chameleon and port it
over to its permanent home once that exists.

Comment 8

[Michael Morgan [:morgamic]]

aus-staging on chameleon should have the necessary behavioral changes.  I tested
this in my sandbox.

Here are our cases:

a) Default, any crappy URL or maligned inputs, unknown versions, build of
patch/update is not newer or lack of data for given args:
https://update-staging.mozilla.org:8711/update2/991238128/foofoobear/

b) URL Scheme 0, update found, build for patch is newer:
https://update-staging.mozilla.org:8711/update2/0/Firefox/1.0+/20020101/Linux_x86-gcc3/en-US/nightly/update.xml

c) URL Scheme 1, channel != 'nightly':
https://update-staging.mozilla.org:8711/update2/1/Firefox/1.0+/20020101/Linux_x86-gcc3/en-US/default/update.xml
https://update-staging.mozilla.org:8711/update2/1/Firefox/1.0+/20020101/Linux_x86-gcc3/en-US/foofoobear/update.xml

d) URL Scheme 1, channel == 'nightly', update data found, build for patch is newer:
https://update-staging.mozilla.org:8711/update2/1/Firefox/1.0+/20020101/Linux_x86-gcc3/en-US/nightly/update.xml

Please test this behavior and see if this is the desired output for each
scenario.  Any changes needed can be made pretty easily - just say the word.  :)

Comment 9

[Michael Morgan [:morgamic]]

Arg.  s/update-staging/aus-staging/g for the previous post.  Sorry about that.

Comment 10

[Darin Fisher]

Thanks for setting this up Mike.  I'll give it a test-spin and report back here
shortly.

Comment 11

[Darin Fisher]

It seems to work great!

Comment 12

[Darin Fisher]

OK, I'm working on a patch now to insert update-channel.txt to our builds.  It
will start out with a value of "default".  Later on, we can change this to
"nightly" when we are ready to open up the update service to all of our nightly
testers *evil-grin*

Comment 13

[Darin Fisher]

Attachment: Add update-channel.txt to the builds

This patch is needed in addition to the previous patch.

Comment 14

[Darin Fisher]

Comment on attachment 191280 [details] [diff] [review]
Add update-channel.txt to the builds

The changes to Makefile.in are just copied from netwerk/test/Makefile.in.  I
don't know if I'm using the preferred mechanism of installing additional files
into dist/bin, but this definitely works ;-)

Comment 15

[Chase Phillips]

Comment on attachment 191280 [details] [diff] [review]
Add update-channel.txt to the builds

r+.  Good that you cc'd bsmedberg on this bug.	He may see a cleaner way to
install update-channel.txt in toolkit/mozapps/update/src/Makefile.in.

bsmedberg, have a preferance?

Comment 16

[Benjamin Smedberg]

Comment on attachment 191280 [details] [diff] [review]
Add update-channel.txt to the builds

fwiw, the build-fu looks good to me... is there a particular reason we're using
a file instead of a pref?

Comment 17

[Darin Fisher]

> fwiw, the build-fu looks good to me... is there a particular reason we're 
> using a file instead of a pref?

My reasoning is (was) the following:

1) I don't want to allow prefs.js to override the setting since that is sticky.

2) I want to make it very easy for Chase to change the update channel through a
build repackaging step.

It occurs to me now that I could solve #1 by making the code only read the
default pref branch, and I could solve #2 by using a new pref file.  Do you
think that would be preferrable?

Comment 18

[Darin Fisher]

The more I think about this the more I like using a default-only pref...  :-/

Comment 19

[Benjamin Smedberg]

I'm not sure I understand "since that is sticky"... why wouldn't we want to give
users the ability to choose a new update channel?

Comment 20

[Darin Fisher]

(In reply to comment #19)
> I'm not sure I understand "since that is sticky"... why wouldn't we want to 
> give users the ability to choose a new update channel?

We do want to give them that ability, but we do not want it to be stored in
their profile.  Otherwise, suppose they uninstall a nightly build of Firefox,
and then install a release build.  If while using the nightly build, they
configure the update channel, then that would end up applying to the release
build as well, which is not what we want.  Since a software update applies to
the application directory, it makes sense to record the channel there.  Our plan
is to make it so that users do not have to select a channel, but instead it is
selected by virtue of what they choose to download:

  * If they download a nightly build, then they are on a nightly channel.
  * If they download a beta build, then they are on a beta channel.
  * If they download a release build, then they are on a release channel.

Finally, the channel can be thought of as applying to the current branch.  So,
if the user has a build from the 1.5 branch, with a nightly channel, then they
will receive nightly updatees of 1.5 branch builds.  If they are using a beta
channel, then they will receive updates to our 1.5.x release candidate builds.

Sound good?

Comment 21

[Jaime Mitchell (use [e-mail] for email)]

What about a local build? Does it/is it possible to ensure that a local build
does not get auto updated?

For example you want the release/nightly build to update itself and you want to
use the same profile with a debug build but don't want the auto update to wipe
out the debug build.
(although I admit that this is an edge case)

Comment 22

[Darin Fisher]

> What about a local build? Does it/is it possible to ensure that a local build
> does not get auto updated?

No worries.  With the patch in this bug, the update channel has a value of
"default", which is ignored by AUS.  As a developer, you could also simply
disable software update in your local builds.  Finally, if we successfully rid
the tree of the name "firefox" for non-official builds, then the URL sent to AUS
will not include the string "firefox" as a product token, so it will not know
how to update the local build.

Comment 23

[Darin Fisher]

Attachment: v2 patch (combined, using default pref branch)

Comment 24

[Darin Fisher]

Comment on attachment 191370 [details] [diff] [review]
v2 patch (combined, using default pref branch)

I think I prefer to land this patch over the previous one if nobody has any
objections.

Comment 25

[Darin Fisher]

Attachment: v2.1 patch (same as v2 patch, but includes updates.properties)

Comment 26

[Chase Phillips]

(In reply to comment #8)
> b) URL Scheme 0, update found, build for patch is newer:
>
https://update-staging.mozilla.org:8711/update2/0/Firefox/1.0+/20020101/Linux_x86-gcc3/en-US/nightly/update.xml

I noticed today that this URL shows scheme 0 and includes channel information. 
Is there any possibility for the channel data to influence whether a patch is
sent in scheme 0?  Since clients should not be sending channel data with a
scheme 0 URL, we can and should simplify our testing to focus on scheme 0 URLs
without channel data.

Comment 27

[Michael Morgan [:morgamic]]

(In reply to comment #26)
> Is there any possibility for the channel data to influence whether a patch is
> sent in scheme 0?  
Since scheme 0 doesn't know what an update channel is, anything past the locale
is just ignored for this case.  The behavior of update2/0.* should remain
consistent with the previous version of AUS Lite.

> Since clients should not be sending channel data with a
> scheme 0 URL, we can and should simplify our testing to focus on scheme 0 URLs
> without channel data.
I am assuming this is a matter of practice on the QA side of things and has
nothing to do with the configuration of AUS Lite?

Comment 28

[Darin Fisher]

Attachment: v3 patch

OK, this version of the patch implements the following approach:

  o  read app.update.url from the default pref branch.
  o  allow app.update.url.override to be user-settable and subject to regexps.
  o  change app.update.url for new spec.

Comment 29

[Darin Fisher]

I think this should be a beta blocker.

Comment 30

[Benjamin Smedberg]

Comment on attachment 192048 [details] [diff] [review]
v3 patch

Maybe I missed something between there and here: the update URL doesn't need to
be localized (and it's been proven a bad idea to have localized URLs). Why not
put the URL directly into firefox.js instead of doing the localized-pref
indirection?

I'm sorry if I didn't realize that we were doing this earlier.

Comment 31

[Darin Fisher]

Attachment: v3.1 patch [checked in]

This version of the patch eliminates pref localized strings.

Comment 32

[Darin Fisher]

The v3.1 patch does not implement locale substitution for app.update.url.manual
yet.  The code to apply substitutions on app.update.url lives in
nsUpdateService.js.in, but the code that handles app.update.url.manual lives in
updates.js.  So, I decided not to copy that code or introduce a new API on
nsIApplicationUpdateService until we figure out how we want the manual URL to be
formated.  Maybe we'll just decide to stick it in firefox-l10n.js afterall.

Comment 33

[Darin Fisher]

Chase: please let me know when you are ready for me to land this.  It will cause
everyone's updates for firefox nightlies to fail until we do the re-packaging fu
to write the nightly channel pref file.

Comment 34

[Darin Fisher]

... which will just encourage people to set app.update.url.override, so I really
don't want to pull the trigger until we are completely ready.

Comment 35

[jshepard04]

ok great that you have a multiple channel,

What would be nice is a channel for mozilla1.8 branch, if there is already one,
please tell me what it is.

Comment 36

[Ben Goodger (use ben at mozilla dot org for email)]

Chase - I can land this for Darin if everything is ready to receive it, just say
the word. 

Comment 37

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: patch to add channel-prefs.js

this sets the nightly channel by default

Comment 38

[Chase Phillips]

Comment on attachment 192129 [details] [diff] [review]
v3.1 patch [checked in]

Close to landing this on branch and trunk.  We need a corresponding patch
cooked up which adds a default update channel pref file to an installation. 
The value of the channel in the patch should be "nightly".

Comment 39

[Scott MacGregor]

(In reply to comment #37)
> Created an attachment (id=193519) [edit]
> patch to add channel-prefs.js
> 
> this sets the nightly channel by default

Ben, what's the motiviation for putting this in a pref file that's separate from
the application default prefs file (i.e. firefox.js, all-thunderbird.js)?

Comment 40

[Scott MacGregor]

Attachment: similar patch for Thunderbird

note: we don't have to add any packaging steps for this because we package all
of the pref files in bin\defaults\pref for a thunderbird build

bin\defaults\pref\*

http://lxr.mozilla.org/mozilla/source/mail/installer/windows/packages-static#133

Comment 41

[Benjamin Smedberg]

Hold on here. We should *not* be setting the update channel to nightly in the
default (developer) build. It should only be set to nightly on the tinderbox or
through repackaging. The reason that these prefs are not in all-thunderbird.js
or firefox.js is because we need to be able to set the pref separately and
specially for nightly/beta/whatever builds.

Comment 42

[Scott MacGregor]

in answer to benjamin's comment, what if we wrapped the default preference with
a #ifndef DEBUG and then pre-process the file?

Comment 43

[Chase Phillips]

(In reply to comment #39)
> Ben, what's the motiviation for putting this in a pref file that's separate from
> the application default prefs file (i.e. firefox.js, all-thunderbird.js)?

  1. We only ever read the channel pref from the default branch, so if a user 
     wants to be on another channel they need to update the channel pref in 
     their installation.

  2. We want the ability to update firefox.js/thunderbird.js either by patch 
     or complete.  If we stored the channel in one of those files, we would 
     overwrite it if we slammed the old version and would fail if the user 
     altered their channel pref value manually and we were patching.

  3. We want the channel to be in the payload of the installation and not ever 
     be a part of the update.  If someone downloads "Firefox 1.5 Beta", we 
     could start them on the "beta" channel and they would stay on it across
     updates.

  4. Automated/scripted repackaging is easier when only needing to update a 
     simpler/smaller .js file.

(In reply to comment #41)
> Hold on here. We should *not* be setting the update channel to nightly in the
> default (developer) build. It should only be set to nightly on the tinderbox or
> through repackaging.

Sure.  We talked about a configure flag for this 1-2 weeks ago.  I can set that
flag on the build systems and without the flag the value could be "default". 
Does that address your concerns, Benjamin?

Comment 44

[Benjamin Smedberg]

Chase, yes, I didn't realize you were still talking about the configure flag.

Comment 45

[Chase Phillips]

Attachment: add --enable-update-channel to browser+mail (trunk), v. 1

Comment 46

[Chase Phillips]

Attachment: add --enable-update-channel to browser+mail, v. 1.1 (trunk)

Comment 47

[Chase Phillips]

Attachment: add --enable-update-channel to browser+mail, v. 1.1 (mozilla 1.8)

Same as trunk patch just applied against the branch.

Comment 48

[Benjamin Smedberg]

Comment on attachment 193643 [details] [diff] [review]
add --enable-update-channel to browser+mail, v. 1.1 (trunk)

This wouldn't work without something else. Perhaps you meant AC_DEFINE_UNQUOTED
instead of AC_SUBST in configure.in? If not, you need to manually add
MOZ_UPDATE_CHANNEL to DEFINES in the makefiles.

Comment 49

[Chase Phillips]

(In reply to comment #48)
> (From update of attachment 193643 [details] [diff] [review] [edit])
> This wouldn't work without something else. Perhaps you meant AC_DEFINE_UNQUOTED
> instead of AC_SUBST in configure.in? If not, you need to manually add
> MOZ_UPDATE_CHANNEL to DEFINES in the makefiles.

I see.  I'm testing out a new version of the patch which addresses this but I'm
out of steam for tonight.  I can pick this up tomorrow morning and we can force
respins as desired.  Thanks for the feedback, Benjamin.

Comment 50

[Chase Phillips]

Attachment: add --enable-update-channel for browser+mail, v. 1.2 (trunk) [checked in]

Fix the incorrect substitution.

Tested this with no flag on Firefox+Thunderbird and flag=nightly for
Thunderbird.  All worked.  Build running to test Firefox with flag=nightly.

Comment 51

[Benjamin Smedberg]

Comment on attachment 193749 [details] [diff] [review]
add --enable-update-channel for browser+mail, v. 1.2 (trunk) [checked in]

+AC_DEFINE_UNQUOTED(MOZ_UPDATE_CHANNEL, $MOZ_UPDATE_CHANNEL)

You may need to remove the space... otherwise this looks fine.

Comment 52

[Chase Phillips]

Attachment: add --enable-update-channel for browser+mail, v. 1.2 (mozilla 1.8) [checked in]

Comment 53

[Chase Phillips]

Comment on attachment 193766 [details] [diff] [review]
add --enable-update-channel for browser+mail, v. 1.2 (mozilla 1.8) [checked in]

Carrying over r+sr+a from trunk patch for this 1.8b4 blocker.

Comment 54

[Chase Phillips]

Comment on attachment 193749 [details] [diff] [review]
add --enable-update-channel for browser+mail, v. 1.2 (trunk) [checked in]

(this patch is for the trunk)

Comment 55

[Chase Phillips]

Comment on attachment 193749 [details] [diff] [review]
add --enable-update-channel for browser+mail, v. 1.2 (trunk) [checked in]

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1515; previous revision: 1.1514
done
Checking in browser/app/Makefile.in;
/cvsroot/mozilla/browser/app/Makefile.in,v  <--  Makefile.in
new revision: 1.88; previous revision: 1.87
done
Checking in browser/app/profile/channel-prefs.js;
/cvsroot/mozilla/browser/app/profile/channel-prefs.js,v  <--  channel-prefs.js
new revision: 1.2; previous revision: 1.1
done
Checking in browser/installer/unix/packages-static;
/cvsroot/mozilla/browser/installer/unix/packages-static,v  <--	packages-static
new revision: 1.53; previous revision: 1.52
done
Checking in browser/installer/windows/packages-static;
/cvsroot/mozilla/browser/installer/windows/packages-static,v  <-- 
packages-static
new revision: 1.55; previous revision: 1.54
done
Checking in mail/app/Makefile.in;
/cvsroot/mozilla/mail/app/Makefile.in,v  <--  Makefile.in
new revision: 1.48; previous revision: 1.47
done
RCS file: /cvsroot/mozilla/mail/app/profile/channel-prefs.js,v
done
Checking in mail/app/profile/channel-prefs.js;
/cvsroot/mozilla/mail/app/profile/channel-prefs.js,v  <--  channel-prefs.js
initial revision: 1.1
done

Comment 56

[Chase Phillips]

Comment on attachment 193766 [details] [diff] [review]
add --enable-update-channel for browser+mail, v. 1.2 (mozilla 1.8) [checked in]

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1503.2.3; previous revision: 1.1503.2.2
done
Checking in browser/app/Makefile.in;
/cvsroot/mozilla/browser/app/Makefile.in,v  <--  Makefile.in
new revision: 1.85.2.4; previous revision: 1.85.2.3
done
Checking in browser/app/profile/channel-prefs.js;
/cvsroot/mozilla/browser/app/profile/channel-prefs.js,v  <--  channel-prefs.js
new revision: 1.1.2.3; previous revision: 1.1.2.2
done
Checking in browser/installer/unix/packages-static;
/cvsroot/mozilla/browser/installer/unix/packages-static,v  <--	packages-static
new revision: 1.50.2.4; previous revision: 1.50.2.3
done
Checking in browser/installer/windows/packages-static;
/cvsroot/mozilla/browser/installer/windows/packages-static,v  <-- 
packages-static
new revision: 1.53.2.3; previous revision: 1.53.2.2
done
Checking in mail/app/Makefile.in;
/cvsroot/mozilla/mail/app/Makefile.in,v  <--  Makefile.in
new revision: 1.46.2.2; previous revision: 1.46.2.1
done
Checking in mail/app/profile/channel-prefs.js;
/cvsroot/mozilla/mail/app/profile/channel-prefs.js,v  <--  channel-prefs.js
new revision: 1.1.2.2; previous revision: 1.1.2.1
done

Comment 57

[Chase Phillips]

Comment on attachment 192129 [details] [diff] [review]
v3.1 patch [checked in]

Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--	firefox.js
new revision: 1.74; previous revision: 1.73
done
Checking in mail/app/profile/all-thunderbird.js;
/cvsroot/mozilla/mail/app/profile/all-thunderbird.js,v	<--  all-thunderbird.js
new revision: 1.46; previous revision: 1.45
done
Checking in toolkit/locales/en-US/chrome/mozapps/update/updates.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/update/updates.properties
,v  <--  updates.properties
new revision: 1.25; previous revision: 1.24
done
Checking in toolkit/mozapps/update/content/updates.js;
/cvsroot/mozilla/toolkit/mozapps/update/content/updates.js,v  <--  updates.js
new revision: 1.46; previous revision: 1.45
done
Checking in toolkit/mozapps/update/src/Makefile.in;
/cvsroot/mozilla/toolkit/mozapps/update/src/Makefile.in,v  <--	Makefile.in
new revision: 1.8; previous revision: 1.7
done
Checking in toolkit/mozapps/update/src/nsUpdateService.js.in;
/cvsroot/mozilla/toolkit/mozapps/update/src/nsUpdateService.js.in,v  <-- 
nsUpdateService.js.in
new revision: 1.78; previous revision: 1.77
done

Comment 58

[Chase Phillips]

Comment on attachment 192129 [details] [diff] [review]
v3.1 patch [checked in]

Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--	firefox.js
new revision: 1.71.2.3; previous revision: 1.71.2.2
done
Checking in mail/app/profile/all-thunderbird.js;
/cvsroot/mozilla/mail/app/profile/all-thunderbird.js,v	<--  all-thunderbird.js
new revision: 1.44.2.2; previous revision: 1.44.2.1
done
Checking in toolkit/locales/en-US/chrome/mozapps/update/updates.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/update/updates.properties
,v  <--  updates.properties
new revision: 1.24.2.1; previous revision: 1.24
done
Checking in toolkit/mozapps/update/content/updates.js;
/cvsroot/mozilla/toolkit/mozapps/update/content/updates.js,v  <--  updates.js
new revision: 1.45.2.1; previous revision: 1.45
done
Checking in toolkit/mozapps/update/src/Makefile.in;
/cvsroot/mozilla/toolkit/mozapps/update/src/Makefile.in,v  <--	Makefile.in
new revision: 1.7.2.1; previous revision: 1.7
done
Checking in toolkit/mozapps/update/src/nsUpdateService.js.in;
/cvsroot/mozilla/toolkit/mozapps/update/src/nsUpdateService.js.in,v  <-- 
nsUpdateService.js.in
new revision: 1.77.2.1; previous revision: 1.77
done

Comment 59

[Chase Phillips]

Attachment: use simpler form of AUS2 update URL [checked in]

a=[e-mail] via rl

Comment 60

[Chase Phillips]

Comment on attachment 193786 [details] [diff] [review]
use simpler form of AUS2 update URL [checked in]

Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--	firefox.js
new revision: 1.71.2.4; previous revision: 1.71.2.3
done
Checking in mail/app/profile/all-thunderbird.js;
/cvsroot/mozilla/mail/app/profile/all-thunderbird.js,v	<--  all-thunderbird.js
new revision: 1.44.2.3; previous revision: 1.44.2.2
done

Comment 61

[Chase Phillips]

Comment on attachment 193786 [details] [diff] [review]
use simpler form of AUS2 update URL [checked in]

Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--	firefox.js
new revision: 1.75; previous revision: 1.74
done
Checking in mail/app/profile/all-thunderbird.js;
/cvsroot/mozilla/mail/app/profile/all-thunderbird.js,v	<--  all-thunderbird.js
new revision: 1.47; previous revision: 1.46
done