-
Notifications
You must be signed in to change notification settings - Fork 909
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
os: net.newUnixFile #4517
os: net.newUnixFile #4517
Conversation
Since the wasm CI seems to be broken in other PRs not touching this as well, I presume this PR did no changes to crash it (see #4521) |
The fix for nix is in... or at least for the CI. Please rebase against latest dev. |
Signed-off-by: leongross <[email protected]>
c66336b
to
68e8a75
Compare
@deadprogram @aykevl ci is green now, would appreciate a review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving to allow this to be merged and not block the net stuff, but I have concerns that we're drifting from Go's requirements for this function.
Thanks for the addition @leongross and to @dgryski and @rminnich for review. Now merging. |
Not sure why, but the Windows CI is now failing consistently after this PR was merged, even though the PR itself was passing. See https://github.com/tinygo-org/tinygo/actions/runs/11408347958/job/31767355704#step:17:587 I have restarted it already 3 times... Any ideas? |
It finally got thru the first step. Seems like some race condition and not related to this PR. Sorry about the noise. |
Building the u-root cmdlet
sluinit
with my experimentalnet
PR #4498 following error occurs:This PR resolves the issue of the missing function
net_newUnixFile
and links it into the net package.Since this package implements functionality for the linux net package, this code can currently not really be tested (see #4498).