-
Notifications
You must be signed in to change notification settings - Fork 13
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
run ci tests on debian 10 #61
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #61 +/- ##
=======================================
Coverage 41.60% 41.60%
=======================================
Files 6 6
Lines 274 274
=======================================
Hits 114 114
Misses 160 160 ☔ View full report in Codecov by Sentry. |
ci/test.sh
Outdated
cp ci/libnss_whatami.so.2 /lib | ||
sed -i 's/\(passwd\|group\):/& whatami/' /etc/nsswitch.conf | ||
dpkg -i nsncd*.deb | ||
/usr/lib/nsncd & |
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.
If you need to do this it means the dpkg -i
isn't starting the daemon correctly (possibly because you're in a container without systemd? less likely, possibly because there's a /usr/sbin/policy-rc.d file in whatever chroot/environment you're in?). I think we should figure out how to run the test in a context where systemd is running, if possible, so we have test coverage for the systemd unit. Maybe that involves splitting this file into two separate tests, one for the systemd unit on the uncontained ubuntu-latest runner and one for the various platforms we support in containers.
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.
It was because there's no systemd in the container, I tried running systemctl status and the command wasn't there.
Yeah, I can add a test on Ubuntu with systemd
ughhhhhhhhhhhhhhhhhh https://github.com/twosigma/nsncd/actions/runs/4841525033/jobs/8627850960 |
Ok @geofft I figured out why this is happening but not what to do about it.
The annoying bit is right near the end:
All the symbols that it sees except for
It's basically saying, we won't admit to there being some version range where Is there some way you know of to tell dpkg-shlibdeps "no really just don't worry about that symbol"? |
Ok this works but it's gross d5dffe6 |
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.
Yeah, I'm not sure of a better way to do that. I think you could use debian/shlibs.local to suppress the normal symbols+shlibs handling of libc? But you'd need to get the version bound right yourself, which seems worse than what you're doing.
The files generated by this PR work on our internal Debian 10 and 11 systems, so this seems like a good change.
As I mentioned to you the other day, I kind of want to just redo this entirely to build the Rust binaries with normal online cargo build --release
inside a manylinux container, on the grounds that manylinux (of whatever version) is a more defensible libc baseline than "the oldest thing Two Sigma happens to run is Debian 10," and then stuff that binary into Debian packages as well as other packaging systems like RPM. Then we can drop the debian/rules vendor
stuff. (But I'd still want to use the ordinary Debian packaging infrastructure as opposed to just tossing a .deb together ourselves, which means dh_shlibdeps
will still get called on the binary, so the workaround remains applicable and having figured it out remains worthwhile.)
@@ -17,6 +17,7 @@ jobs: | |||
|
|||
steps: | |||
- uses: actions/checkout@v3 | |||
- uses: actions-rust-lang/setup-rust-toolchain@v1 |
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.
As far as I can tell, actions-rust-lang
is owned by neither GitHub nor the Rust project. Do you know if either of these parties is working on an official action? If not, either
- is
v1
a tag we can audit or something, which will not change until we change this file? - how annoying is it to use rustup instead?
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.
We can probably(?) do rustup, I copied this from #58
sudo apt-get update | ||
sudo apt-get install build-essential dpkg-dev | ||
apt-get update | ||
apt-get install -y build-essential dpkg-dev ca-certificates sudo curl |
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.
Do you remember what you needed these for? Especially sudo
is a little surprising in a container.
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.
I believe actions-rust-lang/setup-rust-toolchain
wanted them, but I can check in a bit.
No description provided.