add_patcher_config step of MajorUpdateFactory should be set-up to never fail under normal circumstances

VERIFIED FIXED

Status

Release Engineering
General
P2
normal
VERIFIED FIXED
8 years ago
5 years ago

People

(Reporter: bhearsum, Assigned: rail)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [automation][updates])

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

8 years ago
Assignee: nobody → rail
(Assignee)

Comment 1

8 years ago
Created attachment 433316 [details] [diff] [review]
Proposed fix
Attachment #433316 - Flags: review?(bhearsum)
(Reporter)

Updated

8 years ago
Attachment #433316 - Flags: review?(bhearsum) → review+
Ben, could you explain why this is a needed ?
(Reporter)

Comment 3

8 years ago
In general, the patcher config is already going to exist in CVS. Only for the first MU will we have to land it. I guess the better thing to do here would be to only add it if it doesn't already exist.
Ah, I'd forgotten we use patcher-config-creator every time for major updates, rather than patcher-config-creator once then patcher-config-bump. Mostly I'm looking to avoid not committing a config and then not having history.

So yeah, the cvs add step could move before the bump and be conditional on the non-existence of the config file. Might need to use touch to create an empty file first.
(Assignee)

Updated

8 years ago
Attachment #433316 - Flags: checked-in?
(Assignee)

Updated

8 years ago
Blocks: 552994
(Reporter)

Comment 5

8 years ago
Comment on attachment 433316 [details] [diff] [review]
Proposed fix

Based on the recent comments I think we'll end up taking a better fix for this
Attachment #433316 - Attachment is obsolete: true
Attachment #433316 - Flags: checked-in?
(Reporter)

Updated

8 years ago
No longer blocks: 552994
Summary: add_patcher_config step of MajorUpdateFactory should set flunkOnFailure=False → add_patcher_config step of MajorUpdateFactory should be set-up to never fail under normal circumstances
(Assignee)

Updated

8 years ago
Priority: -- → P2
(Assignee)

Comment 6

8 years ago
Created attachment 438347 [details] [diff] [review]
Proposed patch
Attachment #438347 - Flags: review?(bhearsum)
(Reporter)

Comment 7

8 years ago
Comment on attachment 438347 [details] [diff] [review]
Proposed patch

This doesn't look like the right order to do things. This should happen after patcher-config-creator.pl is run, which eliminates the need to touch the file.
Attachment #438347 - Flags: review?(bhearsum) → review-
(Assignee)

Comment 8

8 years ago
As we talked on IRC, using bash is the only way to check if file exists on slave side.

I tested this patch in staging using the following scenario.

* Set up a cvs mirror with patcher-configs. Removed all major-update cfg files.
* Set up staging to use 1.9.1 (3.5.9) with MU configured
* First forced run of the major_update builder generates a MU cfg file from scratch and the diff between the production CVS versoin and generated one is sane: http://diff.pastebin.mozilla.org/714555
* Second forced run of the major_update builder went fine. add_patcher_config step does nothing, the output of diff_patcher_config is zero, cvs commit went as usual.
Status: NEW → ASSIGNED
(Assignee)

Updated

8 years ago
Attachment #438347 - Flags: review- → review?(bhearsum)
(Reporter)

Comment 9

8 years ago
Comment on attachment 438347 [details] [diff] [review]
Proposed patch

We can land this after 3.6.4.
Attachment #438347 - Flags: review?(bhearsum) → review+
(Reporter)

Comment 10

8 years ago
Comment on attachment 438347 [details] [diff] [review]
Proposed patch

changeset:   721:c61313a0054d
Attachment #438347 - Flags: checked-in+
(Reporter)

Comment 11

8 years ago
pm01 and pm02 all reconfiged

Updated

8 years ago
Duplicate of this bug: 563935

Updated

8 years ago
Whiteboard: [automation][updates]
(Reporter)

Comment 13

8 years ago
this is done, i think.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Comment 14

8 years ago
I did notice that it was fixed this week.

Instead of:
cvs add patcher-configs/moz191-branch-major-update-patcher2.cfg
it run this and it did what I wanted:
bash -c if [ ! -f patcher-configs/moz191-branch-major-update-patcher2.cfg ]; then touch patcher-configs/moz191-branch-major-update-patcher2.cfg && cvs add patcher-configs/moz191-branch-major-update-patcher2.cfg; fi

Thanks Rail!
Status: RESOLVED → VERIFIED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.