Skip to content

Allow env vars to specify custom extensions to build #1910

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 4 commits into from
Jul 22, 2025

Conversation

crcrpar
Copy link
Collaborator

@crcrpar crcrpar commented Jun 10, 2025

This would be the first step to support build backends other than setuptools and make setup.py a little bit independent of the backend, i.e., better compatibility with PEP517 and relevant PEPs.

With this we would be able to support some commands that have not been functioning well e.g.

$ APEX_CPP_EXT=1 APEX_CUDA_EXT=1 APEX_ALL_CONTRIB_EXT=1 python -m build --wheel --no-isolation 

@crcrpar crcrpar requested a review from Copilot June 10, 2025 08:53
Copilot

This comment was marked as outdated.

This would be the first step to support build backends other than
setuptools and make setup.py a little bit independent of the backend

Signed-off-by: Masaki Kozuki <[email protected]>
@crcrpar crcrpar force-pushed the env_vars_to_specify_extensions branch from 4edc2f9 to 7d80b19 Compare June 10, 2025 17:34
@crcrpar crcrpar requested a review from Copilot June 10, 2025 18:20
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds environment-variable support for enabling Apex extensions under PEP 517 and allows configuring parallel build jobs via APEX_PARALLEL.

  • Introduce ENV_TO_FLAG, FLAG_TO_ENV, and has_flag to drive extension modules from env vars.
  • Replace hardcoded sys.argv checks with has_flag for each extension flag.
  • Add APEX_PARALLEL environment variable handling for the --parallel build option.
Comments suppressed due to low confidence (3)

setup.py:49

  • The new environment-variable to flag mapping and has_flag logic should have accompanying tests to verify that flags are correctly appended or skipped based on env settings.
for env_var, flag in ENV_TO_FLAG.items():

setup.py:85

  • Add a docstring for has_flag to explain that it checks both sys.argv, the specific environment variable, and APEX_ALL_CONTRIB_EXT for contrib flags.
def has_flag(flag, env_var):

setup.py:1006

  • The annotation int | None requires Python 3.10+ syntax and will fail to parse on earlier versions; use Optional[int] from typing for broader compatibility.
parallel: int | None = None

@crcrpar crcrpar marked this pull request as ready for review July 12, 2025 14:39
@nWEIdia
Copy link
Collaborator

nWEIdia commented Jul 16, 2025

LGTM! With this PR in, what is the default/recommended installation command?

@crcrpar
Copy link
Collaborator Author

crcrpar commented Jul 17, 2025

Env vars would be more modern than the apex's conventional install command.
also. I made a var of APEX_ALL_CONTRIB_EXt to build all the contrib extentions

@crcrpar crcrpar merged commit bfb500c into NVIDIA:master Jul 22, 2025
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.

2 participants