open-mpi / ompi

Open MPI main development repository
https://www.open-mpi.org
Other
2.18k stars 861 forks source link

PRRTE build failure with GCC 14.2.1 on Fedora 40 #12881

Open dalcinl opened 1 month ago

dalcinl commented 1 month ago

This is with ompi@main

../../../../../ompi-main/3rd-party/prrte/src/prted/pmix/pmix_server_queries.c: In function '_query':
../../../../../ompi-main/3rd-party/prrte/src/prted/pmix/pmix_server_queries.c:837:71: error: passing argument 5 of 'PMIx_Get' from incompatible pointer type [-Wincompatible-pointer-types]
  837 |                 ret = PMIx_Get(&pproc, PMIX_MEM_ALLOC_KIND, &info, 1, (void**)&value);
      |                                                                       ^~~~~~~~~~~~~~
      |                                                                       |
      |                                                                       void **
In file included from /home/dalcinl/Devel/REPOS/ompi-main/3rd-party/openpmix/src/util/pmix_error.h:26,
                 from /home/dalcinl/Devel/REPOS/ompi-main/3rd-party/prrte/src/pmix/pmix-internal.h:32,
                 from ../../../../../ompi-main/3rd-party/prrte/src/prted/pmix/pmix_server_queries.c:40:
/home/dalcinl/Devel/REPOS/ompi-main/3rd-party/openpmix/include/pmix.h:208:51: note: expected 'pmix_value_t **' {aka 'struct pmix_value **'} but argument is of type 'void **'
  208 |                                    pmix_value_t **val);
      |                                    ~~~~~~~~~~~~~~~^~~

cc @rhc54

rhc54 commented 1 month ago

Afraid that has nothing to do with me - it's a bug in the OMPI fork of PRRTE, not present in PRRTE itself. FWIW: we've seen some oddities in gcc 14.2, so it may not always be generating true issues. Not saying this one is in that category - just noting for future reference.

bosilca commented 1 month ago

@dalcinl https://github.com/openpmix/prrte/pull/1976

dalcinl commented 1 month ago

Sorry, but I need a small clarification to avoid confusion in future interactions.

If I do

$grep url .git/modules/*/config | grep -v git@github.com:open-mpi
.git/modules/openpmix/config:   url = git@github.com:openpmix/openpmix.git
.git/modules/prrte/config:  url = git@github.com:openpmix/prrte

it looks like both the openmpix and prrte submodules track the respective upstream repositories, and not forks within the open-mpi organization. Is this correct or am I missing something?

In any case, thanks for the prompt fix. I'll wait until the submodule pointer gets updated.

rhc54 commented 1 month ago

Hmmm...I'm pretty sure that isn't correct for OMPI main. I believe they pointed the PRRTE submodule at open-mpi/prrte, which is their fork. Unfortunately, the two aren't tracking very well. For example, that fix was committed upstream in early May!

Note that the OMPI v5 branch does still point at upstream PRRTE, which may be what your little grep is hitting.

dalcinl commented 1 month ago

OK, thanks for the clarification. My ompi@main clone is definitely still using the upstream prrte remote. I've been using git worktrees for checking out main/v5.0.x/v4.0x, I guess that worktrees do not play well with submodule URLs.

PS: @bosilca @jsquyres If that is the case, that is, mixing git worktrees with git submodules is somewhat broken, then this issue may hit other easily, I think savvy git users use worktrees a lot. Perhaps the OMPI and PRRTE devs can negotiate a branch namespace on upstream PRRT (eg. have branches ompi/main and ompi/v5.0.x in the upstream PRRTE git repository) such that you can put there your commits to track as git submodules in the OMPI repository. That way, when switching OMPI branches (or checking out branches in other worktress), the remote URLs for submodules do not change.

rhc54 commented 1 month ago

My ompi@main clone is definitely still using the upstream prrte remote

The bug you reported above only exists in the OMPI fork. Unless you haven't updated since May 7, you must be looking at the OMPI fork.

dalcinl commented 1 month ago

OK, I definitely messed up, I never run git submodule sync to update the submodule remote URL, so that was what confused me.

@bosilca Is there any plain to cherry-pick openpmix/prrte#1976 into the open-mpi/prrte fork and update the submodule pointer?

bosilca commented 1 month ago

https://github.com/open-mpi/prrte/pull/48