Skip to content

bpo-36002: fail to find -llvm-profdata #14998

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
Sep 13, 2019

Conversation

weibullguy
Copy link
Contributor

@weibullguy weibullguy commented Jul 29, 2019

The llvm-profdata is added using the AC_PATH_TARGET_TOOL in configure.ac. Per the autoconf documentation for the AC_PATH_TARGET_TOOL, "If the tool cannot be found with a prefix, and if the build and target types are equal, then it is also searched for without a prefix." However, only the AC_CANONICAL_HOST macro is called in the configure.ac script which means target will be set to '' while host and build are set to the values produced by config.guess. Thus, when configure attempts to find $target_alias-llvm-profdata, it will search for -llvm-profdata (note leading dash), which will not be found on probably any system.

However, the macro AC_PATH_TOOL will search for the un-prefixed tool (llvm-profdata in this case) if a prefixed version is not found on the path. This pull request replaces the AC_PATH_TARGET_TOOL macro with the AC_PATH_TOOL macro in configure.ac in two places (for llvm-profdata and llvm-ar).

With this change, Python built and tested satisfactorily using gcc v5.5.0 and clang v3.8.0, both with the --target switch set and without.

https://bugs.python.org/issue36002

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Our records indicate we have not received your CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@weibullguy weibullguy changed the title fix issue 36002: fail to find -llvm-profdata b.p.o. 36002: fail to find -llvm-profdata Jul 29, 2019
@weibullguy weibullguy changed the title b.p.o. 36002: fail to find -llvm-profdata bpo-36002: fail to find -llvm-profdata Jul 29, 2019
@benjaminp
Copy link
Contributor

Is there a reason we shouldn't fix this problem by calling AC_CANONICAL_TARGET?

@weibullguy
Copy link
Contributor Author

You obviously have me confused with a competent programmer. My understanding of the AC_PATH_TOOL macros and friends is that they use the AC_CANONICAL_XXX macros. But, they also include some additional magic when host/target prefixed applications aren't found to use non-prefixed applications that are found. It seems that behavior would need to be reproduced. That said, the reasons I chose this route were:

1). It was the simplest solution to the problem I could see and felt comfortable making (I'm a hack, not a hacker).
2). I do not know the design basis for using AC_PATH_TARGET_TOOL in the first place, the AC_PATH_TOOL macro is from the same "family" of macros so I stayed in that group. If I remember correctly, target defaults to host and generally you use --build and --host when cross-compiling, so I would think using the macro associated with host rather than target would be appropriate.
3). Use of AC_PATH_TOOL is consistent with the method used to find other build tools in the configure script (e.g., C++ compiler).

@benjaminp benjaminp merged commit 0519d49 into python:master Sep 13, 2019
@miss-islington
Copy link
Contributor

Thanks @weibullguy for the PR, and @benjaminp for merging it 🌮🎉.. 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 Sep 13, 2019
@bedevere-bot
Copy link

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

@bedevere-bot
Copy link

GH-16105 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 13, 2019
@benjaminp
Copy link
Contributor

I'm accepting this on the basis that llvm-ar and llmv-profdata should be host tools not target tools.

miss-islington added a commit that referenced this pull request Sep 13, 2019
miss-islington added a commit that referenced this pull request Sep 13, 2019
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.

5 participants