In reference to this pull request
https://github.com/OpenChemistry/avogadrolibs/pull/1911
I was trying to run tests on elementtest.cpp using Qt6, but it always gives these errors while building
Though in the build directory, the tests directory is created by running this command cmake -DQT_VERSION=6 -DBUILD_MOLEQUEUE=OFF -DENABLE_TESTING=ON -DCMAKE_BUILD_TYPE=Release -S ./openchemistry -B ./build but it is mainly empty.
Please make sure you’re using HEAD. I fixed that in a separate commit.
Thank you so much. Molequeue package error was gone but there is a new error about Qt. I am trying to solve it.
When I was trying to fix this, it was giving these errors
virtual void onRecordEvent(const QString& widget, const QString& command,
const QString& arguments,
const int& eventType = 0) override
{
to
virtual void onRecordEvent(const QString& widget, const QString& command,
const QString& arguments) override
{
const int& eventType = 0;
int getNextEvent(QString& widget, QString& command, QString& arguments,
int& eventType) override
{
to
int getNextEvent(QString& widget, QString& command, QString& arguments) override
{
if (this->XMLStream->name() == "event")
to
if (this->XMLStream->name() == QString("event"))
and then it worked and this error was solved but a new error came in:
What were the changes I made wrong?
You updated your avogadrolibs repository but did not update avogadroapp. There was a change to the qttesting library and it’s updated accordingly.
Since we have pieces in multiple sub repositories you will sometimes need to update all of them.
I updated them using this command git submodule update --remote as mentioned in this documentation https://two.avogadro.cc/develop/build.html#develop-build
Should I update them individually?
You should probably add --recursive i.e. run git submodule update --remote --recursive, and I’ll change the docs to say that.
That shouldn’t make a difference to your current issue though really, as using --remote should be sufficient to update all submodules to point at the remote HEAD, as covered in the git documentation.
One thing to note is that when you run git submodule update --remote it will look to each submodule’s origin remote by default. This means that if you have forked any of the individual submodule repos yourself (avogadroapp, for instance) and have set it up as I have – so that the origin remote for avogadroapp is defined as matterhorn103/avogadroapp and OpenChemistry/avogadroapp is called upstream – then it will update to the current state of your own fork rather than to upstream. But if you haven’t changed anything about the remotes and don’t have any forks then by default origin will be OpenChemistry/avogadroapp, so git submodule update --remote should work fine. If in doubt, git config branch.master.remote will tell you the name of the remote that the submodule is tracking and git remote -vv lists all the configured remotes.
Like I say, it shouldn’t be necessary, but you certainly can simply run git pull from within individual submodules. You can also run git pull --recurse-submodules or git submodule foreach git pull to do it for all of the submodules at once. There is a subtle difference in what these recursive commands actually do in comparison to git submodule update --remote in that they do just essentially work like git pull does.
This is mostly going by the git docs and this stackoverflow thread.
A couple of other things:
git submodule update you need to add the --init flag. You probably did that when you first cloned the repo, since it’s also on the page you linked, but just figured I’d mention it.openchemistry superrepo is up-to-date with OpenChemistry/openchemistry – you’ll need to run git pull or whatever for it too.If none of that helps then it would be good if you could go into avogadroapp and avogadrolibs and use git log to work out which commit each submodule is pointing at, then you can see (by looking at GitHub) or we can tell whether you are definitely using the up-to-date code. Because if you are then naturally it’s a legitimate bug that needs fixing.
Finally, it looks like you’re running the build from within VS Code on Linux? Have you tried building from the command line using your normal terminal?
@ghutchis The big issue with telling people to run git submodule update --remote is that it will also update the third-party git submodules like VTK to point at the latest upstream code.
Our builds don’t do that, they checkout each OpenChemistry submodule individually and leave the third-party ones to point at the commits specified in OpenChemistry/openchemistry. So that becomes a point of difference.
Ok, I think we can say that the canonical way to get your code up-to-date is to navigate to the openchemistry umbrella repo and then run:
git pull
git submodule update --recursive
git submodule update --remote -- avogadroapp avogadrolibs
Which do the following:
openchemistry repo up-to-date with the current state of OpenChemistry/openchemistryOpenChemistry/openchemistryOpenChemistry/avogadroapp)If you have forked anything then the above will likely update to match your fork instead, so you need to make sure your fork is synced with upstream.
IIRC I think you were suggesting some changes to openchemistry so there was a stable branch and then in main the remotes (e.g., avogadroapp and avogadrolibs etc.) would always point to the latest revisions?
That’s probably worth changing.
Yeah I did, that would make things simpler in various places. I just never managed to get it working!
Though rereading the stackoverflow thread that made me think it was possible, I’m no longer sure it is, now that I understand better how it all works. It seems like you’d still need to run git submodule update --remote, and possibly we are already doing what the answers suggest doing.
In any case I have a branch where I have set up the avogadroapp and avogadrolibs submodules to track the branches, which changes .gitmodules from
[submodule "avogadroapp"]
path = avogadroapp
url = https://github.com/OpenChemistry/avogadroapp
to
[submodule "avogadroapp"]
path = avogadroapp
url = https://github.com/OpenChemistry/avogadroapp
branch = master
So we can see if it works any differently to the current setup by just waiting until a new commit is made to avogadrolibs; currently the link points to 05d2dd0. I’ll report back.
This means that if you have forked any of the individual submodule repos yourself (
avogadroapp, for instance) and have set it up as I have – so that theoriginremote foravogadroappis defined asmatterhorn103/avogadroappandOpenChemistry/avogadroappis calledupstream– then it will update to the current state of your own fork rather than to upstream.
I use sync fork before doing that. Is that ok?
Finally, it looks like you’re running the build from within VS Code on Linux? Have you tried building from the command line using your normal terminal?
Yes I have sometimes tried that but I use VS code more often.
After updating the repos as you have mentioned I am still getting the same error.
I removed it by using QString(event).
But this Qstring(event) isn’t a part of remote repository so please guide me if I am doing anything wrong
As long as you have synced any and all of your forks, not just openchemistry, then yes.
I was able to build the tests after making this Qstring(event) change in my local repository. I also ran tests on this PR https://github.com/OpenChemistry/avogadrolibs/pull/1911
