Closed Bug 1175331 Opened 5 years ago Closed 4 years ago

Adjust automation for new file browser/config/version_display.txt

Categories

(Release Engineering :: Release Automation: Other, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: nthomas, Assigned: Sylvestre)

References

Details

Attachments

(5 files, 4 obsolete files)

Bug 1145175 (and the followup bug 1174506) changes the format of browser/config/version.txt, and how it is used. The primary aim is to add the 'release automation' version (eg 39.0b6) to the about: page, but it also shifts setting the version into configure rather than runtime for some make targets.

This has implications for tagging releases, and merge days.
When tagging, we bump browser/config/version.txt for both Firefox & Fennec. The relevant code is at
 http://hg.mozilla.org/build/tools/file/default/scripts/release/tag-release.py#l82
which ends up at
 http://hg.mozilla.org/build/tools/file/default/lib/python/build/versions.py#l28
driven by configs like
 http://hg.mozilla.org/build/buildbot-configs/file/default/mozilla/release-firefox-mozilla-release.py#l72
 http://hg.mozilla.org/build/buildbot-configs/file/default/mozilla/release-fennec-mozilla-release.py#l39

We need to support doing more than one replacement in bumpFiles(), which I think this blocks uplifting to beta.

For Fennec we also bump mobile/android/confvars.sh, but it looks like bug 1174506 will obsolete that. We'll need to update the config templates as the new code is uplifted or merged to release branches though.
I'm less familiar with merging, but mxr says the code to look at is
 http://hg.mozilla.org/build/mozharness/file/254c9420f7bb/scripts/merge_day/gecko_migration.py#l312
and its callers.
Will this affect RCs somehow?
(In reply to Nick Thomas [:nthomas] from comment #2)
> I'm less familiar with merging, but mxr says the code to look at is
>  http://hg.mozilla.org/build/mozharness/file/254c9420f7bb/scripts/merge_day/
> gecko_migration.py#l312
> and its callers.

I think it should just work, because string.replace(...) used in http://hg.mozilla.org/build/mozharness/file/254c9420f7bb/scripts/merge_day/gecko_migration.py#l282 replaces all occurrences unless you limit it.
(In reply to Rail Aliiev [:rail] from comment #4)
> (In reply to Nick Thomas [:nthomas] from comment #2)
> > I'm less familiar with merging, but mxr says the code to look at is
> >  http://hg.mozilla.org/build/mozharness/file/254c9420f7bb/scripts/merge_day/
> > gecko_migration.py#l312
> > and its callers.
> 
> I think it should just work, because string.replace(...) used in
> http://hg.mozilla.org/build/mozharness/file/254c9420f7bb/scripts/merge_day/
> gecko_migration.py#l282 replaces all occurrences unless you limit it.

... unless we want to keep one of the lines unchanged....
Oh hey, things moved again. Bug 1175331 moved the new version to browser/config/version_about.txt. So the existing tagging and merging code will be fine for version.txt, and we just need to add support for version_about.txt.

Lets start by defining the values it will hold, and when+how it will change, as the code is merged up to release.

mozilla-central: 41.0a1     (whole cycle)
mozilla-aurora:  41.0a2     (whole cycle)
mozilla-beta:    41.0b1, 41.0b2, 41.0b3, ...
mozilla-release: 41.0, 41.0.1, ...

So lets say the merge script sets the (initial) value when merging to aurora, beta, and release. Then tagging will bump the version on the relbranch and default to the next version. Bonus points for actually bumping the version on default to the higher value, making the relbranch a no-op.

re RCs, we couldn't put 41.0 RC1, in case we end up shipping it.
Summary: Adjust automation for new format of browser/config/version.txt → Adjust automation for new file browser/config/version_about.txt
Depends on: 1174506
I was planning to propose patches for all that but thanks for taking care of it!
Attached patch bug-1175331.diff (obsolete) — Splinter Review
Not sure that updating the *release.py is necessary but I guess that is better.
Attachment #8642993 - Flags: review?(nthomas)
Assignee: nobody → sledru
Comment on attachment 8642993 [details] [diff] [review]
bug-1175331.diff

Review of attachment 8642993 [details] [diff] [review]:
-----------------------------------------------------------------

I think the changes look fine, except you needed to modify the .template of each of these.
Attachment #8642993 - Flags: review?(nthomas) → review-
I looked again at the gecko migration code, and it'll need some mods too. See calls to bump_version() in 
 http://hg.mozilla.org/build/mozharness/file/default/scripts/merge_day/gecko_migration.py#l312
Sure, here it is!
Attachment #8642993 - Attachment is obsolete: true
Attachment #8643603 - Flags: review?(nthomas)
Attached patch bug-1175331-3.diff (obsolete) — Splinter Review
Nick, I also tried to update gecko_migration.py, I guess I add the file to the list. The buildbot config will set the correct value with the beta number.

I also took the liberty to remove some old stuff about version_about.txt, it seems it has been there for a long time.
Attachment #8643608 - Flags: review?(nthomas)
Attachment #8643603 - Flags: review?(nthomas) → review+
Comment on attachment 8643608 [details] [diff] [review]
bug-1175331-3.diff

Review of attachment 8643608 [details] [diff] [review]:
-----------------------------------------------------------------

It looks like we'll have version_display.txt on every branch we merge from going forward, so this seems OK. f+ though because I think it's going to fail to do the right thing for aurora --> beta at http://hg.mozilla.org/build/mozharness/file/default/scripts/merge_day/gecko_migration.py#l392 - 
  self.bump_version(dirs['abs_to_dir'], mb_version, mb_version, "a2", "")
We want mb_version + 'b1' in version_display.txt then, IIUC.

And at beta --> release we need to remove the 'b1'.
Attachment #8643608 - Flags: review?(nthomas) → feedback+
Attachment #8643603 - Flags: checked-in+
Here it is.
I updated beta_to_release but I was wondering if it is really necessary as the RC build will remove it anyway ?! (but I don't know enough the mechanism here)
Attachment #8643608 - Attachment is obsolete: true
Attachment #8644975 - Flags: review?(nthomas)
Comment on attachment 8644975 [details] [diff] [review]
bug-1175331-4.diff

Review of attachment 8644975 [details] [diff] [review]:
-----------------------------------------------------------------

The main reason I'd like to set the values correctly at merge time is the upcoming release promotion work. Adding a version_files=None argument to bump_version() would be one way of treating version_display.txt differently from version.txt and co.

Since release promotion isn't here yet, and you're right that the release automation will do the right thing for now, lets let rail be the tie-breaker. This is his code really.
Attachment #8644975 - Flags: review?(nthomas) → review?(rail)
Summary: Adjust automation for new file browser/config/version_about.txt → Adjust automation for new file browser/config/version_display.txt
Blocks: 1193201
Blocks: 1193221
We hit an error starting 41.0b1, here's the end of the traceback
 File "/builds/releaserunner/tools/lib/python/release/info.py", line 87, in readConfig
    execfile(configfile, c)
  File "buildbot-configs/mozilla/release-fennec-mozilla-beta.py", line 46, in <module>
    'nextVersion': releaseConfig['nextVersion']
KeyError: 'nextVersion'
Attachment #8646602 - Flags: review?(jlund)
Comment on attachment 8646602 [details] [diff] [review]
[buildbot-configs] Define nextVersion in beta configs

Review of attachment 8646602 [details] [diff] [review]:
-----------------------------------------------------------------

not grepping exactly what's going on but if you want nextVersion to point to current version, this should do it
Attachment #8646602 - Flags: review?(jlund) → review+
Blocks: 1188493
Comment on attachment 8643603 [details] [diff] [review]
bug-1175331-2.diff

I needed to back out the release part of this for bustage tagging 40.0.1 in m-r:

https://hg.mozilla.org/build/buildbot-configs/rev/2e2ca875ed57
https://hg.mozilla.org/build/buildbot-configs/rev/1a5ddd86a052

Lets reland it after the next merge, bug 1190766 is the tracker for that.
And the live config version too   (this is a one-off)
https://hg.mozilla.org/build/buildbot-configs/rev/73ef44d7f7ae
Comment on attachment 8644975 [details] [diff] [review]
bug-1175331-4.diff

Review of attachment 8644975 [details] [diff] [review]:
-----------------------------------------------------------------

The patch didn't apply for me. Probably it's based on an outdated version. Additionaly, mozharness lives in the tree now.

::: scripts/merge_day/gecko_migration.py
@@ +306,5 @@
>          curr_weave_version = str(int(curr_version) + 2)
>          next_weave_version = str(int(curr_weave_version) + 1)
> +        version_files = ["browser/config/version.txt", "browser/config/version_display.txt",
> +                         "config/milestone.txt", "mobile/android/confvars.sh",
> +                         "b2g/confvars.sh"]

This hunk should be handled already. See https://dxr.mozilla.org/mozilla-central/rev/38c1ea9ccae31700630f1fe0d651e94c0c5b9e1d/testing/mozharness/scripts/merge_day/gecko_migration.py#324 through line 330.

@@ +402,5 @@
>              staging beta user repo migrations.
>              """
>          dirs = self.query_abs_dirs()
>          mb_version = self.get_fx_major_version(dirs['abs_to_dir'])
> +        self.bump_version(dirs['abs_to_dir'], mb_version, mb_version, "a2", "b1")

This means that you'll have the "b1" suffix in all files bumped by this method, including confvars.sh, version.txt, etc. Not sure if this is what we want to do. I believe this will affect a lot of things, including addons. I'd probably add a new parameter to bump_version() and call it "display_version_suffix" and set it to "" or None by default.
Attachment #8644975 - Flags: review?(rail) → review-
Bug 1204796 as a result of forgetting to reland the release config changes. Landed at
  https://hg.mozilla.org/build/buildbot-configs/rev/409130f84fdd     (default)
  https://hg.mozilla.org/build/buildbot-configs/rev/636756358ce7     (production)
so that we're good for 41.0 build2 and later.
I updated also release, not sure I should?!
Attachment #8661715 - Flags: review?(nthomas)
Comment on attachment 8661715 [details] [diff] [review]
bug-1175331-remove-android-confvars.diff

The patch looks fine, but probably you should split into two parts. We can land the beta half next week, when bug 1196373 merges there, and the release part 6 weeks later. I'm assuming no further uplifting will be done for 1196373.

We should hook this bug up to the tracking bugs for to mergers. If we forget the file bumper will just not match on anything, AFAICT, and carry on without error.
Attachment #8661715 - Flags: review?(nthomas)
I split Sylvetre's patch into 2 files and removed *.py changes (they will be generated by the templates).

We'll need to merge this to production before the build.
Attachment #8661715 - Attachment is obsolete: true
Attachment #8663708 - Flags: review+
Attached patch android_confvars.sh-beta.diff (obsolete) — Splinter Review
Attachment #8663709 - Flags: review+
Depends on: 1196373
Blocks: 1196183
No longer blocks: 1188493
Err, submitted twice.
Attachment #8663709 - Attachment is obsolete: true
Attachment #8663768 - Flags: review+
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
See Also: → 1271153
You need to log in before you can comment on or make changes to this bug.