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

FreeBSD fixes #724

Merged
merged 9 commits into from
May 12, 2023
Merged

FreeBSD fixes #724

merged 9 commits into from
May 12, 2023

Conversation

valpackett
Copy link
Contributor

@valpackett valpackett commented May 7, 2023

It works, including the initial jump :)

Yeah SCM_REALTIME I'll need to submit to libc…

Fixes #274

@codecov
Copy link

codecov bot commented May 7, 2023

Codecov Report

Merging #724 (d824565) into main (2263697) will increase coverage by 0.04%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##             main     #724      +/-   ##
==========================================
+ Coverage   82.83%   82.88%   +0.04%     
==========================================
  Files          62       62              
  Lines       15946    15941       -5     
==========================================
+ Hits        13209    13212       +3     
+ Misses       2737     2729       -8     
Impacted Files Coverage Δ
ntp-os-clock/src/unix.rs 28.89% <55.55%> (+0.50%) ⬆️
ntp-udp/src/interface_name.rs 97.38% <100.00%> (ø)
ntp-udp/src/raw_socket.rs 74.12% <100.00%> (+0.36%) ⬆️

Copy link
Contributor

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

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

this is great!

  • I left 1 inline comment/question
  • yes it would be nice to have that constant in libc. I've found that process to be straightforward.
  • I'll try to setup CI here. this is a bit tricky because we need the appropriate linker (because ring has a build.rs), and this failed earlier for freebsd. but maybe it works now that the code works.
  • we'll likely want to merge the macos stuff first, and then merge this into main. this may take a little while

folkertdev

This comment was marked as duplicate.

@folkertdev folkertdev deleted the branch pendulum-project:main May 11, 2023 09:32
@folkertdev folkertdev closed this May 11, 2023
@folkertdev
Copy link
Contributor

eh, I'm not sure what happened here? this should have rebased onto main. weird

@folkertdev folkertdev reopened this May 11, 2023
@folkertdev folkertdev changed the base branch from macos-support to main May 11, 2023 09:37
@folkertdev
Copy link
Contributor

@valpackett do you have thoughts on how to have users install ntpd-rs on freebsd? We provide deb and rpm installers which put the binary and config in the right place, and configure some permissions and user groups.

The installers are built with ploutos, which does not support freebsd currently. I'm really not sure how easy that would be to add (though we know the NLNet labs folks well, so we can check with them)

In the meantime we could maybe have some instructions that a user would have to perform manually?

@folkertdev folkertdev force-pushed the fbsd branch 3 times, most recently from 66df438 to 7fb2b55 Compare May 11, 2023 14:29
@valpackett
Copy link
Contributor Author

It's uncommon for projects to build their own .pkg independently and for anyone to want to install such packages :) Generally someone would submit a port and the official package will exist. It's no big deal, there is no barrier to entry, and for such a high profile project you can be sure someone will show up to do it soon. (I'll drop a link to here in some places in fact.. :D)

ploutos is really just using cargo-deb and cargo-generate-rpm, I'm pretty sure nothing like that exists for FreeBSD pkg for the above reason.

Copy link
Member

@davidv1992 davidv1992 left a comment

Choose a reason for hiding this comment

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

Looks good, one question on a weird second call.

Copy link
Contributor

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

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

ok, great. thanks so much for your help here @valpackett !

@folkertdev folkertdev merged commit 2511e7f into pendulum-project:main May 12, 2023
@valpackett valpackett deleted the fbsd branch May 15, 2023 05:16
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.

Add support for FreeBSD
3 participants