Skip to content

ENH Add environment variable to control 'exports' cli argument#3973

Merged
hoodmane merged 17 commits intopyodide:mainfrom
hoodmane:exports-environment-variable
Jul 7, 2023
Merged

ENH Add environment variable to control 'exports' cli argument#3973
hoodmane merged 17 commits intopyodide:mainfrom
hoodmane:exports-environment-variable

Conversation

@hoodmane
Copy link
Copy Markdown
Member

@hoodmane hoodmane commented Jul 1, 2023

If "--exports" is not passed as a CLI argument, check for an environment variable called PYODIDE_EXPORTS. If this is also missing, default to "requested".

cc @henryiii

Checklists

hoodmane and others added 2 commits July 1, 2023 09:03
If "exports" is not passed as a CLI argument, check for an environment variable
called PYODIDE_EXPORTS. If this is also missing, default to "requested".
@hoodmane hoodmane force-pushed the exports-environment-variable branch from ef15ee1 to b078e31 Compare July 1, 2023 16:51
@henryiii
Copy link
Copy Markdown
Contributor

henryiii commented Jul 1, 2023

Not at computer, but Typer (if that’s what you are using) has integrated support for envvars: https://typer.tiangolo.com/tutorial/arguments/envvar/ (most CLI parsers do).

@hoodmane
Copy link
Copy Markdown
Member Author

hoodmane commented Jul 1, 2023

Ah great thanks for pointing that out @henryiii.

@rth
Copy link
Copy Markdown
Member

rth commented Jul 2, 2023

Thanks! Could you also document it under https://pyodide.org/en/stable/development/building-from-sources.html#environment-variables ? Maybe it would make sense to have something more specific? Like PYODIDE_BUILD_EXPORTS to indicate that it's related to the pyodide build command. There are a lot of things that could be exports in different contexts. Otherwise LGTM.

@hoodmane
Copy link
Copy Markdown
Member Author

hoodmane commented Jul 6, 2023

@rth @ryanking13 could one of you give this a quick review? I want to backport it to 0.23.4 and it's the last thing I want to add to that release.

Copy link
Copy Markdown
Member

@ryanking13 ryanking13 left a comment

Choose a reason for hiding this comment

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

Thanks, @hoodmane! and sorry for the late review. I have some minor comments otherwise LGTM. Please also update the changelog.

@rth
Copy link
Copy Markdown
Member

rth commented Jul 6, 2023

Nice! LGTM as well and CI is green.

@hoodmane hoodmane merged commit fcb467d into pyodide:main Jul 7, 2023
@hoodmane hoodmane deleted the exports-environment-variable branch July 7, 2023 02:52
hoodmane added a commit to hoodmane/pyodide that referenced this pull request Jul 7, 2023
hoodmane added a commit to hoodmane/pyodide that referenced this pull request Jul 7, 2023
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.

4 participants