838321 - Use sources.xml to checkout b2g sources

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Chris AtLee [:catlee]]

Instead of using a static snapshot of b2g, we should check out sources directly from git.m.o.

Comment 1

[Aki Sasaki (not active)]

>+    "repo_remote_mappings": {
>+        'https://android.googlesource.com/': 'https://git.mozilla.org/external/aosp',
>+        'git://codeaurora.org/': 'https://git.mozilla.org/external/caf',
>+        'https://git.mozilla.org/b2g': 'https://git.mozilla.org/b2g',
>+        'git://github.com/mozilla-b2g/': 'https://git.mozilla.org/b2g',
>+        'git://github.com/mozilla/': 'https://git.mozilla.org/b2g',
>+        'https://git.mozilla.org/releases': 'https://git.mozilla.org/releases',
>+        'http://android.git.linaro.org/git-ro/': 'https://git.mozilla.org/external/linaro',
>+    },

This does seem fragile, and we both don't know how to fix it elegantly.
Does the manifest-monitoring cron job reduce this risk?
I'm not going to block on this, just noting it as something we'll have to keep an eye on.

>+    def load_json_from_url(self, url, timeout=30):
>+        self.debug("Attempting to download %s; timeout=%i" % (url, timeout))
>+        r = urllib2.urlopen(url, timeout=timeout)
>+        j = json.load(r)
>+        return j

I guess the self.retry() logs the attempt so debug() is ok.
Otherwise I would have a log_level=INFO or log_level=DEBUG and self.log(message, level=log_level)

>+++ b/mozharness/mozilla/repo_manifest.py

We don't have any logging in this file.
It it likely that we'll need to debug unexpected behavior in these methods later?

If we need to add logging, we could, f/e,

def load_manifest(filename, log_obj=None):
    ...
    if log_obj is not None:
        log_obj.info(message)
    ...

and pass self in as log_obj.

>+                                # Deprecated
>                                 'checkout-gecko',
>-                                # Download via tooltool repo in gecko checkout or via explicit url
>                                 'download-gonk',
>                                 'unpack-gonk',
>                                 'checkout-gaia',
>                                 'checkout-gaia-l10n',
>                                 'checkout-gecko-l10n',
>                                 'checkout-compare-locales',
>+                                # End deprecated

Do we know when the old method of checkouts are removable?
Might be nice to add a comment about that if so.

>                             default_actions=[
>-                                'checkout-gecko',
>-                                'download-gonk',
>-                                'unpack-gonk',
>+                                'checkout-sources',
>+                                'get-blobs',
>                                 'build',
>                             ],

Are we bitten by the fact that we now have a much larger set of default actions (since checkout-sources calls more deprecated actions than are listed here)?

>+    def query_remote_gecko_config(self):
>+        repo = self.query_repo()
>+        # TODO: Hardcoding this sucks
>+        if 'hg.mozilla.org' in repo:
>+            rev = self.query_revision()
>+            if rev is None:
>+                rev = 'default'
>+
>+            config_path = self.query_gecko_config_path()
>+            # Handle local files vs. in-repo files
>+            url = self.query_hgweb_url(repo, rev, config_path)
>+            return self.retry(self.load_json_from_url, args=(url,))

Hm, what happens if this fails?

>+            repos = [
>+                {'vcs': 'gittool', 'repo': 'https://git.mozilla.org/b2g/B2G.git', 'dest': dirs['work_dir']},
>+                {'vcs': 'gittool', 'repo': 'https://git.mozilla.org/b2g/b2g-manifest.git', 'dest': os.path.join(dirs['work_dir'], 'b2g-manifest'), 'branch': b2g_manifest_branch},
>+            ]
>+            self.vcs_checkout_repos(repos)

Known hardcodes.
If we ever want to fork to test something, we can get these from config, but I'm going to let this slide, I think.

I don't think I could catch everything in this review, but we can land on default and run some builds on Cedar.  I'm ok landing as-is if you are; if you want to address some of the above questions first that's cool too.  I imagine the work week will mandate timing.

Comment 2

[Chris AtLee [:catlee]]

Attachment: gittool changes

gittool needs some changes to work well with this. the most important change is moving to a local share per remote repo (like hg) instead of a global share.

this patch also handles some errors I hit in staging.

Comment 3

[Rail Aliiev [:rail]]

Comment on attachment 737632 [details] [diff] [review]
gittool changes

Review of attachment 737632 [details] [diff] [review]:
-----------------------------------------------------------------

::: lib/python/util/git.py
@@ +25,5 @@
>          repo = os.path.abspath(repo)
>      return repo
>  
>  
> +def get_repo_name(repo):

Can you add tests for this method?

@@ +51,5 @@
>      except subprocess.CalledProcessError:
>          return False
>  
>  
> +def is_commitid(refname):

and for this one

@@ +53,5 @@
>  
>  
> +def is_commitid(refname):
> +    """Returns True if refname is a valid commit id (contains only a-f0-9), and len == 20"""
> +    if not len(refname) == 40:

== 40 in the comments?

BTW, do you think that supporting short revision names will be useful?

@@ +124,5 @@
> +def set_share(repo, share):
> +    alternates = os.path.join(get_git_dir(repo), 'objects', 'info', 'alternates')
> +    share_objects = os.path.join(get_git_dir(share), 'objects')
> +
> +    open(alternates, 'w').write("%s\n" % share_objects)

can you explicitly close the file? or use "with".

@@ +176,5 @@
> +    # Clean up locks
> +    lock_file = os.path.join(get_git_dir(dest), "index.lock")
> +    if os.path.exists(lock_file):
> +        log.info("removing %s", lock_file)
> +        safe_unlink(lock_file)

does this mean that you may have a race condition with another process?

@@ +215,5 @@
> +                # Something went wrong!
> +                # Clobber share_dir and re-raise
> +                log.info("error fetching into %s - clobbering", share_dir)
> +                remove_path(share_dir)
> +                raise

does this mean that you remove the shared directory on any error, even on temporary ones, like 500, network issues, etc? Probably it's OK because we retry using different mirrors...

@@ +227,5 @@
> +                log.info("clobbering %s", share_dir)
> +                remove_path(share_dir)
> +                log.info("error fetching into %s - clobbering", dest)
> +                remove_path(dest)
> +                raise

the same here

Comment 4

[Chris AtLee [:catlee]]

Attachment: gittool changes

removes unused is_commitid function
adds tests for get_repo_name
uses "with" to write to alternates file
remove file lock cleanup

Comment 5

[Chris AtLee [:catlee]]

Just pushed http://hg.mozilla.org/build/mozharness/rev/466b3b086fab to default. Let's see how it fares on cedar.

Comment 6

[Aki Sasaki (not active)]

Merged to mozharness production.

Comment 7

[Chris AtLee [:catlee]]

Attachment: switch panda/unagi to use manifests

Comment 8

[Rail Aliiev [:rail]]

Comment on attachment 740776 [details] [diff] [review]
switch panda/unagi to use manifests

Whooohoo!

Comment 9

[Chris AtLee [:catlee]]

Comment on attachment 740776 [details] [diff] [review]
switch panda/unagi to use manifests

https://hg.mozilla.org/integration/mozilla-inbound/rev/08a750afe46f

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/08a750afe46f

Comment 11

[Chris AtLee [:catlee]]

Pushed http://hg.mozilla.org/build/mozharness/rev/64b21ad5e9ca to address nightly build bustage

Comment 12

[Aki Sasaki (not active)]

In mozharness production.

Comment 13

[Chris AtLee [:catlee]]

Attachment: switch mozilla-b2g18 to use manifests

Comment 14

[Rail Aliiev [:rail]]

Comment on attachment 742386 [details] [diff] [review]
switch mozilla-b2g18 to use manifests

Woot!

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/81ed682e7724

Comment 16

[Chris AtLee [:catlee]]

Attachment: switch mozilla-b2g18_v1.0.1 to use manifests