Skip to content

Updating VERSION to rc1 #5668

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 12, 2018
Merged

Updating VERSION to rc1 #5668

merged 1 commit into from
Sep 12, 2018

Conversation

gpaulsen
Copy link
Member

No description provided.

@jsquyres
Copy link
Member

@gpaulsen Is there a reason for:

libmpi_mpifh_so_version=61:0:21

instead of

libmpi_mpifh_so_version=60:0:20

@gpaulsen
Copy link
Member Author

Yes. That's carried forward from v3.1.x:
https://github.com/open-mpi/ompi/blob/v3.1.x/VERSION
I followed it back through git and it just looks like it was an initial human error/inconsistency, but I don't think it caused any real problem.

@jsquyres
Copy link
Member

Just because https://github.com/open-mpi/ompi/blob/v3.1.x/VERSION#L89 is 51:2:11 doesn't mean that you have to have v4.0.0 be 61. I think you should make it 60, like all the others.

The v3.x trains have used one of number of their buffer (going from 50 to 51). But that doesn't mean you have to carry that forward to v4.0.x.

I think it's easier to have all of them be 60 at v4.0.x, otherwise we'll have this "Why is it x1?" conversation every time there's a new major release.

@gpaulsen
Copy link
Member Author

Okay, that sounds reasonable. Updating....

@jsquyres
Copy link
Member

I notice that most of your c values are 20:

  1. Per v4.0.0: Check for ABI compatibility with v3.1.x #5447, you only tested ABI compatibility with 3.1.x. A c value of 20 means that v4.0.x will ABI work with v3.0.x. Did you test this?
  2. Can you explain these two values?
libmca_ompi_common_ompio_so_version=60:0:19
libmca_ompi_common_monitoring_so_version=60:0:10

@gpaulsen
Copy link
Member Author

gpaulsen commented Sep 11, 2018

  1. No I did NOT test v3.0.x, I only tested v3.1.x. My understanding is that for v4.0.0 we're reving both a and c by 10, which maintains the compatibility with v3.1.x branch. If it happens to also be compatible with v3.0.x, that's only due to the transitive property of v3.1.x and v3.0.x compatibility. But I might be way off on this. I'll hold RC1 until this PR goes in, and we're all satisfied with this.

  2. Those values are increments of both a and c by 10 from v3.1.x branch:

libmca_ompi_common_ompio_so_version=50:0:9
libmca_ompi_common_monitoring_so_version=50:0:0

@jsquyres
Copy link
Member

ABI with v3.0.x / v3.1.x

Note that v3.1.x was ABI compatible with v3.0.x.

So hypothetically testing v3.1.x is sufficient to test v3.0.x. ...but it might be worth testing v3.0.x, too. Trust, but verify.

Because adding 10 to the v3.1.x values means that you are declaring that 4.0.0 is ABI compatible with v3.0.x, too (which I think is the intent, right?).

Odd VERSION values

Looks like libmca_ompi_common_monitoring_so_version wasn't introduced until v3.1.0. This explains the low c value.

For libmca_ompi_common_ompio_so_version, it looks like we had:

  • 3.0.0: 40:0:0
  • 3.0.1: 41:0:0 ◀️ Looks like we intentionally broke ABI here
  • 3.0.2: 41:1:0
  • 3.0.x head: 41:1:0
  • 3.1.0: 50:0:9 ◀️ Looks like we chose to be ABI compatible with 3.0.1 and beyond
  • 3.1.1: 50:0:9
  • 3.1.2: 50:0:9
  • 3.1.x head: 50.0.9
  • 4.0.x head: 60:0:19

I think these are ok; we clearly made a decision back in 3.1.0 days. But remember that our backwards compatibility guarantee does not cover the common libraries (and users don't notice if we break ABI there because they don't link directly against the common libraries -- and therefore distros/packagers don't care, either).

@gpaulsen
Copy link
Member Author

Thanks for double checking all of this.
I can do the v3.0.x testing, but I can't do that tonight. Let's hold off on RC1 until after this testing against v3.0.x is done... hopefully tomorrow.

@bwbarrett
Copy link
Member

Up to you, but usually the first RC or so, we haven't been testing backwards compat. Make sure the easy works, then pick up the hard stuff...

@gpaulsen
Copy link
Member Author

@hppritcha ??? ^^^

@hppritcha
Copy link
Member

sorry didn't see this

@gpaulsen gpaulsen merged commit 3bf2220 into open-mpi:v4.0.x Sep 12, 2018
@gpaulsen
Copy link
Member Author

Thanks.

@gpaulsen gpaulsen deleted the v4.0.x branch September 12, 2018 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants