Skip to content
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

test link against libaio using distutils #1247

Merged
merged 1 commit into from
Jul 24, 2021

Conversation

adammoody
Copy link
Contributor

Work in progress...

This implements more robust testing for libaio based on the discussion threads in #1126 and #1213

This first commit implements the fallback solution which lets the user declare where libaio is installed. This successfully builds against the libaio installed in my conda environment after doing something like:

# install libaio into conda environment
conda install libaio

# tip the compiler on where to find libaio.h and libaio.so
export CFLAGS="-I${CONDA_PREFIX}/include $CFLAGS"
export LDFLAGS="-L${CONDA_PREFIX}/lib $LDFLAGS"

export DS_BUILD_OPS=1
python setup.py bdist_wheel

I'll work on the package manager detection bit described in #1126 (comment) and submit that as part of this same PR.

@adammoody
Copy link
Contributor Author

Hmm, the first commit by itself leads to the following error on while testing a build on the CI system:

AttributeError: module 'distutils' has no attribute 'ccompiler'

That might be happening because numpy or some other package perhaps has a distutils so that it's looking at the wrong distutils package for ccompiler as described in: AICoE/log-anomaly-detector#376

That page lists a work around that seems to help here, as well.

@tjruwase tjruwase merged commit 6ec8496 into microsoft:master Jul 24, 2021
@adammoody adammoody deleted the findlibaio branch July 24, 2021 21:20
if not aio_compatible:
self.warning(
f"{self.NAME} requires the libraries: {aio_libraries} but are missing. Can be fixed by: `apt install libaio-dev`."
f"{self.NAME} requires libaio but it is missing. Can be fixed by: `apt install libaio-dev`."
Copy link
Collaborator

@stas00 stas00 Jul 27, 2021

Choose a reason for hiding this comment

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

But this is incorrect. It requires some variation of libaio dev package and not libaio.

Probably should say:

f"{self.NAME} requires the dev libaio package but it is missing. Can be fixed by: `apt install libaio-dev` (or the equivalent name which may be called differently on your OS)."

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we can address it in #1250

@@ -160,6 +161,10 @@ def libraries_installed(self, libraries):
valid = valid or result.wait() == 0
return valid

def has_function(self, funcname, libraries):
compiler = distutils.ccompiler.new_compiler()
return compiler.has_function(funcname, libraries=libraries)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if the user doesn't have the compiler installed?

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.

3 participants