Skip to content
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

[patch] Make MPIx, PRRTE and MPL more portable for non-GNU #12815

Open
ksalerno99 opened this issue Sep 17, 2024 · 5 comments
Open

[patch] Make MPIx, PRRTE and MPL more portable for non-GNU #12815

ksalerno99 opened this issue Sep 17, 2024 · 5 comments

Comments

@ksalerno99
Copy link

ksalerno99 commented Sep 17, 2024

Thank you for taking the time to submit an issue!

Background information

Make MPIx and PRRTE more portable for AIX, HP-UX and MSVC - do not include getopt.h (see patches)

Make MPL more portable for AIX, HP-UX, Solaris and MSVC - do not include ifaddrs.h (see patches)

What version of Open MPI are you using? (e.g., v4.1.6, v5.0.1, git branch name and hash, etc.)

5.0.5 stable

Describe how Open MPI was installed (e.g., from a source/distribution tarball, from a git clone, from an operating system distribution package, etc.)

source tarball

If you are building/installing from a git clone, please copy-n-paste the output from git submodule status.

Please describe the system on which you are running

  • Operating system/version: AIX 7.3 TL1 SP4 rev2420
  • Computer hardware: POWER10
  • Network type: Ethernet

Details of the problem

Please describe, in detail, the problem that you are having, including the behavior you expect to see, the actual behavior that you are seeing, steps to reproduce the problem, etc. It is most helpful if you can attach a small program that a developer can use to reproduce your problem.

Note: If you include verbatim output (or a code block), please use a GitHub Markdown code block like below:

shell$ mpirun -n 2 ./hello_world

Patches:

diff -ru a/3rd-party/openpmix/src/util/pmix_cmd_line.h b/3rd-party/openpmix/src/util/pmix_cmd_line.h
--- a/3rd-party/openpmix/src/util/pmix_cmd_line.h	2024-09-17 11:10:30.317085792 -0400
+++ b/3rd-party/openpmix/src/util/pmix_cmd_line.h	2024-09-17 11:24:17.127202960 -0400
@@ -34,7 +34,6 @@
 #include <ctype.h>
 #include <stdio.h>
 #include <string.h>
-#include <getopt.h>
 
 #include "src/class/pmix_list.h"
 #include "src/class/pmix_object.h"
@@ -63,6 +62,13 @@
     .tail = NULL                                    \
 }
 
+/* Copyright (C) 1989-2021 Free Software Foundation, Inc.
+ * Copied x_argument defs from GNU getopt.h part of the GNU C Library not
+ * present on AIX, HP-UX or MSVC
+ */
+#define required_argument   1
+#define no_argument         0
+#define optional_argument   2
 /* define PMIX-named flags for argument required */
 #define PMIX_ARG_REQD       required_argument
 #define PMIX_ARG_NONE       no_argument
@@ -157,6 +163,19 @@
 #define PMIX_CLI_TARGETS                "targets"                   // required
 #define PMIX_CLI_TERMINATE              "terminate"                 // none
 #define PMIX_CLI_PSET_NAME              "pset"                      // required
+/* Copyright (C) 1989-2021 Free Software Foundation, Inc.
+ * Copied `struct option' from GNU getopt.h part of the GNU C Library not
+ * present on AIX, HP-UX or MSVC
+ */
+struct option
+{
+    const char *name;
+    /* has_arg can't be an enum because some compilers complain about
+       type mismatches in all the code that assumes it is an int.  */
+    int has_arg;
+    int *flag;
+    int val;
+};
 
 typedef void (*pmix_cmd_line_store_fn_t)(const char *name, const char *option,
                                          pmix_cli_result_t *results);
diff -ru a/3rd-party/prrte/src/mca/schizo/prte/schizo_prte.c b/3rd-party/prrte/src/mca/schizo/prte/schizo_prte.c
--- a/3rd-party/prrte/src/mca/schizo/prte/schizo_prte.c	2024-09-17 12:27:57.205301610 -0400
+++ b/3rd-party/prrte/src/mca/schizo/prte/schizo_prte.c	2024-09-17 12:28:27.726310508 -0400
@@ -34,7 +34,6 @@
 #    include <unistd.h>
 #endif
 #include <ctype.h>
-#include <getopt.h>
 
 
 #include "src/util/name_fns.h"
diff -ru a/3rd-party/prrte/src/util/prte_cmd_line.h b/3rd-party/prrte/src/util/prte_cmd_line.h
--- a/3rd-party/prrte/src/util/prte_cmd_line.h	2024-09-17 12:12:33.897298204 -0400
+++ b/3rd-party/prrte/src/util/prte_cmd_line.h	2024-09-17 12:13:44.152260075 -0400
@@ -34,7 +34,6 @@
 #include <ctype.h>
 #include <stdio.h>
 #include <string.h>
-#include <getopt.h>
 
 #include "src/class/pmix_list.h"
 #include "src/class/pmix_object.h"
diff -ru a/3rd-party/romio341/mpl/src/sock/mpl_sockaddr.c b/3rd-party/romio341/mpl/src/sock/mpl_sockaddr.c
--- a/3rd-party/romio341/mpl/src/sock/mpl_sockaddr.c	2024-09-17 13:05:44.694225473 -0400
+++ b/3rd-party/romio341/mpl/src/sock/mpl_sockaddr.c	2024-09-17 13:44:38.529626076 -0400
@@ -52,7 +52,6 @@
 #include <netdb.h>
 #include <netinet/in.h>
 #include <string.h>
-#include <ifaddrs.h>
 #include <errno.h>
 #include <stdio.h>
 
@@ -142,6 +141,41 @@
         assert(0);
     }
 }
+
+/* Copyright (C) 2002-2021 Free Software Foundation, Inc.
+ * Copied from GNU ifaddrs.h part of the GNU C library not available on AIX,
+ * HP-UX or MSVC
+ */
+
+struct ifaddrs
+{
+    struct ifaddrs *ifa_next;     /* Pointer to the next structure.  */
+
+    char *ifa_name;               /* Name of this network interface.  */
+    unsigned int ifa_flags;       /* Flags as from SIOCGIFFLAGS ioctl.  */
+
+    struct sockaddr *ifa_addr;    /* Network address of this interface.  */
+    struct sockaddr *ifa_netmask; /* Netmask of this interface.  */
+    union
+    {
+      /* At most one of the following two is valid.  If the IFF_BROADCAST
+         bit is set in `ifa_flags', then `ifa_broadaddr' is valid.  If the
+         IFF_POINTOPOINT bit is set, then `ifa_dstaddr' is valid.
+         It is never the case that both these bits are set at once.  */
+      struct sockaddr *ifu_broadaddr; /* Broadcast address of this interface. */
+      struct sockaddr *ifu_dstaddr; /* Point-to-point destination address.  */
+    } ifa_ifu;
+    /* These very same macros are defined by <net/if.h> for `struct ifaddr'.
+       So if they are defined already, the existing definitions will be fine. */
+# ifndef ifa_broadaddr
+#  define ifa_broadaddr ifa_ifu.ifu_broadaddr
+# endif
+# ifndef ifa_dstaddr
+#  define ifa_dstaddr   ifa_ifu.ifu_dstaddr
+# endif
+
+    void *ifa_data;               /* Address-specific data (may be unused).  */
+};
 
 int MPL_get_sockaddr_iface(const char *s_iface, MPL_sockaddr_t * p_addr)
 {

@rhc54
Copy link
Contributor

rhc54 commented Sep 17, 2024

Need to ponder this a bit more, but I don't believe we can accept those changes. If we copy code from GNU, that code is GPL'd and will therefore cause us to become GPL - which is something we are unwilling to do.

@ksalerno99
Copy link
Author

ksalerno99 commented Sep 17, 2024 via email

@rhc54
Copy link
Contributor

rhc54 commented Sep 17, 2024

Still, merits some careful consideration. We generally steer a very wide berth around GPL of any type. Might be a different approach required that doesn't in any way come near any GPL variation.

@rhc54
Copy link
Contributor

rhc54 commented Oct 13, 2024

After discussing it here, I believe the consensus (at least for the PMIx portion) is to leave things as they are - the GPL connection is simply unacceptable and it doesn't appear we really gain anything. For example, we have not even attempted to support the Microsoft environment for more than a decade, and while this might allow things to compile on AIX and HP-UX, there is no guarantee that anything would work in those environments - and we have no users (let alone developers) with access to them.

So while I appreciate the input, I think this is something we will just have to leave alone. I'll leave the OMPI portion to that community.

@ksalerno99
Copy link
Author

ksalerno99 commented Oct 13, 2024 via email

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

No branches or pull requests

2 participants