Re: [PATCH] maint.mk: improve the public-submodule-commit rule

[Eric Blake]

On 01/20/2011 04:34 AM, Jim Meyering wrote:
> So far, I don't see a down-side to adding this line to maint.mk:
> 
>   check: public-submodule-commit
> 
> Does anyone else?

Yes, but only minor:

>  public-submodule-commit:
> -     if test -d $(srcdir)/.git; then                                 \
> -       git submodule foreach 'test $$(git rev-parse origin)'         \
> +     $(AM_V_GEN)if test -d $(srcdir)/.git; then                      \
> +       cd $(srcdir) &&                                               \
> +       git submodule --quiet foreach 'test $$(git rev-parse origin)' \
>             = '"$$(git merge-base --independent origin $$sha1)"'      \

git 1.7.0.4 (the ubuntu 10.04 version) doesn't understand 'merge-base
--independent'.  I can rewrite it to test plain 'merge-base', as follows:

if $sha1 equals the remote origin, then the answer is obviously $sha1.
if $sha1 is an ancestor of the remote origin, then $sha1 is the
merge-base (good).  If origin is an ancestor of $sha1, then the answer
is origin, and $sha1 is a private commit that can still be pushed
upstream, but the fact that it is private was detected.  And if neither
origin nor $sha1 is an ancestor of the other, the merge base will be
something else altogether.  So, instead of comparing --independent
against origin, I get the same results by comparing $sha1 against plain
merge-base:

diff --git i/top/maint.mk w/top/maint.mk
index d2af3ad..5915814 100644
--- i/top/maint.mk
+++ w/top/maint.mk
@@ -1127,8 +1127,8 @@ submodule-checks ?= no-submodule-changes
public-submodule-commit
 public-submodule-commit:
        $(AM_V_GEN)if test -d $(srcdir)/.git; then                      \
          cd $(srcdir) &&                                               \
-         git submodule --quiet foreach 'test $$(git rev-parse origin)' \
-             = '"$$(git merge-base --independent origin $$sha1)"'      \
+         git submodule --quiet foreach test '$$(git rev-parse $$sha1)' \
+             = '$$(git merge-base origin $$sha1)'                      \
            || { echo '$(ME): found non-public submodule commit' >&2;   \
                 exit 1; };                                             \
        else                                                            \


git 1.5.5.6 (the rhel 5.6 version) doesn't even understand 'submodule
foreach'.  But at least it silently ignores it, so 'make check' will
succeed, although if you actively develop on a platform that old (and
heaven help you if that's your primary devel platform), you won't be
able to detect a private commit.  Oh well - the moment a person
developing on such an old platform pushes a private commit, everyone
else with newer platforms will call them on it :)

-- 
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature