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

[SYCL] Build issue with the latest commit to SYCL configure wrapper #17478

Closed
abagusetty opened this issue Mar 14, 2025 · 6 comments
Closed

[SYCL] Build issue with the latest commit to SYCL configure wrapper #17478

abagusetty opened this issue Mar 14, 2025 · 6 comments
Labels
bug Something isn't working

Comments

@abagusetty
Copy link
Contributor

abagusetty commented Mar 14, 2025

Describe the bug

Using the latest commit (e932cf9) and I was seeing this build error from SYCL Configure wrapper from this latest PR(ae76189). Upon removing several options passed to configure.py, I still keep seeing this python-error.

Since the CI tests nightly, I am not sure if I am missing something very trivial

CC=cc CXX=CC python3 $DPCPP_HOME/llvm/buildbot/configure.py 
Traceback (most recent call last):
  File "/home/abagusetty/today/llvm/buildbot/configure.py", line 429, in <module>
    ret = main()
  File "/home/abagusetty/today/llvm/buildbot/configure.py", line 421, in main
    args, passthrough_args = parser.parse_known_intermixed_args()
AttributeError: 'ArgumentParser' object has no attribute 'parse_known_intermixed_args'
@abagusetty abagusetty added the bug Something isn't working label Mar 14, 2025
@npmiller
Copy link
Contributor

parse_known_intermixed_args was added in Python 3.7, I suspect you're using a python version earlier than that?

Upgrading Python version may fix that, alternatively replacing parse_known_intermixed_args with parse_known_args may fix it, although it's not quite as nice.

@abagusetty
Copy link
Contributor Author

You are right, I was using 3.6.15. Hope this requirement bodes well with users since most OSs installations brings older versions of python. Since this looks like a hard-dependency, was this added to docs.

@npmiller
Copy link
Contributor

I don't think the bump in version was intentional, it seems we've never really documented required python versions. Although to be fair it seems Python 3.6 is no longer supported, it reached end of life in december 2021. Not sure if it's maintained separately by other OSs though.

Either way maybe it's worth switching to parse_known_args as it's slightly more portable, and not a lot worse in functionality. cc @ldrumm

@AlexeySachkov
Copy link
Contributor

I don't think the bump in version was intentional, it seems we've never really documented required python versions. Although to be fair it seems Python 3.6 is no longer supported, it reached end of life in december 2021. Not sure if it's maintained separately by other OSs though.

https://github.com/intel/llvm/blob/sycl/llvm/docs/GettingStarted.rst#software - note that LLVM which we are based on requires Python 3.8 already. I would say that we should copy that requirement into SYCL-specific getting started document and not try and stick to the older version, because there could be other Python scripts (which we inherit from LLVM) that would fail with older Python.

@npmiller
Copy link
Contributor

I agree, it makes a lot of sense to align with LLVM on this. I've opened a PR to update the GetStartedGuide to link to the LLVM required version for Python: #17565

ldrumm pushed a commit that referenced this issue Mar 25, 2025
This patch reformats the pre-requisites in a table (doesn't look great
in the markdown, but looks good when generated).

It also updates the required versions for CMake and Python to link to
the versions required by LLVM. For CMake we were already documenting the
same version as LLVM. For Python we didn't document any minimum version,
but we extensively use LLVM scripts like `lit`, so we really should be
aligned with it.

There's a few requirements for which we don't have minimum versions, but
we can update that if we ever run into issues with them.

Addresses #17478 by documenting the
minimum required python version.
@npmiller
Copy link
Contributor

Closing now as we've merged the documentation update: #17565

Thanks for catching this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants