Skip to content

bpo-38527: fix configure script for Solaris#16845

Merged
miss-islington merged 3 commits into
python:masterfrom
wiggin15:master
Apr 1, 2020
Merged

bpo-38527: fix configure script for Solaris#16845
miss-islington merged 3 commits into
python:masterfrom
wiggin15:master

Conversation

@wiggin15

@wiggin15 wiggin15 commented Oct 19, 2019 •
edited by miss-islington
Loading

Copy link
Copy Markdown
Contributor

On Solaris, the regular "grep" command may be an old version that fails to search a binary file. We need to use the correct command (ggrep, in our case), which is found by the configure script earlier.

https://bugs.python.org/issue38527

Automerge-Triggered-By: @pablogsal

@isidentical
isidentical requested a review from jcea February 29, 2020 16:33
@isidentical

isidentical commented Feb 29, 2020

Copy link
Copy Markdown
Member

Tested on SunOS gcc-solaris11 5.11 11.3 sun4u sparc SUNW,SPARC-Enterprise, works well.

@inorton

inorton commented Mar 23, 2020

Copy link
Copy Markdown
Contributor

Hi, I just hit this on Solaris 11.3. the fix looks ok to me. @wiggin15 I think you only need to add the news item

@isidentical
isidentical requested a review from pablogsal April 1, 2020 14:05
@isidentical

isidentical commented Apr 1, 2020

Copy link
Copy Markdown
Member

This is a really short change but that is needed in every time I had to compile CPython on solaris so it would be super cool to get this merged.

pablogsal
pablogsal reviewed Apr 1, 2020
View reviewed changes
Comment thread Misc/NEWS.d/next/Build/2020-03-28-10-43-09.bpo-38527.fqCRgD.rst Outdated
@miss-islington

miss-islington commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

@wiggin15: Status check is done, and it's a success ✅ .

@miss-islington
miss-islington merged commit 5dd8360 into python:master Apr 1, 2020
@miss-islington

miss-islington commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

Thanks @wiggin15 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 1, 2020
@wiggin15 @miss-islington
bpo-38527: fix configure script for Solaris (pythonGH-16845)
7cd50fe 
On Solaris, the regular "grep" command may be an old version that fails to search a binary file. We need to use the correct command (ggrep, in our case), which is found by the configure script earlier.

Automerge-Triggered-By: @pablogsal
(cherry picked from commit 5dd8360)

Co-authored-by: Arnon Yaari <wiggin15@yahoo.com>
@bedevere-bot

bedevere-bot commented Apr 1, 2020

Copy link
Copy Markdown

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

@miss-islington

miss-islington commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

Sorry, @wiggin15, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 5dd836030e0e399b21ab0865ae0d93934bdb3930 3.7

@miss-islington miss-islington self-assigned this Apr 1, 2020
@miss-islington

miss-islington commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

Thanks @wiggin15 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

@miss-islington

miss-islington commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

Sorry @wiggin15, I had trouble checking out the 3.7 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 5dd836030e0e399b21ab0865ae0d93934bdb3930 3.7

@pablogsal

pablogsal commented Apr 1, 2020

Copy link
Copy Markdown
Member

@wiggin15 @isidentical Could you do the backport to 3.7 using cherry_picker?

miss-islington added a commit that referenced this pull request Apr 1, 2020
@miss-islington @wiggin15
bpo-38527: fix configure script for Solaris (GH-16845)
fc03640 
On Solaris, the regular "grep" command may be an old version that fails to search a binary file. We need to use the correct command (ggrep, in our case), which is found by the configure script earlier.

Automerge-Triggered-By: @pablogsal
(cherry picked from commit 5dd8360)

Co-authored-by: Arnon Yaari <wiggin15@yahoo.com>
@wiggin15

wiggin15 commented Apr 1, 2020

Copy link
Copy Markdown
Contributor Author

@pablogsal Thanks for merging this! The file was added in 3.8. No need to backport.

@pablogsal

pablogsal commented Apr 1, 2020

Copy link
Copy Markdown
Member

Great, thanks for the PR!

@fanisk21

fanisk21 commented Dec 1, 2022

Copy link
Copy Markdown

Hi Guys, why this got lost at bpo-43617: Check autoconf-archive package in configure.ac (GH-25016) ?

@wiggin15

wiggin15 commented Dec 3, 2022

Copy link
Copy Markdown
Contributor Author

Hi @fanisk21 nice catch. Please comment on the linked issue (GH-25016) about this regression.
I'm afraid I don't have a Solaris machine available to test a fix. Any chance you can open a pull request to re-fix this?
Thanks.

@fanisk21 fanisk21 mentioned this pull request Dec 5, 2022
Merged
DanielTOsborne referenced this pull request Feb 29, 2024
@tiran
[3.9] bpo-43617: Check autoconf-archive package in configure.ac (GH-2…
064bc07 
…5016) (GH-25034)

Signed-off-by: Christian Heimes <christian@python.org>.
(cherry picked from commit 5d6e8c1)

Co-authored-by: Christian Heimes <christian@python.org>
@DanielTOsborne

DanielTOsborne commented Mar 1, 2024

Copy link
Copy Markdown

Oops, I spent time tracking down the same issue, and commented on the commit itself, before checking here for updated comments.
Assuming you'll still accept a PR for this, I can submit one.

I specifically needed to build 3.9 though, so I haven't actually checked the main branch yet if it's still an issue there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal pablogsal approved these changes
@isidentical isidentical isidentical approved these changes
@jcea jcea Awaiting requested review from jcea

Assignees

@miss-islington miss-islington

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@wiggin15 @isidentical @inorton @miss-islington @bedevere-bot @pablogsal @fanisk21 @DanielTOsborne @the-knights-who-say-ni