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

Make server4 package compile on Windows #428

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

guillaumerose
Copy link
Contributor

I am working on an userland network stack based on gvisor (https://github.com/containers/gvisor-tap-vsock).
I use this library as the dhcp server. It works really great but doesn't compile on Windows.

This PR allows to start the server on Windows with a net.PacketConn directly server4.WithConn().

It also requires this PR in u-root: u-root/u-root#2019

@pmazzini
Copy link
Collaborator

lgtm, thanks for the PR. Can you add comments to the exported functions?

@guillaumerose
Copy link
Contributor Author

I added comments. I was not really inspired. If you have better ideas, I take them :)

@guillaumerose
Copy link
Contributor Author

guillaumerose commented May 25, 2021

u-root/u-root#2019 is merged and I can't update it here in the go.mod.
I tried go get -u github.com/u-root/u-root, etc.

$ go get github.com/u-root/u-root@efa3f7929d27a919286246551cb146506e734d13
go: github.com/u-root/u-root efa3f7929d27a919286246551cb146506e734d13 => v0.0.0-20210525171738-efa3f7929d27
go get: inconsistent versions:
	github.com/u-root/u-root@v0.0.0-20210525171738-efa3f7929d27 from github.com/u-root/u-root@efa3f7929d27a919286246551cb146506e734d13 requires github.com/u-root/u-root@v7.0.0+incompatible (not github.com/u-root/u-root@v0.0.0-20210525171738-efa3f7929d27 from github.com/u-root/u-root@efa3f7929d27a919286246551cb146506e734d13)

Do you have an idea?

replace github.com/u-root/u-root => github.com/u-root/u-root v0.0.0-20210525171738-efa3f7929d27

works fine.

pmazzini
pmazzini previously approved these changes May 28, 2021
Copy link
Collaborator

@pmazzini pmazzini left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov
Copy link

codecov bot commented May 28, 2021

Codecov Report

Merging #428 (1d3b11e) into master (fb4eaaa) will not change coverage.
The diff coverage is n/a.

❗ Current head 1d3b11e differs from pull request most recent head 3a04e9c. Consider uploading reports for the commit 3a04e9c to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #428   +/-   ##
=======================================
  Coverage   67.42%   67.42%           
=======================================
  Files          88       88           
  Lines        3665     3665           
=======================================
  Hits         2471     2471           
  Misses       1030     1030           
  Partials      164      164           
Flag Coverage Δ
integtests ∅ <ø> (∅)
unittests 67.42% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
dhcpv4/server4/conn.go 37.03% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb4eaaa...3a04e9c. Read the comment docs.

@pmazzini
Copy link
Collaborator

Do you have an idea?

No, not really.

@pmazzini
Copy link
Collaborator

pmazzini commented May 28, 2021

with regards to u-root, maybe this helps: #429

@hugelgupf
Copy link
Collaborator

yeah, #429 def helps. I took the windows change into the new repo as well.

The user can pass directly a net.PacketConn.

Signed-off-by: Guillaume Rose <gurose@redhat.com>
@guillaumerose
Copy link
Contributor Author

Yes #429 is great. I renamed conn to conn_unix. This PR should be good now.

@guillaumerose
Copy link
Contributor Author

up! I think it is now good to merge, thanks

@pmazzini pmazzini merged commit 465dd6c into insomniacslk:master Jun 8, 2021
@pmazzini
Copy link
Collaborator

pmazzini commented Jun 8, 2021

Codecov report failing after merging this PR.

https://github.com/insomniacslk/dhcp/runs/2772122685

{'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')}

cc @insomniacslk

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.

None yet

3 participants