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

[fix] remove vestigial Python code and define dependency for oneDAL make builds #2929

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions INSTALL.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Required Software:
* [DPC++ Compiler](https://www.intel.com/content/www/us/en/developer/tools/oneapi/dpc-compiler.html)
* Microsoft Visual Studio\* (Windows\* only)
* [MSYS2](http://msys2.github.io) (Windows\* only)
* Python
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a line about it in the current master branch, two lines above where this would be inserted:
https://github.com/oneapi-src/oneDAL/blob/66f4858285c229a44be2ab17843edcbafaa989b9/INSTALL.md?plain=1#L23

It does mention "3.9" though so I guess there's no need to forego f-strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just for clarity, the mentioning of 3.9 was just introduced in your PR #2923 just after this PR was submitted (hence why I was confused). 3.9 requirement is set from here #2923 (comment) . I will rebase this PR to reflect your changes.

The only python script that is run for make install is this python file and is the source of the dependency. We do not test python consistently in any of our build steps. The private CI uses 3.12 as defined by infrastructure. The Ubuntu 24.04 docker runners use 3.12. The azure pipelines runners are also using default Ubuntu 22.04 python (3.10) runners. We do not test 3.9 nor 3.11 for oneDAL building.

Thus by changing these f-strings, we can support Ubuntu 20+ default python (i.e. python 2.7+).

@napetrov is there a reason why on build-side for oneDAL there is this python requirement?

* `make` and `dos2unix` tools; install these packages using MSYS2 on Windows\* as follows:

pacman -S msys/make msys/dos2unix
Expand Down
6 changes: 3 additions & 3 deletions deploy/pkg-config/generate_pkgconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def detect_cpu_architecture():
elif architecture.startswith('arm') or architecture == 'aarch64':
return 'aarch64'
else:
sys.stderr.write(f"Unknown Architecture {architecture} Detected. " \
"Only 'x86_64', 'AMD64' and 'aarch64' supported.\n")
sys.stderr.write("Unknown Architecture {} Detected. "
"Only 'x86_64', 'AMD64' and 'aarch64' supported.\n".format(architecture))
sys.exit(1)

LIBS_PAR_STAT, LIBS_PAR_DYN = [], []
Expand Down Expand Up @@ -72,7 +72,7 @@ def detect_cpu_architecture():
elif ARCH == 'aarch64':
LIBDIR = 'lib/arm'
else:
sys.stderr.write(f"Unknown CPU architecture '{ARCH}'\n")
sys.stderr.write("Unknown CPU architecture '{}'\n".format(ARCH))

SUFF_DYN_LIB = ".so"
SUFF_STAT_LIB = ".a"
Expand Down
Loading