Skip to content

Conversation

@mwheinz
Copy link

@mwheinz mwheinz commented May 3, 2019

Refs #6628

In OMPI 2.1.2, buildrpm.sh could work with a value of rpmtopdir that was
set in the environment. In newer versions this is no longer true,
causing such values to be ignored. This patch adds a new argument to
buildrpm.sh, -R, which allows the user to specify where to build the
RPMs.

Michael Heinz added 2 commits May 3, 2019 09:52
In OMPI 2.1.2, buildrpm.sh could work with a value of rpmtopdir that was
set in the environment. In newer versions this is no longer true,
causing such values to be ignored. This patch adds a new argument to
buildrpm.sh, -R, which allows the user to specify where to build the
RPMs.

Signed-off-by: Michael Heinz <[email protected]>
(cherry picked from commit 687a560)

Conflicts:
	contrib/dist/linux/buildrpm.sh
Signed-off-by: Michael Heinz <[email protected]>
(cherry picked from commit 8562211)
@mwheinz mwheinz changed the title buildrpm.sh no longer respects the value of rpmtopdir v3.0.x buildrpm.sh no longer respects the value of rpmtopdir May 3, 2019
@mwheinz mwheinz requested review from jsquyres and rhc54 May 3, 2019 14:34
if test -r $file; then
rpmtopdir=${rpmtopdir:-"`grep %_topdir $file | awk '{ print $2 }'`"}
# if the user did not specify an $rpmtopdir, check for an .rpmmacros file.
if test "$rpmtopdir" == ""; then
Copy link
Member

Choose a reason for hiding this comment

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

Actually, I think the use of == is wrong here. Sorry we missed it on the master PR. 😦

I think it works with Linux test, but it fails (IIRC) with BSD flavors of test.

It might be a little better to if test -z "$rpmtopdir"; then ...

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I think this is moot -- this script is for building RPMs. Which is specific to Linux -- not BSD. So the use of == is probably ok here.

I therefore retract my prior comment. 😄

Copy link
Author

Choose a reason for hiding this comment

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

phew

@jsquyres jsquyres merged commit a06ad1a into open-mpi:v3.0.x May 3, 2019
@jsquyres jsquyres added this to the v3.0.5 milestone May 3, 2019
@mwheinz mwheinz deleted the REF6638-v3.0.x branch September 24, 2019 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants