dgibson / dtc

Device Tree Compiler
235 stars 134 forks source link

meson: allow building from shallow clones #122

Closed petermarko closed 8 months ago

petermarko commented 11 months ago

Shallow clones are useful for air-gapped build environments and to ensure various compliance tasks.

When building from shallow clone, tag is not available and version defaults to git hash. Problem is that some builds check DTC version and fail the comparison. Example is https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git Which fails to build with following error: dtc version too old (039a994), you need at least version 1.4.4

Drop --always from git describe command, see https://github.com/mesonbuild/meson/blob/1.3.0/mesonbuild/utils/universal.py#L773 This will make it more closer to build via Makefile.

dgibson commented 11 months ago

The change looks good to me, but I'm running into an unrelated failure in the pylibfdt tests :(. Looks like something has changed in Fedora that's subtly breaking something.

petermarko commented 11 months ago

Looks like python 3.12 changed swig return values (adding also function return code, not only return parameters). Now the question is if coming swig 4.2.0 will fix this (by adding compatibility with python 3.12) or we need to adapt bindings code to handle python version and drop the first value or just check the python version in testcase and expect or 2 or 3 return values. Unfortunately I have no knowledge of python bindings, so I don't know what should be the correct way to fix this.

dgibson commented 11 months ago

Yeah.. unfortunately I don't know a lot about the Python bindings either. @sjg20 care to weigh in?

sjg20 commented 11 months ago

Yeah.. unfortunately I don't know a lot about the Python bindings either. @sjg20 care to weigh in?

Gosh, it is surprising that swig could change such a thing without some sort of backwards-compatible feature. Is there an indication that swig 4.2.0 will fix this? My reading of the change log for that swig did not produce certainty that the problem will be fixed there ("Python 3.12 support added." - is that it?)

I am not sure if the swig change is a bug, or a feature. If the latter, then checking the Python version would work, I suppose. But it is quite unfortunate. So far as I can tell, this changes the API of pylibfdt, so get_mem_rsv() returns a different result. This will presumably break most code that uses pylibfdt, unless I am missing something? In fact I wonder how pylibfdt works on Fedora today?

dgibson commented 10 months ago

Yeah.. unfortunately I don't know a lot about the Python bindings either. @sjg20 care to weigh in?

Gosh, it is surprising that swig could change such a thing without some sort of backwards-compatible feature. Is there an indication that swig 4.2.0 will fix this? My reading of the change log for that swig did not produce certainty that the problem will be fixed there ("Python 3.12 support added." - is that it?)

:(

I am not sure if the swig change is a bug, or a feature. If the latter, then checking the Python version would work, I suppose. But it is quite unfortunate. So far as I can tell, this changes the API of pylibfdt, so get_mem_rsv() returns a different result. This will presumably break most code that uses pylibfdt, unless I am missing something? In fact I wonder how pylibfdt works on Fedora today?

Looks like it doesn't: https://github.com/dgibson/dtc/actions/runs/7649045871/job/20842803203

zumbi commented 9 months ago

Yeah.. unfortunately I don't know a lot about the Python bindings either. @sjg20 care to weigh in?

Gosh, it is surprising that swig could change such a thing without some sort of backwards-compatible feature. Is there an indication that swig 4.2.0 will fix this? My reading of the change log for that swig did not produce certainty that the problem will be fixed there ("Python 3.12 support added." - is that it?)

I am not sure if the swig change is a bug, or a feature. If the latter, then checking the Python version would work, I suppose. But it is quite unfortunate. So far as I can tell, this changes the API of pylibfdt, so get_mem_rsv() returns a different result. This will presumably break most code that uses pylibfdt, unless I am missing something? In fact I wonder how pylibfdt works on Fedora today?

I tried swig 4.1.0 and 4.2.0, the testcase for pylibfdt fails with both versions using Python3.12, but works fine on previous Python version 3.11.

petermarko commented 9 months ago

Well, python upgrades just break things. There was a hope that "compatibility with python 3.12" will fix it, but it didn't. I don't think I'm able to fix this issue properly, so we either merge this red or wait for someone with skills to fix it...

blmaier commented 8 months ago

The pylibfdt failure on Fedora should be fixed by https://github.com/dgibson/dtc/pull/128

dgibson commented 8 months ago

Despite the pylibfdt failure, I merged this some time ago. Closing.