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

Update nc from 0.8.23 to 0.9.1 #2874

Closed
YJDoc2 opened this issue Aug 8, 2024 · 2 comments · Fixed by #2884
Closed

Update nc from 0.8.23 to 0.9.1 #2874

YJDoc2 opened this issue Aug 8, 2024 · 2 comments · Fixed by #2884
Labels
good first issue Good for newcomers kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@YJDoc2
Copy link
Collaborator

YJDoc2 commented Aug 8, 2024

In this major version update, there is an API breaking change in nc, which we have to fix manually. I believe the fixes are easy to do and validate, but still need to be done properly as they touch some of our imp code parts.

See #2873 for breakage info via lint.

@YJDoc2 YJDoc2 added good first issue Good for newcomers kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Aug 8, 2024
@YJDoc2 YJDoc2 linked a pull request Aug 20, 2024 that will close this issue
@aidanhs
Copy link

aidanhs commented Aug 24, 2024

FWIW this bump makes musl static builds break for me with

  = note: /nix/store/a1l57qpyyr8rpxxk26lldv42fcjvps6h-x86_64-unknown-linux-musl-binutils-2.35.1/bin/x86_64-unknown-linux-musl-ld: /home/aidanhs/zircon/target/x86_64-unknown-linux-musl/release/deps/libnc-84a6815abfb93d73.rlib(nc-84a6815abfb93d73.nc.2c2307cd896e3a7-cgu.3.rcgu.o): in function `__restore_rt':
          nc.2c2307cd896e3a7-cgu.3:(.text+0x0): multiple definition of `__restore_rt'; /nix/store/hcd122h1i7ablkh2ygc2qjlkjvnzyf3i-rust-1.81.0-nightly-2024-07-19-9057c3ffe/lib/rustlib/x86_64-unknown-linux-musl/lib/self-contained/libc.a(restore.lo):/build/musl-cross-make/build/local/x86_64-linux-musl/obj_musl/../src_musl/src/signal/x86_64/restore.s:6: first defined here

My best guess at a quick look is that the musl __restore_rt is defined like so https://github.com/bminor/musl/blob/dd1e63c3638d5f9afb857fccf6ce1415ca5f1b8b/src/signal/x86_64/restore.s#L5-L7

__restore_rt:
	mov $15, %rax
	syscall

nc used to use the same implementation, so I believe the linker could deduplicate the symbols. But this changed to a custom implementation:

XuShaohua/nc@b67d499#diff-685ac5a9e7e590059d38c8c235bd2b94b36a9e15a7743baa045a7f4383100158L6-L8

@XuShaohua
Copy link

nc v0.9.1 has too many broken changes 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants