Windows build is failing

[jonathf]

Failing build:
https://github.com/VROOM-Project/pyvroom/runs/4992174810

Looks like #include getopt_win.h is a Windows only missing dependency in vroom/src/main.cpp.

@nilsnolde, do you mind taking a look when you have a moment?

[nilsnolde]

I didn't follow too closely how you incorporated the main in the bindings tbh, but you'd need this file to solve the win build (right now): https://github.com/VROOM-Project/vroom/blob/master/include/getopt_win.h. it's a baaah win "hack" around a baaah unix implementation of a command line parser.

I say 'right now' bcs there's a PR pending that I didn't get to yet: VROOM-Project/vroom#604. once I address julien's comments there, this hacky file isn't needed anymore. we'd then have to solve including cxxopts as submodule to build the main in the bindings, but that shouldn't be a worry. should we leave it for now? I'll get to it soon-ish, right now I'm not working on anything for lots of reasons, but soon enough I will again:)

[jonathf]

No worries. We all know how things works in open source.

I've made a naive atempt of just adding the missing includes in #48, but without any luck.
I'll might disable the windows build for now so that the docs and the software deviate too much short term.

[nilsnolde]

now that VROOM-Project/vroom#604 is merged, we could finally fix this. I wanted to quickly do it, then I realized that you're of course pointing to your vroom fork.

so I forked your fork, merged upstream master (resolved conflicts with relative naivety), did the tiny setup.py change and it worked on win, vroom.exe -i vroom\docs\example_2.json gave correct results (though vroom.exe -h doesn't print anything on cmd..). I tried CI, but of course I didn't go through the ordeal of test-wise switching vroom's submodule origin repo to mine.

so what we'd need to do:

• merge upstream master properly (if you have the time at some point, quite a few changes & conflicts, see here my try)
• cxxopts is now a submodule of vroom so we need to patch setup.py with just one line:

  pyvroom/setup.py

  Line 13 in 8eb2c1f

  os.path.join("vroom", "include", "cxxopts", "include")

• windows-latest switched to win 2022, we need to update the workflow yml's with conan profile update "settings.compiler.version=17" default (or maybe rather specify windows-2022 so we don't get those surprises?)

that should be it for all OSs and enough to make win work again.

[jonathf]

Hey @nilsnolde.

I've been sick a while now with vestibularis nevrit (virus on the balance nerve) which means nothing has been done in a while. I am slowly getting better, but money making projects need to take precedence for a while.

Sounds like you have overview and formed a plan that should be fairly straight forward to implement. If you make a PR for it, I will prioritize reviewing it. but if not, I'll give it a shot myself when time and health allows.

And as always, thank you for the effort!

[nilsnolde]

Uff, don’t worry at all. There’s no priority on my side. I’ll still put the 2 PRs in the next days, no stress in reviewing! All the best recovering soon!

[jonathf]

@nilsnolde, I'm back a bit more again.

I've made a branch re-enabling windows again at #56.
Looks like we have some issues now building the dependencies. Is this a known issue for you?

[nilsnolde]

you missed the other steps;) not sure if it really builds now, but it should..

so that means that the PR at jonathf/vroom#1 might be ok? it's a horrible diff, but if you diff it to vroom/master commit instead it gets more reviewable;) btw, is there still a lot outstanding before being able to have the original repo as submodule?

[jonathf]

I've updated master on jonathf/vroom. It doesn't look like there are any differences of significances anymore as I think they are fixed upstream.

[jonathf]

Assuming this is solved for now.

[jonathf]

(#56 is now merged.)

[nilsnolde]

@jonathf the release build is failing for win again. didn't have a look, but my guess is you just changed the fork back to yours without merging my PR? there's a few changes in there, without those win won't build.

[jonathf]

I saw it to. I had another branch goung in parallel and suspected something like that. I'm taking a look tomorrow.