Opened 3 weeks ago

Closed 10 days ago

Last modified 4 days ago

#32475 closed task (fixed)

Reduce the number of locales we provide updates for in nightly

Reported by: boklm Owned by: boklm
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, tbb-update, TorBrowserTeam201911R, ux-team
Cc: antonela, tbb-team Actual Points: 1
Parent ID: #18867 Points:
Reviewer: Sponsor: Sponsor9

Description

Making updates available for each locale is costing time on nightly build/signing machines:

  • generating the .mar files
  • generating the incremental .mar files
  • transferring the mar files between hosts to sign them and publish them

With the current resources we have, I think it risks increasing the time to provide updates on nightly builds too much. I think we could start by providing updates for a subset of locales only, and think about increasing that list later.

Child Tickets

Change History (16)

comment:1 Changed 3 weeks ago by pili

Cc: antonela added
Keywords: ux-team added
Sponsor: Sponsor9

Could this somehow be configured/configurable?

I'm thinking of the example where we want to test a feature with a particular audience and we want to be sure to have the right locale for the audience.

Or does it make sense to use a different system other than the automatic nightly builds for this?

comment:2 in reply to:  1 Changed 3 weeks ago by gk

Replying to pili:

Could this somehow be configured/configurable?

I'm thinking of the example where we want to test a feature with a particular audience and we want to be sure to have the right locale for the audience.

I think that's a good idea and we should do it.

Or does it make sense to use a different system other than the automatic nightly builds for this?

It's already hard to keep one system properly maintained and working, thus I don't think we should create yet another one.

comment:3 in reply to:  1 ; Changed 3 weeks ago by boklm

Replying to pili:

Could this somehow be configured/configurable?

Yes, I think we can have this list of locales in rbm.conf, and we can update it as needed.

I'm thinking of the example where we want to test a feature with a particular audience and we want to be sure to have the right locale for the audience.

Or does it make sense to use a different system other than the automatic nightly builds for this?

We would still be building all locales, so they can still all be tested. This ticket is only about automatic updates, so only some locales could be used with automatic updates. If we want a particular audience to use nightly builds every day, we can add that locale to the list, so I don't think we need a different system.

comment:4 Changed 3 weeks ago by pili

Cc: tbb-team added
Owner: changed from tbb-team to boklm
Status: newassigned

Assigning more tickets to boklm for the next few months

comment:5 in reply to:  3 Changed 3 weeks ago by gk

Replying to boklm:

Replying to pili:

Could this somehow be configured/configurable?

Yes, I think we can have this list of locales in rbm.conf, and we can update it as needed.

I'm thinking of the example where we want to test a feature with a particular audience and we want to be sure to have the right locale for the audience.

Or does it make sense to use a different system other than the automatic nightly builds for this?

We would still be building all locales, so they can still all be tested. This ticket is only about automatic updates, so only some locales could be used with automatic updates. If we want a particular audience to use nightly builds every day, we can add that locale to the list, so I don't think we need a different system.

Great. It seemed I actually mis-read the ticket first, but reducing the locales just for testing purposes first would have been a good plan anyway. :)

comment:6 Changed 2 weeks ago by boklm

Keywords: TorBrowserTeam201911R added; boklm201911 TorBrowserTeam201911 removed
Status: assignedneeds_review

There is a patch for review in branch bug_32475_v2:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v2&id=ad51083c68573527b826f11e2b834a07f7136f5e

The list of locales we build mar files for is:

  • de
  • es-ES
  • en-US
  • fr
  • ru

This seems to be the most used locales according to https://metrics.torproject.org/webstats-tb-locale.html.

comment:7 Changed 2 weeks ago by emmapeel

yeah that list of locales will make most of our users

comment:8 Changed 2 weeks ago by cypherpunks

why no Chinese?

comment:9 Changed 2 weeks ago by emmapeel

we dont have many Chinese locale Tor Browser users. They must be using the English version.

Also, this is not about Tor Browser releases, is about the nightlies only.

update pings:

english: approx 1525321
russian: approx 426981
german: approx 163276
french: approx 63067
spanish: approx 54154

chinese hant: approx 10418

comment:10 Changed 2 weeks ago by boklm

I pushed a new revision of the patch in branch bug_32475_v3:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v3&id=0327f0a0676d6a377f4f8b042bb5d34247d9cd94

This removes the hack that was used to check if a locale is in the mar_locales list.

comment:11 in reply to:  10 ; Changed 11 days ago by gk

Keywords: TorBrowserTeam201911 added; TorBrowserTeam201911R removed
Status: needs_reviewneeds_revision

Replying to boklm:

I pushed a new revision of the patch in branch bug_32475_v3:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v3&id=0327f0a0676d6a377f4f8b042bb5d34247d9cd94

This removes the hack that was used to check if a locale is in the mar_locales list.

The update generation is following our bundle generation which is kind of split in two parts. The first part is used for en-US which is shipping no lang packs. The second part is used for all the other locales which ship with the respective lang pack.

It's confusing that you only patch the second part but include en-US in the mar_locales given that en-US is *not* in var/locales. I guess we might got lucky as I can still see things working (the en-US .mar gets built in the first part as var/build_mar is defined but gets ignored in your patch because IF mar_lang == lang; is never true. I find it error-prone as well that we continue to use at one hand [% IF c("var/build_mar") -%] and at the other hand [% IF build_mar -%] for deciding whether to create full .mar files or not.

comment:12 in reply to:  11 ; Changed 11 days ago by boklm

Keywords: TorBrowserTeam201911R added; TorBrowserTeam201911 removed
Status: needs_revisionneeds_review

Replying to gk:

Replying to boklm:

I pushed a new revision of the patch in branch bug_32475_v3:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v3&id=0327f0a0676d6a377f4f8b042bb5d34247d9cd94

This removes the hack that was used to check if a locale is in the mar_locales list.

The update generation is following our bundle generation which is kind of split in two parts. The first part is used for en-US which is shipping no lang packs. The second part is used for all the other locales which ship with the respective lang pack.

Yes. I'm wondering if we could simplify that and treat en-US like the other locales, maybe as a separate ticket.

It's confusing that you only patch the second part but include en-US in the mar_locales given that en-US is *not* in var/locales. I guess we might got lucky as I can still see things working (the en-US .mar gets built in the first part as var/build_mar is defined but gets ignored in your patch because IF mar_lang == lang; is never true.

Indeed, there is no need to have en-US in var/mar_locales as it is done separately. I removed it in a fixup commit:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v3&id=f5d41e06db6a8fc575b8a111711bbf9b3135ff38

It seems also that we generate mar files for nightly builds even if var/build_mar is set to 0. I fixed that in an other fixup commit:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v3&id=4000bbdbd796ffb24766a4f327edc16bbce7aeed

And I squashed those two commits in bug_32475_v4:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v4&id=64eb73431061f1785aa791a3f779a2e1e92e1460

I find it error-prone as well that we continue to use at one hand [% IF c("var/build_mar") -%] and at the other hand [% IF build_mar -%] for deciding whether to create full .mar files or not.

Hmm, I'm not sure how to make that less confusing. In the first case (for en-US) we only need to look at c("var/build_mar"). In the second case we need to use a local variable as it is more complicate. Would renaming the variable to something else than build_mar be less confusing?

comment:13 in reply to:  12 ; Changed 10 days ago by gk

Keywords: TorBrowserTeam201911 added; TorBrowserTeam201911R removed
Status: needs_reviewneeds_revision

Replying to boklm:

Replying to gk:

Replying to boklm:

I pushed a new revision of the patch in branch bug_32475_v3:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v3&id=0327f0a0676d6a377f4f8b042bb5d34247d9cd94

This removes the hack that was used to check if a locale is in the mar_locales list.

