Closed Bug 702832 Opened 13 years ago Closed 12 years ago

solidify strategy to sync mozbase packages to m-c

Categories

(Testing :: Mozbase, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: k0scist, Unassigned)

References

Details

(Whiteboard: [mozbase])

Attachments

(3 files, 1 obsolete file)

Mozbase software lives at https://github.com/mozilla/mozbase  However, a big part of the mozbase effort is to use mozbase in m-c test harnesses.  peptest will be going to m-c. Not only does it have to be mirrored from its canonical repository, but it also uses mozbase packages that should be mirrored to m-c.  Traditionally, we have mirrored things by hand, but a script would be nice.  In any case, we should

- decide on a strategy for mirroring mozbase core packages and consumers to mozilla-central
- build whatever technology we need to to make this happen, if any (though i'm guessing there will be)
- document how this works in the obvious places
- mail stakeholders and otherwise communicate about it

It would also be nice to use a centralized strategy for all (or most) of the resources in m-c which are mirrored there, but that should not block the existing effort
This is a strawman implementation of a fetching script.  Currently it can fetch from files, tarballs, hg, and git, though these are easy to extend.  It is a bit messy (it *is* a strawman, after all), but I hope the general idea is clear.  If not, questions are welcome.  The full repository is available here: http://k0s.org/mozilla/hg/fetch
Attachment #574737 - Flags: feedback?(jmaher)
nice, first glance this looks good, I want to spend a bit more time looking through it in more detail.
Oh, especially for a strawman a README and example manifest would be nice to link to:

- http://k0s.org/mozilla/hg/fetch/file/tip/README.txt
- http://k0s.org/mozilla/hg/fetch/file/tip/example.txt
We met on this today. Summary:

- we agreed that mozbase and peptest will be mirrored to m-c.  We'll probably want to do this for many harnesses but can decide that on a case by case basis

- the general process:

 0. Bump version of the external resource (and release to PyPI if applicable).  This is important for a two reasons.  The first is sanity.  The second is so pegged versions accurately resolve to the software you want them to and there is no possibility of accidentally using the old mirrored version (or any other version, for that matter).

 1. resource is mirrored to mozilla-central and a patch created from the diff.  A script (e.g. http://k0s.org/mozilla/hg/fetch/file/tip/fetch.py) may be used, or the git repo in m-c methodology, or this can be done by hand.  While we should have maintenance/update steps and people should use them, it really doesn't matter how the patch is generated as long as it is correct.

 2. Patch is posted to bugzilla for review.  On r+, the patch is landed.  The version bumping step should actually probably be acted on now instead of when I said it was.

- what to do about changes that should be upstreamed? If someone changes the harness on m-c...
   - firstly, they mostly shouldn't.  They should file a bug with a patch of which one of us is aware.  We    can then fix m-c and upstream simultaneously
   - we can (and probably should) look at the logs for mirrored resources before clobbering with an updated mirror.  If there are changes (though see above point), we can upstream them
   - and if we do wipe them, that means that someone didn't follow process.  Probably not a huge deal as random fixes to harness code (by not us) are uncommon.

- after peptest and mozbase pieces are mirrored to m-c, bugs should be filed and acted upon for the duplication of manifestparser.py and mozinfo.py
Comment on attachment 574737 [details]
strawman implementation of a fetching script

this looks pretty good.  I didn't test this out, but reading through it (twice now), I don't see anything that stands out as a WTF or bad style.  

Thanks for this great script!
Attachment #574737 - Flags: feedback?(jmaher) → feedback+
Whether we want to use/build on `fetch` or not is another question.  It certainly could use some cleanup and testing (not hard, just time consuming), but it works for cases I've tested.
So bug 787263 and bug 722451 are at least conceptual duplicates of this bug.  So lets move the discussion over to here.

I have a python script at http://k0s.org/mozilla/mozbase/mc-diff.py that I've been steadily improving.  It does generate useful diffs and includes both sides of the diff : that is changes to m-c and changes from github.  My only real complaint is that it shells out to `cp`, frankly, because it was easier.  That can be improved.  It also doesn't do tagging, but IMHO that is out of scope.

I'll upload it and ping :wlach for feedback, though quite frankly if anyone else has any ideas, i'd love feedback.  I would much prefer a python script for this to a shell script (for several reasons which I'm happy to discuss but preferably off-bug).  

This will also need to change now that mozdevice lives actually *in* testing/mozbase (Go Will!)
Attached patch generates a diff against m-c (obsolete) — Splinter Review
Obviously all of the 'MOZILLA_TRY' nonsense can be completely ignored (and replaced with `hg root` once in-tree (though we probably need to check to ensure there are no outstanding changes).
Attachment #659400 - Flags: feedback?(wlachance)
Attached file updated
Thinking about it, this should go in m-c.  That way you can remove all of the crazy MOZILLA_TRY directory stuff that I do.  (Obviously the script will have to be updated before landing on m-c.)
Attachment #659400 - Attachment is obsolete: true
Attachment #659400 - Flags: feedback?(wlachance)
Attachment #660222 - Flags: feedback?(wlachance)
Comment on attachment 660222 [details]
updated

So this looks ok as a first start. I agree about keeping this in m-c.

My ideal would be that this would generate a patch suitable for use with "hg import". E.g., I'd call it like:

./mc-diff.py --reviewer jhammel --bug 123456 merge-mc.diff

And it would generate something like:


From: William Lachance <wlachance@mozilla.com>
Bug 780282 - Add option to disable pulse listener;r=mcote


diff --git a/testing/mozbase/...

Then it would be very easy to run the result on try, and eventually push the result to inbound.

>        f = file(output, 'w')
>        f.write(stdout)
>        f.close()

Can we use "with" here? (for python 2.5 compatibility, use "from __future__ import with_statement")
Attachment #660222 - Flags: feedback?(wlachance) → feedback+
(In reply to William Lachance (:wlach) from comment #13)
> Comment on attachment 660222 [details]
> updated
> 
> So this looks ok as a first start. I agree about keeping this in m-c.
> 
> My ideal would be that this would generate a patch suitable for use with "hg
> import". E.g., I'd call it like:
> 
> ./mc-diff.py --reviewer jhammel --bug 123456 merge-mc.diff
> 
> And it would generate something like:
> 
> 
> From: William Lachance <wlachance@mozilla.com>
> Bug 780282 - Add option to disable pulse listener;r=mcote
> 
> 
> diff --git a/testing/mozbase/...
> 
> Then it would be very easy to run the result on try, and eventually push the
> result to inbound.

So I *think* that this would require some bugzilla client goodness which isn't in m-c...yet.  But we can iterate on this a bit after I get something that actually patched against m-c.

> >        f = file(output, 'w')
> >        f.write(stdout)
> >        f.close()
> 
> Can we use "with" here? (for python 2.5 compatibility, use "from __future__
> import with_statement")

We can.  TBH, I avoid using from __future__ imports when possible, but this seems to be against the general grain here.
I've made this script somewhat fascist to ensure no data loss in an m-c clone.  It generates the diffs I would expect so far from testing
Attachment #661012 - Flags: review?(wlachance)
Comment on attachment 661012 [details] [diff] [review]
patch for addition to m-c

r+, see minor comments below

>+    # calculate hg root
>+    depth = 2  # testing/mozbase
>+    hg_root = here
>+    for i in range(depth):
>+        hg_root = os.path.dirname(hg_root)

Would it be possible to simplify this? The logic here is pretty hard to understand given that we're hardcoding things anyway.

I'd prefer something like:
hg_root = os.path.dirname(os.path.dirname(here))

...

>+    print "Diff at %s" % output

I was just thinking it would be nice to be able to just spit the diff directly to standard out (maybe if no output file is specified). Anyway, not required for a first pass.
Attachment #661012 - Flags: review?(wlachance) → review+
(In reply to William Lachance (:wlach) from comment #16)
> Comment on attachment 661012 [details] [diff] [review]
> patch for addition to m-c
> 
> r+, see minor comments below
> 
> >+    # calculate hg root
> >+    depth = 2  # testing/mozbase
> >+    hg_root = here
> >+    for i in range(depth):
> >+        hg_root = os.path.dirname(hg_root)
> 
> Would it be possible to simplify this? The logic here is pretty hard to
> understand given that we're hardcoding things anyway.
> 
> I'd prefer something like:
> hg_root = os.path.dirname(os.path.dirname(here))
> 
> ...

I did this because if the location of mozbase in m-c moved that depth would be the only thing to change.  That said, I think you're right and I'll just use your method.  The checking for the '.hg' directory should be a good enough warning if things fail.

> >+    print "Diff at %s" % output
> 
> I was just thinking it would be nice to be able to just spit the diff
> directly to standard out (maybe if no output file is specified). Anyway, not
> required for a first pass.

The subprocess calls (git, hg, etc) generate output so its not like you could redirect to a file meaningfully.  There's a large chunk of logic I often include in standalone scripts like this that

- runs commands silently
... UNLESS the command fails
- in which case you:
** print the command and exit code
** print its stdout
** print its stderr

I'd rather wait until we have some unified (and agreed on) way of doing this in m-c before proliferating this pattern further
(In reply to Jeff Hammel [:jhammel] from comment #17)
> (In reply to William Lachance (:wlach) from comment #16)

> > >+    print "Diff at %s" % output
> > 
> > I was just thinking it would be nice to be able to just spit the diff
> > directly to standard out (maybe if no output file is specified). Anyway, not
> > required for a first pass.
> 
> The subprocess calls (git, hg, etc) generate output so its not like you
> could redirect to a file meaningfully.  There's a large chunk of logic I
> often include in standalone scripts like this that
> 
> - runs commands silently
> ... UNLESS the command fails
> - in which case you:
> ** print the command and exit code
> ** print its stdout
> ** print its stderr
> 
> I'd rather wait until we have some unified (and agreed on) way of doing this
> in m-c before proliferating this pattern further

Oh yes, I was definitely only suggesting outputting the end result (the diff) to standard output. Not all the intermediary steps.
And of course I forgot the license header o_O
(In reply to Jeff Hammel [:jhammel] from comment #20)
> And of course I forgot the license header o_O

pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/1c04d97d0f74
https://hg.mozilla.org/mozilla-central/rev/3470d77caa66
https://hg.mozilla.org/mozilla-central/rev/1c04d97d0f74
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Mostly a note to self: the documentation at https://wiki.mozilla.org/Auto-tools/HowTo/MirrorRepo should be updated
(In reply to Jeff Hammel [:jhammel] from comment #23)
> Mostly a note to self: the documentation at
> https://wiki.mozilla.org/Auto-tools/HowTo/MirrorRepo should be updated

It would be great if you could do this sooner than later, as having up to date documentation on how to use this tool is basically a precondition for distributing the new work of merge sheriffing.
done: https://wiki.mozilla.org/Auto-tools/HowTo/MirrorRepo#Steps_to_mirror_mozbase

Please feel free to add more details
Target Milestone: mozilla18 → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: