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

Status: VERIFIED FIXED

(Release Engineering :: General, defect, P2)

Comment 1

[Rail Aliiev [:rail]]

Attachment: Proposed fix

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Ben, could you explain why this is a needed ?

Comment 3

[[e-mail] (:bhearsum)]

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.

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

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.

Comment 5

[[e-mail] (:bhearsum)]

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

Comment 6

[Rail Aliiev [:rail]]

Attachment: Proposed patch

Comment 7

[[e-mail] (:bhearsum)]

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.

Comment 8

[Rail Aliiev [:rail]]

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.

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 438347 [details] [diff] [review]
Proposed patch

We can land this after 3.6.4.

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 438347 [details] [diff] [review]
Proposed patch

changeset:   721:c61313a0054d

Comment 11

[[e-mail] (:bhearsum)]

pm01 and pm02 all reconfiged

Comment 13

[[e-mail] (:bhearsum)]

this is done, i think.

Comment 14

[Armen [:armenzg]]

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!