Closed
Bug 1329355
Opened 8 years ago
Closed 8 years ago
Remove pretty-* automation steps if they're no longer used
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox54 fixed)
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: chmanchester, Assigned: mshal)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
`MOZ_AUTOMATION_PRETTY` doesn't appear to be in use anywhere. If not, we should remove the associated automation steps.
Assignee | ||
Comment 1•8 years ago
|
||
It looks like we stopped using MOZ_AUTOMATION_PRETTY in bug 1121000 when non-unified builds were removed. IIRC we were using it there as a way to check that all the pretty targets worked, so things wouldn't break for release builds where the pretty targets were actually used. So at the very least we can get rid of the automation/pretty-foo stuff in moz-automation.mk
What I can't quite figure out is if the top-level 'make pretty-foo' targets or 'make foo MOZ_PKG_PRETTYNAMES=1' are still used anywhere. From buildbot-configs it looks like two windows build types set test_pretty_names=True, though I can't find any logs that seem to actually contain these steps. The ReleaseBuildFactory also sets MOZ_PKG_PRETTYNAMES=1, though I think now that we have release promotion, that factory is no longer used - :rail, can you confirm whether we still need these targets and/or MOZ_PKG_PRETTYNAMES=1 support? If not, we can probably delete a lot more things than just the moz-automation.mk rules.
Flags: needinfo?(rail)
Assignee | ||
Comment 2•8 years ago
|
||
Actually, the top-level 'make pretty-package-names' targets were specifically added for the moz-automation stuff, so that can be removed. But I'm still curious if the MOZ_PKG_PRETTYNAMES logic is still used in buildbot.
Comment 3•8 years ago
|
||
I think we stopped using pretty names in release automation, but we may still use them for nightly builds on m-a and m-c. Also, we run pretty name tests, see https://dxr.mozilla.org/build-central/source/buildbot-configs/mozilla/config.py#2617-2622
Flags: needinfo?(rail)
Comment 4•8 years ago
|
||
Also, I'm sure that Thunderbird still uses them. We should let them know if we decide to drop support.
Assignee | ||
Comment 5•8 years ago
|
||
I chatted with rail in IRC - the pretty name tests steps are in factories that we no longer use. I couldn't find any instance of them in the logs where they appeared to be enabled, so I think we can remove both the moz-automation steps as well as the MOZ_PKG_PRETTYNAMES=1 logic.
Assignee | ||
Comment 6•8 years ago
|
||
CC'd some tb folks since it looks like tb still uses MOZ_PKG_PRETTYNAMES=1. If we remove it from m-c, how will that impact you? Since it's just removing some dead (to firefox) code, we can wait to land it.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 9•8 years ago
|
||
Thunderbird uses MOZ_PKG_PRETTYNAMES=1 because Firefox used to, so we should do anything short of migrating away from buildbot to follow (although that is surely also an option, for another day). Can you let us know what you changed in Firefox to continue building with MOZ_PKG_PRETTYNAMES removed?
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] from comment #9)
> Thunderbird uses MOZ_PKG_PRETTYNAMES=1 because Firefox used to, so we should
> do anything short of migrating away from buildbot to follow (although that
> is surely also an option, for another day). Can you let us know what you
> changed in Firefox to continue building with MOZ_PKG_PRETTYNAMES removed?
I believe it is less a change in Firefox and more a change in our release pipeline. Previously we used to do:
1) Build the tree with non-pretty package names
2) Test the package from 1)
3) Build the tree again with MOZ_PKG_PRETTYNAMES=1
4) Release the stuff in 3)
But with Release Promotion we now do something like:
1) Build the tree with non-pretty package names
2) Test the package from 1)
3) Promote the packages built in 1 to "released" versions (which maybe involves renaming them to match the pretty style?)
In the new pipeline, the MOZ_PKG_PRETTYNAMES logic in the build system is unused, so we are looking to remove it.
rail, did I sum things up approximately correctly? Anything I left out?
Flags: needinfo?(rail)
Comment 11•8 years ago
|
||
mozreview-review |
Comment on attachment 8826744 [details]
Bug 1329355 - Remove MOZ_PKG_PRETTYNAMES;
https://reviewboard.mozilla.org/r/104620/#review106056
fwiw, I am pretty sure that Thunderbird and SeaMonkey both use PRETTY NAMES for release packaging. And removing this stuff from toolkit could break the automation for those.
"we" still have an obligation to keep Thunderbird releases working...
I did not code-dive to verify this memory though.
Comment 12•8 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #10)
> rail, did I sum things up approximately correctly? Anything I left out?
Sounds sane to me.
Just as an additional point, current release automation uses in-tree templates to rename regular (non pretty) file names to their "pretty" equivalents, see https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/beetmover/en_us.yml.tmpl#42
Flags: needinfo?(rail)
Comment 13•8 years ago
|
||
mozreview-review |
Comment on attachment 8826743 [details]
Bug 1329355 - Remove MOZ_AUTOMATION_PRETTY*;
https://reviewboard.mozilla.org/r/104618/#review110696
Whee!
Attachment #8826743 -
Flags: review?(ted) → review+
Comment 14•8 years ago
|
||
mozreview-review |
Comment on attachment 8826744 [details]
Bug 1329355 - Remove MOZ_PKG_PRETTYNAMES;
https://reviewboard.mozilla.org/r/104620/#review110700
I love patches that are nothing but code removal. :)
::: toolkit/mozapps/installer/package-name.mk
(Diff revision 1)
>
> # assemble package names, see convention at
> # http://developer.mozilla.org/index.php?title=En/Package_Filename_Convention
> -# for (at least Firefox) releases we use a different format with directories,
> -# e.g. win32/de/Firefox Setup 3.0.1.exe
> -# the latter format is triggered with MOZ_PKG_PRETTYNAMES=1
You might want to mention that release packages get named differently, but that happens in the release pipeline, not the build system nowadays.
Attachment #8826744 -
Flags: review?(ted) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•8 years ago
|
||
Fallen, will this cause you trouble if we land now? Or, when would a good time be to do so?
Flags: needinfo?(philipp)
Comment 18•8 years ago
|
||
Given I don't know what part of or if our builds will actually break, I think it is best for you to land this now and we'll handle the fallout during our next build. The only reason I'd hold off is if we have a build coming up in which case I'd ask you to land this shortly after.
I'm ni'ing Wayne who has a better overview on our schedule.
Flags: needinfo?(philipp) → needinfo?(vseerror)
Comment 19•8 years ago
|
||
I'd say run with it assuming we can fix the fallout = manpower :)
I think the next earliest beta will be Wed/Thurs
Flags: needinfo?(vseerror)
Comment 20•8 years ago
|
||
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #19)
> I'd say run with it assuming we can fix the fallout = manpower :)
> I think the next earliest beta will be Wed/Thurs
change of plans - today I expect to build 45.7.1.
Is there an urgency to landing this patch for Firefox purposes? Or can it wait to a time of our choosing?
Assignee | ||
Comment 21•8 years ago
|
||
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #20)
> change of plans - today I expect to build 45.7.1.
>
> Is there an urgency to landing this patch for Firefox purposes? Or can it
> wait to a time of our choosing?
There's no urgency for this bug. Please just ping me or ni? me on this bug when it would cause the least disruption for you. Thanks!
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mshal
Assignee | ||
Comment 22•8 years ago
|
||
Just a reminder to let me know when you are ready.
Flags: needinfo?(vseerror)
Comment 23•8 years ago
|
||
Our beta built and is shipping today. If you don't hear from me by Monday noon EST that we need to fix something, please assume you can proceed.
Flags: needinfo?(vseerror)
Comment 24•8 years ago
|
||
Pushed by mshal@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/68d491c00355
Remove MOZ_AUTOMATION_PRETTY*; r=ted
https://hg.mozilla.org/integration/autoland/rev/318f1bcd336e
Remove MOZ_PKG_PRETTYNAMES; r=ted
Comment 25•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/68d491c00355
https://hg.mozilla.org/mozilla-central/rev/318f1bcd336e
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•