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

Upgrade extract-idl.py for Python3; regenerate IDLs. #3606

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

kenrussell
Copy link
Member

Update to html5lib-1.1, and incorporate webencodings-0.5.1 dependency, both of which which are required in order to run the extract-idl.py script on python3. html5lib is MIT licensed and webencodings is BSD licensed; both are compatible with this repository.

Regenerate WebGL 1.0 and 2.0 IDL from current specs.

Follow-on to #3222 .

Update to html5lib-1.1, and incorporate webencodings-0.5.1 dependency,
both of which which are required in order to run the extract-idl.py
script on python3. html5lib is MIT licenseed and webencodings is BSD
licensed; both are compatible with this repository.

Regenerate WebGL 1.0 and 2.0 IDL from current specs.
Copy link
Contributor

@kainino0x kainino0x left a comment

Choose a reason for hiding this comment

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

LGTM. Would be nice if something checked that the IDL files were up to date on CQ :)

Also a comment about how we could keep this working in the future.

@@ -7,4 +7,4 @@ webgl.idl: index.html extract-idl.py Makefile
echo "//" >> webgl.idl
echo "// WebGL IDL definitions scraped from the Khronos specification:" >> webgl.idl
echo "// https://www.khronos.org/registry/webgl/specs/latest/" >> webgl.idl
PYTHONPATH=../../../resources/html5lib/src python extract-idl.py index.html >> webgl.idl
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing that when this script was updated to python3, the author ran it directly (with system-installed dependencies). It's not really obvious that it's supposed to be run through the Makefile instead, especially since it's executable and has a shebang. What do you think about either chmod -x extract-idl.py so that it's not executable directly on *nix (and/or changing the shebang to #!/usr/bin/false # Run this via Makefile instead) or printing a warning from extract-idl.py if PYTHONPATH is not set?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's possible that someone rebuilt the IDL having had those dependencies installed locally. From talking with you and @greggman just now, it sounds like guaranteeing a hermetic environment is a little tricky. For now I'd like to merge this so that the IDL is updated, and will look into adding a check to the Github actions to ensure the IDL is up to date with every pull request. That should help.

@kenrussell kenrussell merged commit a8e0770 into KhronosGroup:main Dec 20, 2023
2 checks passed
@kenrussell kenrussell deleted the extract-idl branch December 20, 2023 22:15
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