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

Add a Windows CI. #695

Merged
merged 9 commits into from
Aug 31, 2024
Merged

Add a Windows CI. #695

merged 9 commits into from
Aug 31, 2024

Conversation

mgautierfr
Copy link
Member

Compilation of kiwix-tools is broken on Windows.
But let's setup the CI to validate the PR (to be fixed)

Compilation of kiwix-tools is broken on Windows.
But let's setup the CI to validate the PR.
@kelson42
Copy link
Contributor

I though we have Windows CD using win2019 to ensure binaries run on older versions of Windows. But here CI is only win2022... which would be a pb.

What are version of windows used for CI, for CD? And why?

@mgautierfr
Copy link
Member Author

win2019 vs win2022 is mainly about visual studio version.
Windows itself is the same.

I suppose we should use win2022 everywhere. But it seems not important as you have added a libzim CI using win2022 which compile with deps build on win2019...

@kelson42
Copy link
Contributor

@mgautierfr Then why relying on win2019 at all? We should use only 2022 version everywhere.

@mgautierfr
Copy link
Member Author

Then why relying on win2019 at all?

Where ?

@mgautierfr mgautierfr force-pushed the windows_ci branch 2 times, most recently from b48daf4 to 0ba913f Compare August 30, 2024 12:29
@mgautierfr mgautierfr marked this pull request as ready for review August 30, 2024 12:34
@mgautierfr
Copy link
Member Author

Packages workflow because kiwix-tools cannot found the docopt dependencies.
However, I have added the dependency to debian/control and it is installed (from logs).
We are doing the same thing than in zim-tools (where it works).

The only difference with zim-tools is that docopt is a new dependency here. Maybe there is some kind of cache somewhere ?
@kelson42 @legoktm do you have an idea ?

Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

I mainly looked at the C++ code. Note that I didn't check that the usage strings were updated accurately.

src/searcher/kiwix-search.cpp Outdated Show resolved Hide resolved
src/searcher/kiwix-search.cpp Outdated Show resolved Hide resolved
src/server/kiwix-serve.cpp Show resolved Hide resolved
meson.build Show resolved Hide resolved
src/server/kiwix-serve.cpp Outdated Show resolved Hide resolved
@mgautierfr
Copy link
Member Author

New version splits commit and small fixes of previous review are already squashed.

Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

Rebase-fixup-and-merge before @kelson42 merges without fixing-up :)

@kelson42
Copy link
Contributor

@mgautierfr All the deb packages pass on main, so I guess it should be green here too.

@mgautierfr
Copy link
Member Author

Yes, see this comment : #695 (comment)

@kelson42
Copy link
Contributor

kelson42 commented Aug 31, 2024

@mgautierfr @veloman-yunkan For an unknow reason xmake seems to be missing https://github.com/kiwix/kiwix-tools/actions/runs/10634049570/job/29480511704#step:10:507

@tusharpm Maybe you understand better what is missing here?

@tusharpm
Copy link

Try adding cmake to Build-Depends in debian/control. It seems meson is unable to locate the installed libdocopt-dev using pkg-config.

@kelson42
Copy link
Contributor

So, now it passes and I'm going to merge.

@kelson42 kelson42 merged commit 9848317 into main Aug 31, 2024
10 of 11 checks passed
@kelson42 kelson42 deleted the windows_ci branch August 31, 2024 12:24
@kelson42 kelson42 added this to the 3.8.0 milestone Aug 31, 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.

4 participants