Skip to content

autogen.pl: fix submodule hash check #10807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 15, 2022

Conversation

jsquyres
Copy link
Member

Update the git submodule check to just look at the first character in each line of "git submodule status" output:

  • If it's "-", then the submodule is missing (this check was already there)
  • If it's "+", then the locally-checked out hash is different than what is expected by the submodule, so emit a warning (this check was there, but was incorrect)

Signed-off-by: Jeff Squyres [email protected]


This PR is a follow-on to #10803, which caused me to get a warning in autogen.pl output. Digging into the warning, I realized that the check it was performing was just wrong. This PR fixes the check.

Update the git submodule check to just look at the first character in
each line of "git submodule status" output:

* If it's "-", then the submodule is missing (this check was already
  there)
* If it's "+", then the locally-checked out hash is different than
  what is expected by the submodule, so emit a warning (this check was
  there, but was incorrect)

Signed-off-by: Jeff Squyres <[email protected]>
@rhc54
Copy link
Contributor

rhc54 commented Sep 15, 2022

Do we need to port this to PMIx and PRRTE now that they have the submodule check in autogen.pl?

@bwbarrett
Copy link
Member

Do we need to port this to PMIx and PRRTE now that they have the submodule check in autogen.pl?

I would; we straight copied that from OMPI. But I might wait until we've gotten it straightened out and committed here (having not yet looked at the PR).

@jsquyres jsquyres merged commit 9b87ffd into open-mpi:main Sep 15, 2022
@jsquyres jsquyres deleted the pr/autogen-fixup-for-oac branch September 15, 2022 22:41
@rhc54
Copy link
Contributor

rhc54 commented Sep 16, 2022

This script does not work on the Mac - I tried it both when ported to PMIx and in OMPI main. After removing the PMIx and PRRTE submodules, I get this in autogen.pl:

=== Submodule: 3rd-party/openpmix
    Local hash == remote hash (good!)
=== Submodule: 3rd-party/prrte
    Local hash == remote hash (good!)

When I run git submodule status, I see this output:

$ git submodule status
+8465992fc88cd3e0e9315da488b2bafb0941d8ce 3rd-party/openpmix (v1.1.3-3618-g8465992)
+e8640f1fcc6b45b97dacd94fedb3a9f5048141c2 3rd-party/prrte (psrvr-v2.0.0rc1-4437-ge8640f1)

The script indicates that this should generate an error, but it doesn't. When I print out the contents of the status and path variables:

STATUS Ralphs-iMac-2 PATH 

So I don't know what that perl section is supposed to be doing, but it isn't doing what you think on the Mac.

@jsquyres
Copy link
Member Author

=== Submodule: 3rd-party/openpmix
    Local hash == remote hash (good!)

This is output from the old autogen.pl -- that message changed when this PR merged. Are you sure you're running the newly-patched autogen.pl?

@rhc54
Copy link
Contributor

rhc54 commented Sep 16, 2022

Hmmm...weird. I had to do all kinds of convulsions to get it to properly update. Everything else would update - but not autogen.pl. Anyway, after convulsing, I did indeed manage to get your error output.

However, the same code in PMIx doesn't work:

if (-f ".gitmodules") {
    open(IN, "git submodule status|")
        || die "Can't run \"git submodule status\"";
    while (<IN>) {
        $_ =~ m/^(.).{40} ([^ ]+) /;
        my $status = $1;
        my $path   = $2;
print("STATUS $status PATH $path\n");
        # Make sure the submodule is there
        if ($status eq "-") {
            print("    ==> ERROR: Missing

The submodule \"$path\" is missing.

Perhaps you forgot to \"git clone --recursive ...\", or you need to
\"git submodule update --init --recursive\"...?\n\n");
            exit(1);
        }

        # See if the commit in the submodule is not the same as the
        # commit that the git submodule thinks it should be.
        elsif ($status eq "+") {
            print("    ==> WARNING: Submodule hash is different than upstream.
         If this is not intentional, you may want to run:
         \"git submodule update --init --recursive\"\n");
        } else {
            print("    Local hash is what is expected by the submodule (good!)\n");
        }
    }
exit(1);
}

results in:

$ ./autogen.pl
PMIx autogen (buckle up!)

1. Checking tool versions

   Searching for autoconf
     Found autoconf version 2.71; checking version...
       Found version component 2 -- need 2
       Found version component 71 -- need 69
     ==> ACCEPTED
   Searching for libtoolize
     Found libtoolize version 2.4.7; checking version...
       Found version component 2 -- need 2
       Found version component 4 -- need 4
       Found version component 7 -- need 2
     ==> ACCEPTED
   Searching for automake
     Found automake version 1.16.5; checking version...
       Found version component 1 -- need 1
       Found version component 16 -- need 13
     ==> ACCEPTED

2. Checking for git submodules

STATUS Ralphs-iMac-2 PATH 
    Local hash is what is expected by the submodule (good!)

even though

$ git submodule status
-5c8de3d97b763bf8981fb49cbedd36e201b8fc0a config/oac

I'm not entirely sure what the issue is in this case (if it is the cute perlsie stuff or an error in the code), but it is getting incorrect values. Any thoughts?

@jsquyres
Copy link
Member Author

I can replicate your error in PMIx. Lemme dig a little...

smalls

@jsquyres
Copy link
Member Author

jsquyres commented Sep 16, 2022

@rhc54 I'm pretty sure #10810 will fix the problem.

I notice that the PMIx autogen.pl is missing a print statement compared to the Open MPI autogen.pl. It looks like this:

2. Checking for git submodules

    ==> ERROR: Missing

The submodule "config/oac" is missing.
...etc.

Specifically, the following statement is missing in the check-for-submodules loop:

        print("=== Submodule: $path\n");

When you put it in there, you get output like this (from OMPI's autogen.pl):

2. Checking for git submodules

=== Submodule: 3rd-party/openpmix
    Local hash is what is expected by the submodule (good!)
=== Submodule: 3rd-party/prrte
    Local hash is what is expected by the submodule (good!)
=== Submodule: config/oac
    Local hash is what is expected by the submodule (good!)
...etc.

@bwbarrett
Copy link
Member

I bet that's my fault; I over-deleted when I removed the 3rdparty checks.

@rhc54
Copy link
Contributor

rhc54 commented Sep 16, 2022

I probably over-deleted too. Let me try to update with the new fix.

@rhc54
Copy link
Contributor

rhc54 commented Sep 16, 2022

Yeah, that gets it:

2. Checking for git submodules

=== Submodule: config/oac
    ==> ERROR: Missing

The submodule "config/oac" is missing.

Perhaps you forgot to "git clone --recursive ...", or you need to
 "git submodule update --init --recursive"...?

@rhc54
Copy link
Contributor

rhc54 commented Sep 16, 2022

Thanks Jeff! I like to think that my little problems help you to improve your code 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants