Reduce the number of locales we provide updates for in nightly

Trac:
Parent Ticket: #18867 (moved)

added TorBrowserTeam201911R actualpoints::1 component::applications/tor browser owner::boklm parent::18867 priority::medium resolution::fixed severity::normal sponsor::9 status::closed tbb-rbm tbb-update type::task ux-team labels

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?

Trac:
Cc: N/A to antonela
Keywords: N/A deleted, ux-team added
Sponsor: N/A to Sponsor9

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.

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.

Assigning more tickets to boklm for the next few months

Trac:
Owner: tbb-team to boklm
Status: new to assigned
Cc: antonela to antonela, tbb-team

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. :)

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.

Trac:
Keywords: TorBrowserTeam201911, boklm201911 deleted, TorBrowserTeam201911R added
Status: assigned to needs_review

yeah that list of locales will make most of our users

why no Chinese?

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

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.

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.

Trac:
Keywords: TorBrowserTeam201911R deleted, TorBrowserTeam201911 added
Status: needs_review to needs_revision

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?

Trac:
Status: needs_revision to needs_review
Keywords: TorBrowserTeam201911 deleted, TorBrowserTeam201911R added

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 (moved). 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.

Trac:
Keywords: TorBrowserTeam201911R deleted, TorBrowserTeam201911 added
Status: needs_review to needs_revision

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.

Trac:
Keywords: TorBrowserTeam201911 deleted, TorBrowserTeam201911R added
Status: needs_revision to needs_review

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

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

Trac:
Actualpoints: N/A to 1

closed

added 8h of time spent

moved to tpo/applications/tor-browser#32475 (closed)

Reduce the number of locales we provide updates for in nightly

Child items 0

Activity