-
Notifications
You must be signed in to change notification settings - Fork 858
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
openmpi.spec: fix configure external 3rd-party package conflict #12542
Conversation
@@ -495,7 +495,7 @@ CXXFLAGS="%{?cxxflags:%{cxxflags}}%{!?cxxflags:$RPM_OPT_FLAGS}" | |||
FCFLAGS="%{?fcflags:%{fcflags}}%{!?fcflags:$RPM_OPT_FLAGS}" | |||
export CFLAGS CXXFLAGS FCFLAGS | |||
|
|||
%configure %{configure_options} %{_configure_3rd_party} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like the wrong fix. All you've done is make the configure line ./configure --with-<foo>=external --with-<foo>=<path>
and assume that configure will take the last option. That's likely to always be true, but a better option seems to be handling this way earlier in the process, by being selective about how the --with-<foo>=external
flags are added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the update we inspect user provided configure_options
and only set --with-<pkg>=external
if:
all_external_3rd_party
is true- user did not specify
--with-<pkg>=<path>
at all
I was looking for a better place to put this logic but landed here right before configure ...
, since that is the only consumer of those options.
91df683
to
b4042fb
Compare
When the user provides both `--with-<package>=/path/to/local/install` and all_external_3rd_party=1(which is the default value), the latter overrides `--with-<package>=external`. In this case the former explicitly provided options should be followed instead. Signed-off-by: Wenduo Wang <[email protected]>
b4042fb
to
fb78c22
Compare
If we detect |
@jsquyres That should also work. Right now
In other words it would be all-or-nothing. I'm not sure how much churn this will cause for the packagers in the field using the current spec file, but at least now we don't have the ambiguity anymore. |
I'm ok with just erroring if If |
Sorry I won't have time to work on this. Closing... |
When the user provides both
--with-<package>=/path/to/local/install
and all_external_3rd_party=1(which is the default value), the latter overrides--with-<package>=external
. In this case the former explicitly provided options should be followed instead.