Closed Bug 591388 Opened 9 years ago Closed 9 years ago

Teach client.py how to apply local patches

Categories

(MailNews Core :: Build Config, defect)

defect
Not set

Tracking

(thunderbird3.1 .7-fixed)

RESOLVED FIXED
Thunderbird 3.3a2
Tracking Status
thunderbird3.1 --- .7-fixed

People

(Reporter: gozer, Assigned: gozer)

References

Details

Attachments

(1 file)

This came out of a need for the tryserver builds to be able to apply patches to mozilla-central, for instance, before running the build.

The most straight forward solution was to teach client.py how to apply local patches to the various repository it clones.

It makes the tryserver much nicer, and I could see scenarios where release automation would also possibly make use of it.

In the currently attached patch, it's disabled by default and can be enabled with --apply-patches.

Basic idea is mapping each cloned repo to a file pattern, find files matching and applying them.

For instance with:
  mozilla-bug123.patch
  mozilla-bug124.patch
  inspector-bug125.patch

next to client.py,  they would be magically applied by client.py if invoked with --apply-patches
Attachment #469919 - Flags: review?(bugzilla)
Attachment #469919 - Flags: review?(kairo)
Comment on attachment 469919 [details] [diff] [review]
client.py --apply-patches

Hmm, why do all this mapping and hg import instead of just using the patch command?
Also, as it is a patch to shared c-c build system stuff, let's put this in MailNews Core.
Product: Thunderbird → MailNews Core
QA Contact: build-config → build-config
Comment on attachment 469919 [details] [diff] [review]
client.py --apply-patches

And, actually, my python knowledge is quite weak, I'd like to Callek do reviews here instead, he's done a lot of client.py work.
Attachment #469919 - Flags: review?(kairo) → review?(bugspam.Callek)
Comment on attachment 469919 [details] [diff] [review]
client.py --apply-patches

I'm not really keen on taking this... I think automation support of this form should take place on buildbot commands. (especially with all the varying ways to apply a patch in hg).

Mark/Gozer, if this is *absolutely* required for client.py to let our tryserver do what it used to be able to do, then I'll give it a review pass. (I'd much rather be able to specify a m-c repo-url/cset to use than this)
Attachment #469919 - Flags: review?(bugspam.Callek) → feedback-
(In reply to comment #4)
> I'm not really keen on taking this... I think automation support of this form
> should take place on buildbot commands. (especially with all the varying ways
> to apply a patch in hg).

The counter argument, is why should we be making the buildbot set-up very complex to support something that could easily be done in one step with one call. This hides buildbot from some of the complexities.

For example MozMill just has one call - make mozmill; the buildbot side would be far more complex and difficult to maintain.

> Mark/Gozer, if this is *absolutely* required for client.py to let our tryserver
> do what it used to be able to do, then I'll give it a review pass. (I'd much
> rather be able to specify a m-c repo-url/cset to use than this)

Note: iirc you can already do a specific m-c repo by using the repo url - one issue is most people don't have a try (or want to have) a clone of the m-c repo lying around.

If we had a try-cc-mozilla-central then I guess this is possible. It is probably about the same as to what has to be done:

- Push patches to try-cc-mc
- Get revision of try-cc-mc
- alter something in local c-c repository
- push to try-cc

versus

- Take applied patches/changes from m-c repo (assuming already applied, if you didn't have them then you would have to download and apply them versus just downloading).
- Add as patch files to c-c
- push to try-cc

The latter feels slightly cleaner for users to understand.
No doubt the patch is an improvement over existing functionality.  The mechanism it provides can always be further improved or replaced in the future if desired.
(In reply to comment #5)
> (In reply to comment #4)
> > I'm not really keen on taking this... I think automation support of this form
> > should take place on buildbot commands. (especially with all the varying ways
> > to apply a patch in hg).

> The latter feels slightly cleaner for users to understand.

Ok, I'm convinced that this is the role you feel strongly about taking, I'm not that strongly opposed, (I just hate cruft), but will get it done. [nit I have an idea to pass this slight client.py complexity off to buildbot still, but I'll see about doing it in another bug; and relies on another client.py idea of mine]
(In reply to comment #1)
> Hmm, why do all this mapping and hg import instead of just using the patch
> command?

Patch would have to support git mode, or you'd need to use git-apply. I'm not convinced we've got either available by default. hg import is equivalent to what developers use, so that seems reasonable IMO.

(In reply to comment #7)
> Ok, I'm convinced that this is the role you feel strongly about taking, I'm not
> that strongly opposed, (I just hate cruft), but will get it done. [nit I have
> an idea to pass this slight client.py complexity off to buildbot still, but
> I'll see about doing it in another bug; and relies on another client.py idea of
> mine]

I don't think this is cruft ;-) I still think buildbot should be dumb and not have much complexity.

However, I'm happy to consider other ideas, but they haven't been forthcoming yet, and I'm getting requests for this functionality. So I think we push forward with this for now and consider other options later.
Attachment #469919 - Flags: review?(bugzilla) → review+
Assignee: nobody → gozer
Checked in: http://hg.mozilla.org/comm-central/rev/da69529850e9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Target Milestone: --- → Thunderbird 3.3a2
Comment on attachment 469919 [details] [diff] [review]
client.py --apply-patches

So that try server can still work on 1.9.2, we'll need this there as well, a=Standard8 for that, and I've just landed the patch:

http://hg.mozilla.org/releases/comm-1.9.2/rev/a09641fee6e5
Attachment #469919 - Flags: approval-thunderbird3.1.7+
Blocks: 615664
You need to log in before you can comment on or make changes to this bug.