KhronosGroup / KTX-Software

KTX (Khronos Texture) Library and Tools
Other
834 stars 222 forks source link

Move self-hosted dependencies to `external` folder #909

Closed MathiasMagnus closed 1 month ago

MathiasMagnus commented 2 months ago

This PR splits the work prototyped in #889 to have only the file reorg in preparation for Clang-tidy and Clang-format.

Notes for the reviewers

CLAassistant commented 2 months ago

CLA assistant check
All committers have signed the CLA.

aqnuep commented 2 months ago

@MarkCallow, this is the first step we already discussed in PR #889, moving the external dependencies to a dedicated external repo. Please review it so that we can merge it ASAP.

MarkCallow commented 2 months ago

Not sure why you need that fix for gl3loadtests. It's been building on Windows for a very long time.

You still need to fix .reuse/dep5 for some of the moved files. The reuse test on TravisCI is one of the failing jobs.

You need to fix paths in the Web-related sources, interface/js_binding. That is the cause of the other TravisCI failures.

I'll start a proper review later this week. I'm still working on PR #874 (coding not reviewing).

MarkCallow commented 1 month ago

reuse is still failing related to external/fmt.

I've reviewed the basisu-related changes. They look good. More tomorrow.

MathiasMagnus commented 1 month ago

@MarkCallow I fixed the Reuse issues. CI is now failing on the Emscripten bindings, but I believe it is unrelated to the changes here and is a result of the image having changed in the last few hours. The compilation error is not include or link related but a DTOR being explicitly marked deleted.

MarkCallow commented 1 month ago

@MarkCallow I fixed the Reuse issues. CI is now failing on the Emscripten bindings, but I believe it is unrelated to the changes here and is a result of the image having changed in the last few hours. The compilation error is not include or link related but a DTOR being explicitly marked deleted.

Aargh! Why do people keep making breaking changes?

I am just finishing up major changes to the JS wrapper. I've been using Emscripten 3.1.59. This failing build is using 3.1.60. It is appropriate that the subject copy constructor is marked deleted. I'll probably have to consult the Emscripten people to figure out a fix. I might have to get the major wrapper changes merged to main before you can get the fix. I'll look into it tomorrow. It is late evening here now.

MarkCallow commented 1 month ago

The changes to Emscripten are apparently intentional. However the fix they suggested makes zero difference. Finding a fix will take more time.

MarkCallow commented 1 month ago

Here is a patch for the Emscripten 3.1.60 issue. Please add it to this PR.

ktx_wrapper.patch

MathiasMagnus commented 1 month ago

@MarkCallow @aqnuep All lights are green.

MarkCallow commented 1 month ago

@MathiasMagnus thank you for this well organized piece of work.