Windows: Fix compile after SB interface#277
Windows: Fix compile after SB interface#277mavenugo merged 1 commit intomoby:masterfrom microsoft:10662-compile3
Conversation
sandbox/sandbox.go
Outdated
There was a problem hiding this comment.
I know it can be frustrating to see windows builds broken all the time. But this cannot be a non-windows version. I don't think we should duplicate the interface itself
There was a problem hiding this comment.
@mrjana - Why not? I don't understand. Based on previous statements, at this time we do not expect Windows to be using Sandbox, which is why _unsupported is set to build on Windows (via +build !linux). This change simply addresses the compile break from introducing the interface. As I've stated, we still fully expect docker (via libnetwork) to be passthrough on Windows down to the Hyper-V virtual switch via HCS and not using sandbox. What sandbox does on Linux has no parallel on Windows - the networking stacks are so vastly different.
(BTW, at the time of submission, I did have Windows compilation working. However, revisiting, it appears that is not the case, not sure why. Will update this PR or submit a new one when that is solved - working on it pri1. However, the sentiment of this PR still remains)
There was a problem hiding this comment.
@jhowardmsft I agree with @mrjana. The main reason for having a generic Sandbox interface and platform specific implementation is to provide just that support. Its perfectly understandable that the implementation of that API interface in windows could be different (or even be absent for the fore-seeable future). But that detail must be kept on the implementation and keep the API free of platform specific details.
Yes, without a proper CI, Linux specific implementation commits are inadvertently breaking other platforms. As Maintainers we will be extra cautious. And infact push harder to have a good CI in place for cross-platform testing.
I think that will solve the nagging issue that you are facing.
There was a problem hiding this comment.
@mavenugo Sure, I can rework this if you feel necessary (I tried and failed on 2 attempts - it's really not that easy). But, I really do have to ask why. You're asking to keep an interface in a platform which isn't being used, requiring far more code to be pulled in for compilation and execution path than is necessary, and making x-platform CI harder for everyone. That IMO simply doesn't make sense. Not meaning to vent - trying to be constructive, please take it that way :)
|
|
|
LGTM |
Signed-off-by: John Howard <jhoward@microsoft.com>
|
LGTM. |
Windows: Fix compile after SB interface
3 participants
Signed-off-by: John Howard jhoward@microsoft.com
@swernli. After Sandbox was refactored to an Interface, Windows compilation was broken again. This fixes it up.