search

KhronosGroup / EGL-Registry

EGL API and Extension Registry
115 stars 105 forks source link

add fix for lxml import based on suggestion from lxml.de/tutorial.html #198

Closed pcm1289 closed 2 months ago

pcm1289 commented 7 months ago

After installing the lxml dependency in MSYS on Windows using:

pacman -S python-lxml

..there was still an error:

$ make
python3 -B genheaders.py  -registry egl.xml EGL/egl.h
Traceback (most recent call last):
  File "EGL-Registry/api/genheaders.py", line 7, in <module>
    from reg import *
  File "EGL-Registry/api/reg.py", line 7, in <module>
    from lxml import etree
ImportError: No such process
make: *** [Makefile:17: EGL/egl.h] Error 1

image

It appears as if these may be well known because this website presented an alternative for those not depending explicitly on functions included in the lxml implementation of etree, found here: https://lxml.de/tutorial.html

Upon making this change.. image

..the ability to use make out of the box was restored: image

This pull request addresses this. A visual inspection of reg.py on surface level looks to not use any lxml.etree specific functions.

stonesthrow commented 7 months ago

@oddhack can you review this?

oddhack commented 4 months ago

@oddhack can you review this?

Sorry, I overlooked this. It is true that ElementTree is API-compatible for the functionality we use here, but it actually isn't semantically compatible in some corner cases including one we might use in the scripts. I'll take a look and if the outputs are identical, we can just use ElementTree.

oddhack commented 4 months ago

@oddhack can you review this?

Sorry, I overlooked this. It is true that ElementTree is API-compatible for the functionality we use here, but it actually isn't semantically compatible in some corner cases including one we might use in the scripts. I'll take a look and if the outputs are identical, we can just use ElementTree.

Verified we can replace lxml entirely for purposes of this repo. Suggest closing this and I'll merge #200 instead.

oddhack commented 2 months ago

@pcm1289 could you look at #200 and see if that will work for you as an alternate? If so, please comment over there - I cannot assign you as a reviewer because GitHub - and close this.

pcm1289 commented 2 months ago

Yep, that works. Thanks, closing