Skip to content

examples: add a unit test to ensure that qrc works#981

Merged
LeonMatthesKDAB merged 1 commit intoKDAB:mainfrom
ahayzen-kdab:add-qrc-test
Jul 8, 2024
Merged

examples: add a unit test to ensure that qrc works#981
LeonMatthesKDAB merged 1 commit intoKDAB:mainfrom
ahayzen-kdab:add-qrc-test

Conversation

@ahayzen-kdab
Copy link
Collaborator

No description provided.

@LeonMatthesKDAB
Copy link

Hm, I wonder whether we even want to support QRC files this way in a CMake-driven build.

There is really little reason to do this over simply adding the qrc file to the CMake build 🤔

This should definitely work if you're running a Cargo-only build, but maybe we shouldn't support this in CMake-based builds?

@ahayzen-kdab
Copy link
Collaborator Author

I think it'd be tricky to explicitly disable for CMake builds and in theory shouldn't really be any different between CMake or Cargo builds as seen in the diff here.

For the object files the initialiser can just go into the crate's general initialiser so that we don't need to know the qrc name at the figure stage?

@LeonMatthesKDAB
Copy link

That would work, as long as the qrc call happens in the build script of the final crate.
However, it wouldn't work for libraries, unless we somehow add this to the opts as well.
Which would definitely require a different API, as you couldn't just call qrc on the builder 🤔

@ahayzen-kdab
Copy link
Collaborator Author

Hmm fun, but that is also true for cargo builds too? Any qrc's added in cxx-qt-lib would need an initialiser to be passed through via opts to the final build script?

@LeonMatthesKDAB
Copy link

Cargo builds should work, even with dependencies as I'm adding the object file generated by each qrc file to the linker if it's a cargo-only build.

But I also haven't tested that yet.

Copy link

@LeonMatthesKDAB LeonMatthesKDAB left a comment

Choose a reason for hiding this comment

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

more test, more better, and this should be doable to support in our upcoming changes in #986

@LeonMatthesKDAB LeonMatthesKDAB enabled auto-merge (rebase) July 8, 2024 08:48
@LeonMatthesKDAB LeonMatthesKDAB merged commit cb71242 into KDAB:main Jul 8, 2024
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