Closed Bug 386806 Opened 17 years ago Closed 17 years ago

client.py script pulls from mozilla-central by default

Categories

(Firefox Build System :: General, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jorendorff, Assigned: jorendorff)

Details

Attachments

(1 file, 1 obsolete file)

If you run "python client.py checkout", one of the things the script does is an "hg pull" or even "hg clone" from the mozilla-central repo.  This is a nasty surprise if your workspace is based on some other repo, like actionmonkey.

Yeah, learned that one the hard way...  :)

For now I've hacked my client.py to default to -m actionmonkey.  I think the right thing is to make -m and -t require full URLs, not relative URLs, and make them default to whatever's in $DEST/.hg/hgrc and $DEST/js/tamarin/.hg/hgrc instead of defaulting to hardcoded values.

(That would also fix another bug-- the scripts default to ssh, but J. Random Hacker won't have ssh access.)
Yeah, that sounds reasonable. They default to ssh because when they were wriiten there was no HTTP access ;-)
Severity: normal → minor
OS: Mac OS X → All
Hardware: PC → All
OK, this patch fixes it.

This patch also changes something else -- it makes client.py print out the commands it executes.  I find this reassuring and it could be handy for troubleshooting.  Also helpful for context, in case Mercurial prompts you for something (like a passphrase).
Attachment #271096 - Flags: review?
Attachment #271096 - Flags: review? → review?(ted.mielczarek)
Comment on attachment 271096 [details] [diff] [review]
Change client.py as suggested in description, + echo commands

>+def fixup_repo_options(options):
>+    if options.mozilla_repo is None and not os.path.exists(os.path.join(topsrcdir, '.hg')):
>+        o.print_help()
>+        print
>+        print "*** The -m option is required for the initial checkout."
>+        sys.exit(2)
>+
>+    # Handle special case: tamarin directory doesn't exist yet
>+    if options.tamarin_repo is None and not os.path.exists(os.path.join(topsrcdir, 'js', 'tamarin')):
>+        moz_repo = options.mozilla_repo
>+        if moz_repo is None:
>+            cp = SafeConfigParser()
>+            cp.read([os.path.join(topsrcdir, '.hg', 'hgrc')])
>+            try:
>+                moz_repo = cp.get("paths", "default")
>+            except:
>+                moz_repo = 'http://hg.mozilla.org/mozilla-central'
>+        base = moz_repo.rsplit('/', 1)[0]
>+        options.tamarin_repo = base + '/tamarin-central'

Now maybe I'm just reading this wrong, but it looks like you never set moz_repo unless tamarin_repo isn't set and js/tamarain doesn't exist.  Don't you want to pull the part about moz_repo out of the block?  I gather that once you've done a pull you don't have to specify the repo, since Hg will have stored it, but if that's your intention, maybe you could make it clearer with a comment?
Attached patch Revised patchSplinter Review
OK, added lots of comments.  Also: made DEFAULT_TAMARIN_REPO a global constant; made the script refuse to recover if the expected URL isn't found in ${TOPSRCDIR}/.hg/hgrc.
Attachment #271096 - Attachment is obsolete: true
Attachment #271096 - Flags: review?(ted.mielczarek)
Status: NEW → ASSIGNED
Attachment #272036 - Flags: review?(ted.mielczarek)
Comment on attachment 272036 [details] [diff] [review]
Revised patch

Ok, looks good!
Attachment #272036 - Flags: review?(ted.mielczarek) → review+
Pushed changeset 899ca1f39f07 to hg.mozilla.org/mozilla-central .
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Ugh, closed the wrong bug :(
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed rev 178218b863ec to mozilla-central.
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Component: Build Config → General
Product: Firefox → Firefox Build System
You need to log in before you can comment on or make changes to this bug.