Skip to content

autoconf: fix --enable-mca-dso: correct two bugs that broke it #8799

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 2 commits into from
Apr 12, 2021

Conversation

acolinisi
Copy link
Contributor

This PR fixes two bugs introduced in 000f7e8 that broke --enable-mca-dso=foo.

config: fix --enable-mca-dso: correct the AS_VAR macro

Fix bug introduced in 000f7e8.

AS_VAR_COPY(dest, source) interprets the 'source' argument as a variable name. So, before this patch, the value was not '1', but '$1', which evaluated to garbage and broke the --enable-mca-dso argument.

— Macro: AS_VAR_COPY (dest, source)

Emit shell code to assign the contents of the polymorphic shell variable source to the polymorphic shell variable dest. For example, executing this M4sh snippet will output ‘bar hi’:

          foo=bar bar=hi
          AS_VAR_COPY([a], [foo])
          AS_VAR_COPY([b], [$foo])
          echo "$a $b"

config: fix --enable-mca-dso: correct copy-paste typo

Typo introduced in 000f7e8.

@bwbarrett

Fix bug introduced in 000f7e8.

AS_VAR_COPY(dest, source) interprets the 'source' argument as a variable name.
So, before this patch, the value was not '1', but '$1', which evaluated to
garbage and broke the --enable-mca-dso argument.

Signed-off-by: Alexei Colin <acolin@isi.edu>
Typo introduced in 000f7e8.

Signed-off-by: Alexei Colin <acolin@isi.edu>
@ompiteam-bot
Copy link

Can one of the admins verify this patch?

@@ -149,7 +149,7 @@ AC_DEFUN([OPAL_MCA],[
AC_MSG_ERROR([*** The enable-mca-direct flag requires a
*** list of type-component pairs. Invalid input detected.])
else
AS_VAR_COPY([AS_TR_SH([DIRECT_$type])], [AS_TR_SH([$comp])])
AS_VAR_SET([AS_TR_SH([DIRECT_$type])], [AS_TR_SH([$comp])])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe AS_VAR_SET Is correct here too, because, simplifying, it would be either AS_VAR_COPY([DIRECT_foo], [comp]) or AS_VAR_SET([DIRECT_foo], [$comp]) but not AS_VAR_COPY([DIRECT_foo], [$comp]). But I didn't test this path.

@rhc54
Copy link
Contributor

rhc54 commented Apr 11, 2021

ok to test

@rhc54
Copy link
Contributor

rhc54 commented Apr 11, 2021

@bwbarrett Can you review?

@bwbarrett bwbarrett merged commit d1c71ec into open-mpi:master Apr 12, 2021
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