-
Notifications
You must be signed in to change notification settings - Fork 32
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
CMake build system + msys2 windows binaries CI (fixes #11, fixes #148) #177
Conversation
fe847b0
to
b95bdfc
Compare
b95bdfc
to
60250a3
Compare
2906671
to
08c801b
Compare
08c801b
to
a5469e9
Compare
@gkdr, I made this^^ ready for review now. The pre-merge GitHub Action logs can be viewed at https://github.com/hartwork/lurch/actions . Moving CodeCov integration to GitHub Actions will need a new secret on the repository. We can do that migration before or after merge of this pull request. PS: AppVeyor CI seems to also need disabling on their website, deleting the file alone doesn't make it stop. |
1fc7b2a
to
3383abd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like the windows result is not loadable yet.
report from a friend using windows, for both versions, all the zip contents unpacked into the plugins dir:
(21:41:43) plugins: probing C:\Users\USER\AppData\Roaming\.purple\plugins\lurch.dll
(21:41:43) plugins: C:\Users\USER\AppData\Roaming\.purple\plugins\lurch.dll is not loadable: `C:\Users\USER\AppData\Roaming\.purple\plugins\lurch.dll': The specified module could not be found.
0e87f83
to
4d1c180
Compare
@gkdr I notice now that for Windows test |
4d1c180
to
dea5c85
Compare
@gkdr it turned out that doing that^^ fixed the issue in the CI. It also implies that your friend did not have all needed files at the right places. My guess is still the jabber plugin DLL. |
dea5c85
to
76ef2ce
Compare
is it maybe related to setting the Line 90 in 71c806b
|
76ef2ce
to
d784899
Compare
@gkdr thanks for bringing RPATH to my attention, this is important and was not working as needed. What I found out is that:
Because of all that, what I have done now is this:
Does that approach work for you? |
@gkdr you keep running away from #pidgin on librachat before you see my replies :) It looks like the lurch dll's are trying to load libxml2-2.dll weirdly, trying to load DllMain() as a funtion, rather than linking to the functions in libxml: I'm using https://github.com/lucasg/Dependencies and https://www.dependencywalker.com to scan for dependencies Compare to other dlls that get loaded which look like Also the plugin is trying to load in libsqlite3-0.dll, but Pidgin ships with sqlite3.dll, so might be superflous? |
thanks a lot, @EionRobb! that does indeed look a bit weird and is a very good starting point :D |
Also, (and its probably unrelated) just running something else through GDB and saw:
|
@gkdr Can you merge this PR? |
d784899
to
8cc57b8
Compare
@gkdr: Any news on this old important PR? |
Hi @Neustradamus! At this point, as far as I know, this does not work, as in the resulting plugin won't load on Windows. Neither of us has a Windows computer to test this, so I don't know how to develop it. |
@gkdr as long as a re-run gets green CI, we could merge it and ask for pull requests from people actually using Windows. What do you think? |
I can take another look. |
I couldn't get it to build on Windows, as there's no Tested by building on Windows with |
.. e.g. to address warning "Node.js 16 actions are deprecated".
@EionRobb thanks for trying and the hint, I have adjusted for 64bit now and the CI is now back to green. |
Would you like to continue with the current Makefile on |
@hartwork Thanks for the reminder. Especially the test for loadability seems nice. I tried looking at CMake for a while and to be honest I just cannot grasp it. I am hesitant to add it because currently I cannot use it. Do you have a few pointers? The CMake homepage didn't seem to help. |
Hi @gkdr, you and I migrated both of Lurch's dependencies axc and libomemo to CMake in 2022…
…, and at the time you considered that "a major step towards the automated windows build of Now after two and a half years of this pull request being mostly ignored, I have no motivation left for helping you value and merge this pull request any more. It's not been fun to wait for you forever after all the effort I put into this, it's been too long, and I will not make the same mistake twice. I am now closing this pull request, it's about time. Thanks for your understanding. Best, Sebastian |
Yes, the goal was an automated build for Windows, like this PR's title also says. We didn't quite get there, that's why it's not merged. Thanks for all your time and effort up to this point. If you leave your branch up, I might pick it up someday, if you don't mind. |
Fixes #11
Fixes #148