Openembedded Bitbake Development
 help / color / mirror / Atom feed
From: Martin Jansa <martin.jansa@gmail.com>
To: Richard Purdie <richard.purdie@linuxfoundation.org>
Cc: bitbake-devel <bitbake-devel@lists.openembedded.org>
Subject: Re: [PATCH] git: Use merge-base instead of log for testing if a commit is present
Date: Mon, 18 Nov 2013 19:32:13 +0100	[thread overview]
Message-ID: <20131118183213.GI3727@jama> (raw)
In-Reply-To: <1384795036.6460.247.camel@ted>

[-- Attachment #1: Type: text/plain, Size: 3416 bytes --]

On Mon, Nov 18, 2013 at 05:17:16PM +0000, Richard Purdie wrote:
> The current use of git log to check if a given revision is present can be
> a little fragile.
> 
> For example if revision X was on branch A, and then later added to branch
> B, the update checks would not notice this since they just check for X
> being in the repository.
> 
> We also had some autobuilder corruption where an older packed-refs file
> was copied over a new repository containing newer pack files. There
> was no update to the refs file since the revision was present but
> not accessible in any branch.
> 
> The correct fix is to check that the required revisions are present
> on the specific branches. This patch does this using merge-base.

I guess that merge-base is probably faster or easier to use, but did you
consider using git branch --contains?

e.g. checking if selected branch is in
git branch --contains ud.revisions[name]

I'm asking only because I'm using "git branch --contains" in some
scripts and maybe there is good reason I should rewrite them to use git
merge-base instead.

Thanks

> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
> diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
> index 6175e4c..99230c1 100644
> --- a/bitbake/lib/bb/fetch2/git.py
> +++ b/bitbake/lib/bb/fetch2/git.py
> @@ -150,7 +150,7 @@ class Git(FetchMethod):
>              return True
>          os.chdir(ud.clonedir)
>          for name in ud.names:
> -            if not self._contains_ref(ud.revisions[name], d):
> +            if not self._contains_ref(ud.revisions[name], ud.branches[name], d):
>                  return True
>          if ud.write_tarballs and not os.path.exists(ud.fullmirror):
>              return True
> @@ -197,7 +197,7 @@ class Git(FetchMethod):
>          # Update the checkout if needed
>          needupdate = False
>          for name in ud.names:
> -            if not self._contains_ref(ud.revisions[name], d):
> +            if not self._contains_ref(ud.revisions[name], ud.branches[name], d):
>                  needupdate = True
>          if needupdate:
>              try: 
> @@ -281,13 +281,14 @@ class Git(FetchMethod):
>      def supports_srcrev(self):
>          return True
>  
> -    def _contains_ref(self, tag, d):
> +    def _contains_ref(self, tag, branch, d):
>          basecmd = data.getVar("FETCHCMD_git", d, True) or "git"
> -        cmd = "%s log --pretty=oneline -n 1 %s -- 2> /dev/null | wc -l" % (basecmd, tag)
> -        output = runfetchcmd(cmd, d, quiet=True)
> -        if len(output.split()) > 1:
> -            raise bb.fetch2.FetchError("The command '%s' gave output with more then 1 line unexpectedly, output: '%s'" % (cmd, output))
> -        return output.split()[0] != "0"
> +        cmd = "%s merge-base --is-ancestorlog %s %s" % (basecmd, tag, branch)
> +        try:
> +            output = runfetchcmd(cmd, d, quiet=True)
> +        except bb.fetch2.FetchError:
> +            return False
> +        return True
>  
>      def _revision_key(self, url, ud, d, name):
>          """
> 
> 
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/bitbake-devel

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

  reply	other threads:[~2013-11-18 18:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-18 17:17 [PATCH] git: Use merge-base instead of log for testing if a commit is present Richard Purdie
2013-11-18 18:32 ` Martin Jansa [this message]
2013-11-18 21:36   ` Richard Purdie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131118183213.GI3727@jama \
    --to=martin.jansa@gmail.com \
    --cc=bitbake-devel@lists.openembedded.org \
    --cc=richard.purdie@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox