Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further fixes for Qt6 #1713

Merged
merged 4 commits into from
Sep 16, 2024
Merged

Further fixes for Qt6 #1713

merged 4 commits into from
Sep 16, 2024

Conversation

matterhorn103
Copy link
Contributor

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: Matthew J. Milner <[email protected]>
Copy link
Contributor

Here are the build results
Avogadro2.AppImage
macOS.dmg
Win64.exe
Artifacts will only be retained for 90 days.

Copy link
Contributor

Here are the build results
Avogadro2.AppImage
macOS.dmg
Win64.exe
Artifacts will only be retained for 90 days.

@avo-bot
Copy link

avo-bot commented Sep 16, 2024

This pull request has been mentioned on Avogadro Discussion. There might be relevant details there:

https://discuss.avogadro.cc/t/dso-linking-error/5587/4

@ghutchis
Copy link
Member

Some minor gripes from clang-format
clang-format.txt

#include <QOpenGLWidget>

#if QT_VERSION >= 0x060000
#include <QtOpenGLWidgets/QOpenGLWidget>
Copy link
Member

Choose a reason for hiding this comment

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

That's weird - it shouldn't be necessary, but 🤷

Avogadro::Rendering
Avogadro::QtOpenGL)

target_include_directories(BondCentric
Copy link
Member

Choose a reason for hiding this comment

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

There's got to be a better way to do this with cmake. In theory, this is set up through the function(avogadro_plugin bits in qtplugins/CMakeLists.txt.

I'd guess that's a better thing to modify so this isn't needed in a bunch of plugin CMakeLists.txt

Copy link
Contributor Author

@matterhorn103 matterhorn103 Sep 16, 2024

Choose a reason for hiding this comment

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

Yeah, I think all those things aren't needed now that I've fixed the issue with Avogadro::Rendering being PRIVATE.

That was a fix I did 6+ months ago so the exact errors these fixes were solving are a little lost to the depths of my memory.

Hence also them being all in a single commit, which I know isn't very helpful – it's cause I had to do a messy manual rebase on top of all the changes to master that had happened in the meantime

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed all the target_include_directories bits with the latest commit.

@matterhorn103
Copy link
Contributor Author

Some minor gripes from clang-format clang-format.txt

Thanks, incorporated

@avo-bot
Copy link

avo-bot commented Sep 16, 2024

This pull request has been mentioned on Avogadro Discussion. There might be relevant details there:

https://discuss.avogadro.cc/t/september-2024-live-updates/5571/1

@ghutchis ghutchis added the build label Sep 16, 2024
Copy link
Contributor

Here are the build results
macOS.dmg
Avogadro2.AppImage
Win64.exe
Artifacts will only be retained for 90 days.

Copy link
Member

@ghutchis ghutchis left a comment

Choose a reason for hiding this comment

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

Good for now - I'd love to see if we can get rid of the target_include_directories pieces soon.

@ghutchis ghutchis merged commit 7d04a44 into OpenChemistry:master Sep 16, 2024
22 checks passed
@matterhorn103
Copy link
Contributor Author

Good for now - I'd love to see if we can get rid of the target_include_directories pieces soon.

I believe I did get rid of them all in 497d54b.

Copy link
Contributor

Here are the build results
Avogadro2.AppImage
macOS.dmg
Win64.exe
Artifacts will only be retained for 90 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants