Skip to content

bpo-11122: always use rpmbuild in bdist_rpm#10594

Merged
csabella merged 1 commit into
python:masterfrom
n0npax:rpmbuild-hardcode-fix-11122
Jun 8, 2019
Merged

bpo-11122: always use rpmbuild in bdist_rpm#10594
csabella merged 1 commit into
python:masterfrom
n0npax:rpmbuild-hardcode-fix-11122

Conversation

@n0npax

@n0npax n0npax commented Nov 18, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

@n0npax n0npax force-pushed the rpmbuild-hardcode-fix-11122 branch from 16b0fe7 to 8c277a7 Compare November 18, 2018 21:48
@n0npax n0npax mentioned this pull request Nov 18, 2018
Closed
@n0npax n0npax force-pushed the rpmbuild-hardcode-fix-11122 branch from 379c379 to 4189641 Compare April 19, 2019 05:14
merwok
merwok reviewed May 29, 2019
View reviewed changes
Comment thread Lib/distutils/command/bdist_rpm.py
if os.path.exists('/usr/bin/rpmbuild') or \
os.path.exists('/bin/rpmbuild'):
rpm_cmd = ['rpmbuild']
rpm_cmd = ['rpmbuild']

@merwok merwok May 29, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should line 338 be changed too?

@merwok merwok changed the title bpo-11122: fix hardcoded rpmbuild path checking in bdist_rpm.py bpo-11122: always use rpmbuild in bdist_rpm May 29, 2019
@csabella csabella mentioned this pull request May 29, 2019
Closed
@csabella

csabella commented May 29, 2019

Copy link
Copy Markdown
Contributor

In #10172, the suggested change was to use if shutil.which('rpmbuild'), but that also added an additional import. I closed that pull request as a duplicate of this one.

@ned-deily ned-deily removed their request for review May 29, 2019 14:52
ericvsmith
ericvsmith approved these changes May 29, 2019
View reviewed changes

@ericvsmith ericvsmith left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see much point in running which, just to find the path. It doesn't buy you anything vs. just running the command.

I think the patch as-is is fine.

@csabella csabella merged commit 45a1494 into python:master Jun 8, 2019
@miss-islington

miss-islington commented Jun 8, 2019

Copy link
Copy Markdown
Contributor

Thanks @n0npax for the PR, and @csabella for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot

bedevere-bot commented Jun 8, 2019

Copy link
Copy Markdown

GH-13922 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Jun 8, 2019
@miss-islington @n0npax
bpo-11122: fix hardcoded path checking for rpmbuild in bdist_rpm.py (G…
3f7629d 
…H-10594)

(cherry picked from commit 45a1494)

Co-authored-by: Marcin Niemira <marcin.niemira@gmail.com>
@merwok

merwok commented Jun 10, 2019

Copy link
Copy Markdown
Member

There was no answer to my review question!

DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@n0npax @DinoV
bpo-11122: fix hardcoded path checking for rpmbuild in bdist_rpm.py (p…
56cf5e5 
…ythonGH-10594)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merwok merwok merwok left review comments

@ericvsmith ericvsmith ericvsmith approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@n0npax @csabella @miss-islington @bedevere-bot @merwok @ericvsmith @the-knights-who-say-ni