feat(runtime): Adds conditional compilation for windows

[thomastaylor312]

This adds conditional compilation for the SIGINT interrupts. Due
to the macro, this is mostly a copy/paste job with one version
for windows

[nightkr]

1. Can't we just provide a dummy SIGTERM never-Future on Windows, rather than duplicating everything else?
2. Might want to enable CI for Windows. Evidently nobody really cares about Windows support at the moment, so regressions seem fairly likely.
3. You might interested in my proposal for a ::runtime redesign, which does away with "self-driving" and signal listeners entirely: Create a Controller<K> runtime #148 (comment)

[thomastaylor312]

Hmmm...so as for the dummy SIGTERM, let me give it a whirl. Should I enable CI as part of this PR or do it in another once this is merged?

With regards to the runtime work, is it going to be done soon enough that this PR won't be worth it? I am fine waiting if it is going to be done relatively soon, otherwise, I'd love to get this in

[nightkr]

Should I enable CI as part of this PR or do it in another once this is merged?

Should be fine to do it in this PR.

With regards to the runtime work, is it going to be done soon enough that this PR won't be worth it? I am fine waiting if it is going to be done relatively soon, otherwise, I'd love to get this in

watcher and reflector are already complete enough that they can replace kube::runtime::{Informer, Reflector}. controller could do with some more polish (notably, event deduplication) but kube doesn't have an equivalent abstraction anyway.

[thomastaylor312]

Ok, I just pushed a fix to use a dummy channel for the sigterm. Working on enabling windows support in CI. If you honestly feel like this work is probably not worth it given the incoming refactor, I am fine closing this

[thomastaylor312]

I'll do some more digging about why the ring build is failing next week (I think I need to use the msvc target, but not entirely sure as gnu seemed to work for me when using cross). This is my first time I've really been building a Rust project for windows, so if anyone has pointers, feel free to drop them here

[nightkr]

IMO it's fine if some optional features are broken, considering that the current state is "doesn't even build at all".

[clux]

Oh, this is strange. Why is this necessary?

[thomastaylor312]

Cargo tries to pull using git urls instead of https. Couldn't find another workaround

[clux]

Ok, I just pushed a fix to use a dummy channel for the sigterm. Working on enabling windows support in CI. If you honestly feel like this work is probably not worth it given the incoming refactor, I am fine closing this

Even if we are likely changing the way this is done, the hack is completely fine for now.

Having a working CI solution on windows would be the real benefit here, since it's not a platform that gets tested a lot. So if you are able to get circle to accept it, then this would be great 👍

[rylev]

You should be able to create a new scope and conditionally compile that scope to make this easier to read:

 #[cfg(not(target_family = "windows"))]
 let sigterm_fut = {
  use signal::unix::{signal, SignalKind};
  let mut sigterm = signal(SignalKind::terminate()).unwrap();
  sigterm.recv().fuse()
};

[thomastaylor312]

Sweet, I was wondering how I could do this

[thomastaylor312]

So it looks like this doesn't work because recv borrows the receiver and then that borrow doesn't last outside the scope

[thomastaylor312]

@clux @teozkr I think this is ready to go! Sorry for all of the notifications you probably got from all the force pushes while testing the windows build, but now the build seems to be working great.

[clux]

Sorry for all of the notifications you probably got from all the force pushes while testing the windows build, but now the build seems to be working great.

Not a problem at all! Thanks a lot for the PR.