The update generation is following our bundle generation which is kind of split in two parts. The first part is used for en-US which is shipping no lang packs. The second part is used for all the other locales which ship with the respective lang pack.

Yes. I'm wondering if we could simplify that and treat en-US like the other locales, maybe as a separate ticket.

I think we should fix that by moving to single-locale bundles which we want to do anyway for a number of reasons, see: #27466. Then the whole treatment of en-US vs. the other bundles vanishes for free. I am not sure whether it is worth filing a separate bug for the simplification part, though.

[snip]

It seems also that we generate mar files for nightly builds even if var/build_mar is set to 0. I fixed that in an other fixup commit:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v3&id=4000bbdbd796ffb24766a4f327edc16bbce7aeed

That's a good catch. However, I feel there is no need to iterate through all the locales just to hit the case that .mar files should not be built. Can't we get that change by just doing something like
IF c("var/nightly") && c("var/build_mar") once at the beginning?

[snip]

I find it error-prone as well that we continue to use at one hand [% IF c("var/build_mar") -%] and at the other hand [% IF build_mar -%] for deciding whether to create full .mar files or not.

Hmm, I'm not sure how to make that less confusing. In the first case (for en-US) we only need to look at c("var/build_mar"). In the second case we need to use a local variable as it is more complicate. Would renaming the variable to something else than build_mar be less confusing?

No, I think we should keep it that way if we need it because otherwise folks might just grep for var/build_mar while searching for the code parts they need to change and thereby missing some. This can already happen with your proposed changes if one greps for [% IF c("var/build_mar") -%] assuming all the update relevant checks are done that way. What we could do is setting build_mar earlier and then use [% IF build_mar -%] check everywhere in the tor-browser build script. But maybe it's enough someone actually working on that part of the code would just grep for build_mar, dunno. I think I could live with the status quo.

comment:14 in reply to:  13 Changed 10 days ago by boklm

Keywords: TorBrowserTeam201911R added; TorBrowserTeam201911 removed
Status: needs_revisionneeds_review

Replying to gk:


It seems also that we generate mar files for nightly builds even if var/build_mar is set to 0. I fixed that in an other fixup commit:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v3&id=4000bbdbd796ffb24766a4f327edc16bbce7aeed

That's a good catch. However, I feel there is no need to iterate through all the locales just to hit the case that .mar files should not be built. Can't we get that change by just doing something like
IF c("var/nightly") && c("var/build_mar") once at the beginning?

Ok, I made this change in branch bug_32475_v5:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_32475_v5&id=8d5805ae26b873762ec823ae7ec1180d8d89b585


[snip]

I find it error-prone as well that we continue to use at one hand [% IF c("var/build_mar") -%] and at the other hand [% IF build_mar -%] for deciding whether to create full .mar files or not.

Hmm, I'm not sure how to make that less confusing. In the first case (for en-US) we only need to look at c("var/build_mar"). In the second case we need to use a local variable as it is more complicate. Would renaming the variable to something else than build_mar be less confusing?

No, I think we should keep it that way if we need it because otherwise folks might just grep for var/build_mar while searching for the code parts they need to change and thereby missing some. This can already happen with your proposed changes if one greps for [% IF c("var/build_mar") -%] assuming all the update relevant checks are done that way. What we could do is setting build_mar earlier and then use [% IF build_mar -%] check everywhere in the tor-browser build script. But maybe it's enough someone actually working on that part of the code would just grep for build_mar, dunno. I think I could live with the status quo.

If grepping for c("var/build_mar") however, it should be possible to see that we assign it to build_mar.

comment:15 Changed 10 days ago by gk

Resolution: fixed
Status: needs_reviewclosed

Okay, let's move forward here. I merged v5 to master (commit 8d5805ae26b873762ec823ae7ec1180d8d89b585).

comment:16 Changed 4 days ago by boklm

Actual Points: 1
Note: See TracTickets for help on using tickets.