KTX-Software icon indicating copy to clipboard operation
KTX-Software copied to clipboard

Move self-hosted dependencies to `external` folder

Open MathiasMagnus opened this issue 1 year ago • 8 comments

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

  • The changes here are broken into multiple commits with descriptive names for easier review. I would recommend skipping over changes introduced by commits "Move XYZ to external folder", as they are uninteresting shuffling of unchanged content.
  • The changes are expected to get squashed, as not every commit builds. (Moving and build script updates are in separate commits.)
  • The final commit fixes a hard build error on Windows (when building glloadtests)

MathiasMagnus avatar May 14 '24 14:05 MathiasMagnus

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 14 '24 14:05 CLAassistant

@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.

aqnuep avatar May 14 '24 15:05 aqnuep

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 avatar May 15 '24 14:05 MarkCallow

reuse is still failing related to external/fmt.

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

MarkCallow avatar May 20 '24 13:05 MarkCallow

@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.

MathiasMagnus avatar May 21 '24 10:05 MathiasMagnus

@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 avatar May 21 '24 10:05 MarkCallow

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

MarkCallow avatar May 22 '24 06:05 MarkCallow

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

ktx_wrapper.patch

MarkCallow avatar May 23 '24 02:05 MarkCallow

@MarkCallow @aqnuep All lights are green.

MathiasMagnus avatar May 28 '24 06:05 MathiasMagnus

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

MarkCallow avatar May 28 '24 08:05 MarkCallow